Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 78 additions & 8 deletions packages/app/src/cli/models/project/active-config.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {selectActiveConfig} from './active-config.js'
import {Project} from './project.js'
import {AppConfigurationAbortError} from '../app/error-parsing.js'
import {describe, expect, test, vi, beforeEach} from 'vitest'
import {inTemporaryDirectory, writeFile, mkdir} from '@shopify/cli-kit/node/fs'
import {joinPath, basename} from '@shopify/cli-kit/node/path'
Expand Down Expand Up @@ -155,34 +156,103 @@
})
})

test('throws when requested config does not exist', async () => {
test('throws a structured configuration abort when requested config does not exist', async () => {
await inTemporaryDirectory(async (dir) => {
await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "default"')
const project = await Project.load(dir)

await expect(selectActiveConfig(project, 'nonexistent')).rejects.toThrow()
try {
await selectActiveConfig(project, 'nonexistent')
expect.unreachable('Expected selectActiveConfig to throw')
} catch (error) {
if (!(error instanceof AppConfigurationAbortError)) throw error

expect(error).toMatchObject({

Check failure on line 170 in packages/app/src/cli/models/project/active-config.test.ts

View workflow job for this annotation

GitHub Actions / Unit tests with Node 20.14.0 in windows-latest (shard 2/2)

src/cli/models/project/active-config.test.ts > selectActiveConfig > throws a structured configuration abort when requested config does not exist

AssertionError: expected Error: Couldn't find shopify.app.nonexist… { …(6) } to match object { issues: [ { …(4) } ] } (5 matching properties omitted from actual) - Expected + Received - { + AppConfigurationAbortError { "issues": [ { "filePath": "C:/Users/runneradmin/AppData/Local/Temp/cbbc8552cfeafd1ded20bea6e4f5f33e/shopify.app.nonexistent.toml", - "message": "Couldn't find shopify.app.nonexistent.toml in C:\\Users\\runneradmin\\AppData\\Local\\Temp\\cbbc8552cfeafd1ded20bea6e4f5f33e.", + "message": "Couldn't find shopify.app.nonexistent.toml in C:/Users/runneradmin/AppData/Local/Temp/cbbc8552cfeafd1ded20bea6e4f5f33e.", "path": [], "pathString": "root", }, ], } ❯ src/cli/models/project/active-config.test.ts:170:23 ❯ runTask ../../node_modules/.pnpm/tempy@3.1.0/node_modules/tempy/index.js:18:10 ❯ src/cli/models/project/active-config.test.ts:160:5

Check failure on line 170 in packages/app/src/cli/models/project/active-config.test.ts

View workflow job for this annotation

GitHub Actions / Unit tests with Node 24.1.0 in windows-latest (shard 2/2)

src/cli/models/project/active-config.test.ts > selectActiveConfig > throws a structured configuration abort when requested config does not exist

AssertionError: expected Error: Couldn't find shopify.app.nonexist… { …(6) } to match object { issues: [ { …(4) } ] } (5 matching properties omitted from actual) - Expected + Received - { + AppConfigurationAbortError { "issues": [ { "filePath": "C:/Users/runneradmin/AppData/Local/Temp/b1f961a82656ec3a3e1717e89d0de274/shopify.app.nonexistent.toml", - "message": "Couldn't find shopify.app.nonexistent.toml in C:\\Users\\runneradmin\\AppData\\Local\\Temp\\b1f961a82656ec3a3e1717e89d0de274.", + "message": "Couldn't find shopify.app.nonexistent.toml in C:/Users/runneradmin/AppData/Local/Temp/b1f961a82656ec3a3e1717e89d0de274.", "path": [], "pathString": "root", }, ], } ❯ src/cli/models/project/active-config.test.ts:170:23 ❯ runTask ../../node_modules/.pnpm/tempy@3.1.0/node_modules/tempy/index.js:18:10 ❯ src/cli/models/project/active-config.test.ts:160:5
issues: [
{
filePath: joinPath(dir, 'shopify.app.nonexistent.toml'),
path: [],
pathString: 'root',
message: `Couldn't find shopify.app.nonexistent.toml in ${dir}.`,
},
],
})
}
})
})

test('throws when the only app config is malformed (no valid configs to fall back to)', async () => {
test('throws a structured configuration abort when the only app config is malformed', async () => {
await inTemporaryDirectory(async (dir) => {
// The only config is broken TOML — Project.load skips it and finds 0 valid configs
await writeFile(joinPath(dir, 'shopify.app.toml'), '{{invalid toml')

await expect(Project.load(dir)).rejects.toThrow(/Could not find/)
try {
await Project.load(dir)
expect.unreachable('Expected Project.load to throw')
} catch (error) {
if (!(error instanceof AppConfigurationAbortError)) throw error

expect(error).toMatchObject({
issues: [
{
filePath: joinPath(dir, 'shopify.app.toml'),
path: [],
pathString: 'root',
},
],
})
}
})
})

test('surfaces parse error when selecting a broken config while a valid one exists', async () => {
await inTemporaryDirectory(async (dir) => {
// Two configs: one good, one broken. Selecting the broken one by name should
// surface the real parse error via the fallback re-read, not a generic "not found".
await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "good"')
await writeFile(joinPath(dir, 'shopify.app.broken.toml'), '{{invalid toml')

const project = await Project.load(dir)

await expect(selectActiveConfig(project, 'shopify.app.broken.toml')).rejects.toThrow()
try {
await selectActiveConfig(project, 'shopify.app.broken.toml')
expect.unreachable('Expected selectActiveConfig to throw')
} catch (error) {
if (!(error instanceof AppConfigurationAbortError)) throw error

expect(error).toMatchObject({
issues: [
{
filePath: joinPath(dir, 'shopify.app.broken.toml'),
path: [],
pathString: 'root',
},
],
})
}
})
})

test('surfaces parse error when the default config is malformed and a named config exists', async () => {
await inTemporaryDirectory(async (dir) => {
await writeFile(joinPath(dir, 'shopify.app.toml'), '{{invalid toml')
await writeFile(joinPath(dir, 'shopify.app.staging.toml'), 'client_id = "staging"')

const project = await Project.load(dir)

try {
await selectActiveConfig(project)
expect.unreachable('Expected selectActiveConfig to throw')
} catch (error) {
if (!(error instanceof AppConfigurationAbortError)) throw error

expect(error).toMatchObject({
issues: [
{
filePath: joinPath(dir, 'shopify.app.toml'),
path: [],
pathString: 'root',
},
],
})
}
})
})

Expand Down
10 changes: 8 additions & 2 deletions packages/app/src/cli/models/project/active-config.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import {Project} from './project.js'
import {resolveDotEnv, resolveHiddenConfig} from './config-selection.js'
import {AppHiddenConfig} from '../app/app.js'
import {AppConfigurationAbortError} from '../app/error-parsing.js'
import {getAppConfigurationFileName} from '../app/config-file-naming.js'
import {getCachedAppInfo} from '../../services/local-storage.js'
import use from '../../services/app/config/use.js'
import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
import {DotEnvFile} from '@shopify/cli-kit/node/dot-env'
import {fileExistsSync} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputContent, outputToken} from '@shopify/cli-kit/node/output'

/** @public */
Expand Down Expand Up @@ -87,8 +87,14 @@ export async function selectActiveConfig(project: Project, userProvidedConfigNam
const configurationFileName = getAppConfigurationFileName(configName)
const file = project.appConfigByName(configurationFileName)
if (!file) {
throw new AbortError(
const malformedFile = project.malformedAppConfigByName(configurationFileName)
if (malformedFile) {
throw new AppConfigurationAbortError(malformedFile.message, malformedFile.path)
}

throw new AppConfigurationAbortError(
outputContent`Couldn't find ${configurationFileName} in ${outputToken.path(project.directory)}.`,
joinPath(project.directory, configurationFileName),
)
}

Expand Down
12 changes: 10 additions & 2 deletions packages/app/src/cli/models/project/project.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {Project} from './project.js'
import {AppConfigurationAbortError} from '../app/error-parsing.js'
import {describe, expect, test} from 'vitest'
import {inTemporaryDirectory, writeFile, mkdir} from '@shopify/cli-kit/node/fs'
import {joinPath, normalizePath} from '@shopify/cli-kit/node/path'
Expand Down Expand Up @@ -116,9 +117,16 @@ describe('Project', () => {
})
})

test('throws when no app config files found', async () => {
test('throws a structured configuration abort when no app config files are found', async () => {
await inTemporaryDirectory(async (dir) => {
await expect(Project.load(dir)).rejects.toThrow()
try {
await Project.load(dir)
expect.unreachable('Expected Project.load to throw')
} catch (error) {
if (!(error instanceof AppConfigurationAbortError)) throw error

expect(error.issues).toHaveLength(1)
}
})
})

Expand Down
71 changes: 60 additions & 11 deletions packages/app/src/cli/models/project/project.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {configurationFileNames} from '../../constants.js'
import {AppConfigurationAbortError} from '../app/error-parsing.js'
import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
import {readAndParseDotEnv, DotEnvFile} from '@shopify/cli-kit/node/dot-env'
import {fileExists, glob, findPathUp, readFile} from '@shopify/cli-kit/node/fs'
Expand All @@ -9,7 +10,6 @@ import {
usesWorkspaces as detectUsesWorkspaces,
} from '@shopify/cli-kit/node/node-package-manager'
import {joinPath, basename} from '@shopify/cli-kit/node/path'
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {JsonMapType} from '@shopify/cli-kit/node/toml'

Expand All @@ -21,6 +21,11 @@ const DEFAULT_EXTENSION_DIR = 'extensions/*'
const NODE_MODULES_EXCLUDE = '**/node_modules/**'
const DOTENV_GLOB = '.env*'

interface MalformedTomlFile {
path: string
message: string
}

/**
* A Project is the Shopify app as it exists on the filesystem.
*
Expand All @@ -45,9 +50,14 @@ export class Project {
const directory = await findProjectRoot(startDirectory)

// Discover all app config files
const appConfigFiles = await discoverAppConfigFiles(directory)
const {appConfigFiles, malformedAppConfigFiles} = await discoverAppConfigFiles(directory)
if (appConfigFiles.length === 0) {
throw new AbortError(`Could not find a Shopify app TOML file in ${directory}`)
const preferredMalformedConfig = getPreferredMalformedAppConfig(malformedAppConfigFiles)
if (preferredMalformedConfig) {
throw new AppConfigurationAbortError(preferredMalformedConfig.message, preferredMalformedConfig.path)
}

throw new AppConfigurationAbortError(`Could not find a Shopify app TOML file in ${directory}`, directory)
}

// Discover extension files from all app configs' extension_directories (union).
Expand Down Expand Up @@ -92,6 +102,7 @@ export class Project {
nodeDependencies,
usesWorkspaces,
appConfigFiles,
malformedAppConfigFiles,
extensionConfigFiles,
webConfigFiles,
dotenvFiles,
Expand All @@ -104,6 +115,7 @@ export class Project {
readonly nodeDependencies: Record<string, string>
readonly usesWorkspaces: boolean
readonly appConfigFiles: TomlFile[]
readonly malformedAppConfigFiles: MalformedTomlFile[]
readonly extensionConfigFiles: TomlFile[]
readonly webConfigFiles: TomlFile[]

Expand All @@ -119,6 +131,7 @@ export class Project {
nodeDependencies: Record<string, string>
usesWorkspaces: boolean
appConfigFiles: TomlFile[]
malformedAppConfigFiles: MalformedTomlFile[]
extensionConfigFiles: TomlFile[]
webConfigFiles: TomlFile[]
dotenvFiles: Map<string, DotEnvFile>
Expand All @@ -129,6 +142,7 @@ export class Project {
this.nodeDependencies = options.nodeDependencies
this.usesWorkspaces = options.usesWorkspaces
this.appConfigFiles = options.appConfigFiles
this.malformedAppConfigFiles = options.malformedAppConfigFiles
this.extensionConfigFiles = options.extensionConfigFiles
this.webConfigFiles = options.webConfigFiles
this.dotenvFiles = options.dotenvFiles
Expand All @@ -147,6 +161,11 @@ export class Project {
return this.appConfigFiles.find((file) => file.content.client_id === clientId)
}

/** Find a malformed app config file by filename. */
malformedAppConfigByName(fileName: string): MalformedTomlFile | undefined {
return this.malformedAppConfigFiles.find((file) => basename(file.path) === fileName)
}

/** The default app config (shopify.app.toml), if it exists */
get defaultAppConfig(): TomlFile | undefined {
return this.appConfigByName(configurationFileNames.app)
Expand All @@ -167,18 +186,22 @@ async function findProjectRoot(startDirectory: string): Promise<string> {
},
)
if (!found) {
throw new AbortError(
throw new AppConfigurationAbortError(
`Could not find a Shopify app configuration file. Looked in ${startDirectory} and parent directories.`,
startDirectory,
)
}
return found
}

async function discoverAppConfigFiles(directory: string): Promise<TomlFile[]> {
async function discoverAppConfigFiles(
directory: string,
): Promise<{appConfigFiles: TomlFile[]; malformedAppConfigFiles: MalformedTomlFile[]}> {
const pattern = joinPath(directory, APP_CONFIG_GLOB)
const paths = await glob(pattern)
const validPaths = paths.filter((filePath) => APP_CONFIG_REGEX.test(basename(filePath)))
return readTomlFilesSafe(validPaths)
const {files, malformedFiles} = await readTomlFiles(validPaths)
return {appConfigFiles: files, malformedAppConfigFiles: malformedFiles}
}

async function discoverExtensionFiles(directory: string, extensionDirectories?: string[]): Promise<TomlFile[]> {
Expand All @@ -202,19 +225,45 @@ async function discoverWebFiles(directory: string, webDirectories?: string[]): P
* This prevents a malformed inactive config or extension TOML
* from blocking the active config from loading.
*/
async function readTomlFilesSafe(paths: string[]): Promise<TomlFile[]> {
async function readTomlFiles(paths: string[]): Promise<{files: TomlFile[]; malformedFiles: MalformedTomlFile[]}> {
const results = await Promise.all(
paths.map(async (filePath) => {
try {
return await TomlFile.read(filePath)
return {file: await TomlFile.read(filePath)}
// eslint-disable-next-line no-catch-all/no-catch-all
} catch {
} catch (error) {
outputDebug(`Skipping malformed TOML file: ${filePath}`)
return undefined
return {
malformedFile: {
path: filePath,
message: error instanceof Error ? error.message : `Failed to parse ${filePath}`,
},
}
}
}),
)
return results.filter((file): file is TomlFile => file !== undefined)

const files: TomlFile[] = []
const malformedFiles: MalformedTomlFile[] = []

for (const result of results) {
if ('file' in result && result.file) {
files.push(result.file)
} else {
malformedFiles.push(result.malformedFile)
}
}

return {files, malformedFiles}
}

async function readTomlFilesSafe(paths: string[]): Promise<TomlFile[]> {
const {files} = await readTomlFiles(paths)
return files
}

function getPreferredMalformedAppConfig(malformedFiles: MalformedTomlFile[]): MalformedTomlFile | undefined {
return malformedFiles.find((file) => basename(file.path) === configurationFileNames.app) ?? malformedFiles[0]
}

/** Discover all .env* files in the project root */
Expand Down
Loading