Surface malformed discovered configs in validate json

Author: dmerand

packages/app/src/cli/models/app/loader.test.ts

@@ -29,6 +29,7 @@ import {installNodeModules, PackageJson} from '@shopify/cli-kit/node/node-packag
 import {inTemporaryDirectory, moveFile, mkdir, mkTmpDir, rmdir, writeFile} from '@shopify/cli-kit/node/fs'
 import {joinPath, dirname, cwd, normalizePath} from '@shopify/cli-kit/node/path'
 import {platformAndArch} from '@shopify/cli-kit/node/os'
+import {stringifyMessage} from '@shopify/cli-kit/node/output'
 import {zod} from '@shopify/cli-kit/node/schema'
 import colors from '@shopify/cli-kit/node/colors'
 import {showMultipleCLIWarningIfNeeded} from '@shopify/cli-kit/node/multiple-installation-warning'
@@ -608,6 +609,27 @@ describe('load', () => {
     expect(app.errors.isEmpty()).toBe(false)
   })
 
+  test('collects a malformed discovered extension TOML as an app error', async () => {
+    // Given
+    await writeConfig(appConfiguration)
+    const configurationPath = blockConfigurationPath({name: 'my-extension'})
+    await writeBlockConfig({
+      blockConfiguration: '{{broken toml',
+      name: 'my-extension',
+    })
+
+    // When
+    const app = await loadTestingApp()
+
+    // Then
+    expect(getRealExtensions(app)).toHaveLength(0)
+    const error = app.errors.getError(configurationPath)
+    expect(error).toBeDefined()
+    expect(stringifyMessage(error!)).toContain(configurationPath)
+    expect(stringifyMessage(error!)).toContain('Unknown character')
+    expect(app.errors.getIssues(configurationPath)).toEqual([])
+  })
+
   test('collects an error if the extension configuration is missing both extensions and type', async () => {
     // Given
     await writeConfig(appConfiguration, {
@@ -632,6 +654,24 @@ describe('load', () => {
     expect(app.errors.isEmpty()).toBe(false)
   })
 
+  test('collects a malformed discovered web TOML as an app error', async () => {
+    // Given
+    const {webDirectory} = await writeConfig(appConfiguration)
+    const configurationPath = joinPath(webDirectory, blocks.web.configurationName)
+    await writeFile(configurationPath, '{{broken toml')
+
+    // When
+    const app = await loadTestingApp()
+
+    // Then
+    expect(app.webs).toHaveLength(0)
+    const error = app.errors.getError(configurationPath)
+    expect(error).toBeDefined()
+    expect(stringifyMessage(error!)).toContain(configurationPath)
+    expect(stringifyMessage(error!)).toContain('Unknown character')
+    expect(app.errors.getIssues(configurationPath)).toEqual([])
+  })
+
   test('loads the app with web blocks', async () => {
     // Given
     const {webDirectory} = await writeConfig(appConfiguration)

packages/app/src/cli/models/app/loader.ts

@@ -40,6 +40,8 @@ import {
   resolveDotEnv,
   resolveHiddenConfig,
   extensionFilesForConfig,
+  malformedExtensionFilesForConfig,
+  malformedWebFilesForConfig,
   webFilesForConfig,
 } from '../project/config-selection.js'
 import {showMultipleCLIWarningIfNeeded} from '@shopify/cli-kit/node/multiple-installation-warning'
@@ -538,6 +540,14 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
   async loadWebs(appDirectory: string, webDirectories?: string[]): Promise<{webs: Web[]; usedCustomLayout: boolean}> {
     const activeConfig = this.activeConfigFile
     const webFiles = activeConfig ? webFilesForConfig(this.project, activeConfig) : this.project.webConfigFiles
+    const malformedWebFiles = activeConfig
+      ? malformedWebFilesForConfig(this.project, activeConfig)
+      : this.project.malformedWebConfigFiles
+
+    for (const malformedWebFile of malformedWebFiles) {
+      this.errors.addError(malformedWebFile.path, malformedWebFile.message)
+    }
+
     const webTomlPaths = webFiles.map((file) => file.path)
     const webResults = await Promise.all(
       webFiles.map(async (webFile) => {
@@ -692,6 +702,13 @@ class AppLoader<TConfig extends CurrentAppConfiguration, TModuleSpec extends Ext
     const extensionFiles = activeConfig
       ? extensionFilesForConfig(this.project, activeConfig)
       : this.project.extensionConfigFiles
+    const malformedExtensionFiles = activeConfig
+      ? malformedExtensionFilesForConfig(this.project, activeConfig)
+      : this.project.malformedExtensionConfigFiles
+
+    for (const malformedExtensionFile of malformedExtensionFiles) {
+      this.errors.addError(malformedExtensionFile.path, malformedExtensionFile.message)
+    }
 
     return extensionFiles.map(async (extensionFile) => {
       const configurationPath = extensionFile.path

packages/app/src/cli/models/app/local-config-error.ts

@@ -1,6 +1,6 @@
+import {toRootValidationIssue, type AppValidationIssue} from './error-parsing.js'
 import {AbortError} from '@shopify/cli-kit/node/error'
-import type {OutputMessage} from '@shopify/cli-kit/node/output'
-import type {AppValidationIssue} from './error-parsing.js'
+import {stringifyMessage, type OutputMessage} from '@shopify/cli-kit/node/output'
 
 /**
  * Structured local configuration failure shared by lower layers.
@@ -10,11 +10,15 @@ import type {AppValidationIssue} from './error-parsing.js'
  * results, or other higher-level flows.
  */
 export class LocalConfigError extends AbortError {
+  public readonly issues: AppValidationIssue[]
+
   constructor(
     message: OutputMessage,
     public readonly configurationPath: string,
-    public readonly issues: AppValidationIssue[] = [],
+    issues: AppValidationIssue[] = [],
   ) {
     super(message)
+    this.issues =
+      issues.length > 0 ? issues : [toRootValidationIssue(configurationPath, stringifyMessage(message).trim())]
   }
 }

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 {LocalConfigError} from '../app/local-config-error.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,66 @@ describe('selectActiveConfig', () => {
     })
   })
 
-  test('throws when requested config does not exist', async () => {
+  test('throws LocalConfigError 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.fail('Expected selectActiveConfig to throw')
+      } catch (error) {
+        if (!(error instanceof LocalConfigError)) throw error
+        expect(error.configurationPath).toBe(joinPath(dir, 'shopify.app.nonexistent.toml'))
+        expect(error.message).toBe(`Couldn't find shopify.app.nonexistent.toml in ${dir}.`)
+        expect(error.issues[0]).toMatchObject({
+          filePath: joinPath(dir, 'shopify.app.nonexistent.toml'),
+          path: [],
+          pathString: 'root',
+        })
+      }
     })
   })
 
-  test('throws when the only app config is malformed (no valid configs to fall back to)', async () => {
+  test('throws a structured parse error 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
+      // The only config is broken TOML — Project.load should surface the parse error.
       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.fail('Expected Project.load to throw')
+      } catch (error) {
+        if (!(error instanceof LocalConfigError)) throw error
+        expect(error.message).toMatch(/Unknown character/)
+        expect(error.issues[0]).toMatchObject({
+          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.fail('Expected selectActiveConfig to throw')
+      } catch (error) {
+        if (!(error instanceof LocalConfigError)) throw error
+        expect(error.message).toMatch(/Unknown character/)
+        expect(error.issues[0]).toMatchObject({
+          filePath: joinPath(dir, 'shopify.app.broken.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 {LocalConfigError} from '../app/local-config-error.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 LocalConfigError(malformedFile.message, malformedFile.path)
+    }
+
+    throw new LocalConfigError(
       outputContent`Couldn't find ${configurationFileName} in ${outputToken.path(project.directory)}.`,
+      joinPath(project.directory, configurationFileName),
     )
   }
 

packages/app/src/cli/models/project/config-selection.test.ts

@@ -1,4 +1,11 @@
-import {resolveDotEnv, resolveHiddenConfig, extensionFilesForConfig, webFilesForConfig} from './config-selection.js'
+import {
+  resolveDotEnv,
+  resolveHiddenConfig,
+  extensionFilesForConfig,
+  malformedExtensionFilesForConfig,
+  malformedWebFilesForConfig,
+  webFilesForConfig,
+} from './config-selection.js'
 import {Project} from './project.js'
 import {describe, expect, test} from 'vitest'
 import {inTemporaryDirectory, writeFile, mkdir} from '@shopify/cli-kit/node/fs'
@@ -230,6 +237,33 @@ describe('extensionFilesForConfig', () => {
       expect(stagingExts[0]!.content.name).toBe('func2')
     })
   })
+
+  test('filters malformed extension files to the active config extension_directories', async () => {
+    await inTemporaryDirectory(async (dir) => {
+      await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "default"\nextension_directories = ["ext-a/*"]')
+      await writeFile(
+        joinPath(dir, 'shopify.app.staging.toml'),
+        'client_id = "staging"\nextension_directories = ["ext-b/*"]',
+      )
+
+      await mkdir(joinPath(dir, 'ext-a', 'broken-default'))
+      await writeFile(joinPath(dir, 'ext-a', 'broken-default', 'shopify.extension.toml'), '{{broken toml')
+      await mkdir(joinPath(dir, 'ext-b', 'broken-staging'))
+      await writeFile(joinPath(dir, 'ext-b', 'broken-staging', 'shopify.extension.toml'), '{{broken toml')
+
+      const project = await Project.load(dir)
+      const defaultConfig = project.appConfigByName('shopify.app.toml')!
+      const stagingConfig = project.appConfigByName('shopify.app.staging.toml')!
+
+      const defaultMalformed = malformedExtensionFilesForConfig(project, defaultConfig)
+      expect(defaultMalformed).toHaveLength(1)
+      expect(defaultMalformed[0]!.path).toContain('ext-a/broken-default/shopify.extension.toml')
+
+      const stagingMalformed = malformedExtensionFilesForConfig(project, stagingConfig)
+      expect(stagingMalformed).toHaveLength(1)
+      expect(stagingMalformed[0]!.path).toContain('ext-b/broken-staging/shopify.extension.toml')
+    })
+  })
 })
 
 describe('webFilesForConfig', () => {
@@ -287,4 +321,28 @@ describe('webFilesForConfig', () => {
       expect(webFiles[0]!.content.name).toBe('web-a')
     })
   })
+
+  test('filters malformed web files to the active config web_directories', async () => {
+    await inTemporaryDirectory(async (dir) => {
+      await writeFile(joinPath(dir, 'shopify.app.toml'), 'client_id = "default"\nweb_directories = ["web-a"]')
+      await writeFile(joinPath(dir, 'shopify.app.staging.toml'), 'client_id = "staging"\nweb_directories = ["web-b"]')
+
+      await mkdir(joinPath(dir, 'web-a'))
+      await writeFile(joinPath(dir, 'web-a', 'shopify.web.toml'), '{{broken toml')
+      await mkdir(joinPath(dir, 'web-b'))
+      await writeFile(joinPath(dir, 'web-b', 'shopify.web.toml'), '{{broken toml')
+
+      const project = await Project.load(dir)
+      const defaultConfig = project.appConfigByName('shopify.app.toml')!
+      const stagingConfig = project.appConfigByName('shopify.app.staging.toml')!
+
+      const defaultMalformed = malformedWebFilesForConfig(project, defaultConfig)
+      expect(defaultMalformed).toHaveLength(1)
+      expect(defaultMalformed[0]!.path).toContain('web-a/shopify.web.toml')
+
+      const stagingMalformed = malformedWebFilesForConfig(project, stagingConfig)
+      expect(stagingMalformed).toHaveLength(1)
+      expect(stagingMalformed[0]!.path).toContain('web-b/shopify.web.toml')
+    })
+  })
 })

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

@@ -1,4 +1,4 @@
-import {Project} from './project.js'
+import {Project, type MalformedTomlFile} from './project.js'
 import {AppHiddenConfig} from '../app/app.js'
 import {getAppConfigurationShorthand} from '../app/loader.js'
 import {dotEnvFileNames} from '../../constants.js'
@@ -76,19 +76,23 @@ export async function resolveHiddenConfig(project: Project, clientId: string | u
  * @public
  */
 export function extensionFilesForConfig(project: Project, activeConfig: TomlFile): TomlFile[] {
-  const configDirs = activeConfig.content.extension_directories
-  const dirs = Array.isArray(configDirs) && configDirs.length > 0 ? (configDirs as string[]) : ['extensions/*']
-
-  // Replicate the same glob patterns used by discoverExtensionFiles in project.ts:
-  // each directory pattern becomes "<dir>/*.extension.toml"
-  const globPatterns = dirs.map((dir) => `${dir}/*.extension.toml`)
+  const globPatterns = extensionGlobPatternsForConfig(activeConfig)
 
   return project.extensionConfigFiles.filter((file) => {
     const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/')
     return globPatterns.some((pattern) => matchGlob(relPath, pattern))
   })
 }
 
+export function malformedExtensionFilesForConfig(project: Project, activeConfig: TomlFile): MalformedTomlFile[] {
+  const globPatterns = extensionGlobPatternsForConfig(activeConfig)
+
+  return project.malformedExtensionConfigFiles.filter((file) => {
+    const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/')
+    return globPatterns.some((pattern) => matchGlob(relPath, pattern))
+  })
+}
+
 /**
  * Filter web config files to those belonging to the active config's
  * web_directories. If not specified, returns all web files.
@@ -98,17 +102,35 @@ export function extensionFilesForConfig(project: Project, activeConfig: TomlFile
  * @public
  */
 export function webFilesForConfig(project: Project, activeConfig: TomlFile): TomlFile[] {
-  const configDirs = activeConfig.content.web_directories
-  if (!Array.isArray(configDirs) || configDirs.length === 0) {
-    return project.webConfigFiles
-  }
-
-  // Replicate the same glob patterns used by discoverWebFiles in project.ts:
-  // each directory pattern becomes "<dir>/shopify.web.toml"
-  const globPatterns = (configDirs as string[]).map((dir) => `${dir}/shopify.web.toml`)
+  const globPatterns = webGlobPatternsForConfig(activeConfig)
 
   return project.webConfigFiles.filter((file) => {
     const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/')
     return globPatterns.some((pattern) => matchGlob(relPath, pattern))
   })
 }
+
+export function malformedWebFilesForConfig(project: Project, activeConfig: TomlFile): MalformedTomlFile[] {
+  const globPatterns = webGlobPatternsForConfig(activeConfig)
+
+  return project.malformedWebConfigFiles.filter((file) => {
+    const relPath = relativePath(project.directory, file.path).replace(/\\/g, '/')
+    return globPatterns.some((pattern) => matchGlob(relPath, pattern))
+  })
+}
+
+function extensionGlobPatternsForConfig(activeConfig: TomlFile): string[] {
+  const configDirs = activeConfig.content.extension_directories
+  const dirs = Array.isArray(configDirs) && configDirs.length > 0 ? (configDirs as string[]) : ['extensions/*']
+
+  return dirs.map((dir) => `${dir}/*.extension.toml`)
+}
+
+function webGlobPatternsForConfig(activeConfig: TomlFile): string[] {
+  const configDirs = activeConfig.content.web_directories
+  if (!Array.isArray(configDirs) || configDirs.length === 0) {
+    return ['**/shopify.web.toml']
+  }
+
+  return (configDirs as string[]).map((dir) => `${dir}/shopify.web.toml`)
+}