Align project config failures with validate json

Author: dmerand

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',
+            },
+          ],
+        })
+      }
     })
   })
 

packages/app/src/cli/models/project/active-config.ts

@@ -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 */
@@ -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),
     )
   }
 

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)
+        }
       })
     })
 

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<string, string>
   readonly usesWorkspaces: boolean
   readonly appConfigFiles: TomlFile[]
+  readonly malformedAppConfigFiles: MalformedTomlFile[]
   readonly extensionConfigFiles: TomlFile[]
   readonly webConfigFiles: TomlFile[]
 
@@ -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>
@@ -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<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[]> {
@@ -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 */