diff --git a/packages/app/src/cli/models/project/active-config.test.ts b/packages/app/src/cli/models/project/active-config.test.ts index c50a64d257..06ac4cbd3a 100644 --- a/packages/app/src/cli/models/project/active-config.test.ts +++ b/packages/app/src/cli/models/project/active-config.test.ts @@ -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' @@ -155,34 +156,103 @@ describe('selectActiveConfig', () => { }) }) - 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({ + 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', + }, + ], + }) + } }) }) diff --git a/packages/app/src/cli/models/project/active-config.ts b/packages/app/src/cli/models/project/active-config.ts index dbf8dd86ee..da31af3474 100644 --- a/packages/app/src/cli/models/project/active-config.ts +++ b/packages/app/src/cli/models/project/active-config.ts @@ -1,6 +1,7 @@ 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' @@ -8,7 +9,6 @@ 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 */ @@ -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), ) } diff --git a/packages/app/src/cli/models/project/project.test.ts b/packages/app/src/cli/models/project/project.test.ts index 77b0c5a864..5632400b3c 100644 --- a/packages/app/src/cli/models/project/project.test.ts +++ b/packages/app/src/cli/models/project/project.test.ts @@ -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' @@ -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) + } }) }) diff --git a/packages/app/src/cli/models/project/project.ts b/packages/app/src/cli/models/project/project.ts index 7ffbba3bd5..dfc08a8eb6 100644 --- a/packages/app/src/cli/models/project/project.ts +++ b/packages/app/src/cli/models/project/project.ts @@ -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' @@ -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' @@ -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. * @@ -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). @@ -92,6 +102,7 @@ export class Project { nodeDependencies, usesWorkspaces, appConfigFiles, + malformedAppConfigFiles, extensionConfigFiles, webConfigFiles, dotenvFiles, @@ -104,6 +115,7 @@ export class Project { readonly nodeDependencies: Record readonly usesWorkspaces: boolean readonly appConfigFiles: TomlFile[] + readonly malformedAppConfigFiles: MalformedTomlFile[] readonly extensionConfigFiles: TomlFile[] readonly webConfigFiles: TomlFile[] @@ -119,6 +131,7 @@ export class Project { nodeDependencies: Record usesWorkspaces: boolean appConfigFiles: TomlFile[] + malformedAppConfigFiles: MalformedTomlFile[] extensionConfigFiles: TomlFile[] webConfigFiles: TomlFile[] dotenvFiles: Map @@ -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 @@ -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) @@ -167,18 +186,22 @@ async function findProjectRoot(startDirectory: string): Promise { }, ) 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 { +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 { @@ -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 { +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 { + 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 */