From 81c77bf7db216b2e4f7dc21f337d67830bb67327 Mon Sep 17 00:00:00 2001 From: Alex Montague Date: Thu, 6 Aug 2026 12:06:55 -0400 Subject: [PATCH] Treat missing Theme Check configs as expected errors Translate an ENOENT for the explicitly selected Theme Check config into AbortError for check, --print, and --list. This gives developers actionable output and classifies the failure as expected instead of reporting it to Bugsnag and Observe. Continue re-raising missing extended configs, unrelated missing files, and non-ENOENT I/O failures so genuine CLI defects remain visible. Closes https://github.com/shop/issues/issues/71880 --- .changeset/calm-themes-check.md | 5 ++ .../src/cli/commands/theme/check.test.ts | 46 +++++++++++++++++- .../theme/src/cli/commands/theme/check.ts | 13 +++-- packages/theme/src/cli/services/check.test.ts | 47 +++++++++++++++++-- packages/theme/src/cli/services/check.ts | 36 ++++++++++++-- 5 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 .changeset/calm-themes-check.md diff --git a/.changeset/calm-themes-check.md b/.changeset/calm-themes-check.md new file mode 100644 index 00000000000..3c3c2222361 --- /dev/null +++ b/.changeset/calm-themes-check.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Report missing Theme Check config files as expected user errors. diff --git a/packages/theme/src/cli/commands/theme/check.test.ts b/packages/theme/src/cli/commands/theme/check.test.ts index fcfe5a1cd6a..b365ea02c15 100644 --- a/packages/theme/src/cli/commands/theme/check.test.ts +++ b/packages/theme/src/cli/commands/theme/check.test.ts @@ -1,7 +1,8 @@ -import Check from './check.js' +import Check, {runThemeCheck} from './check.js' import {describe, vi, expect, test, beforeEach} from 'vitest' import {Config} from '@oclif/core' import {themeCheckRun, Theme, Config as ThemeConfig, Offense} from '@shopify/theme-check-node' +import {AbortError} from '@shopify/cli-kit/node/error' vi.mock('@shopify/theme-check-node') const CommandConfig = new Config({root: __dirname}) @@ -78,5 +79,48 @@ describe('Check', () => { await run([`--config=${expectedConfig}`]) }) + + test('reports a missing explicitly configured file as an expected error', async () => { + const config = './.theme-check.yml' + const missingConfigError = Object.assign(new Error(`ENOENT: no such file or directory, open '${config}'`), { + code: 'ENOENT', + path: config, + }) + vi.mocked(themeCheckRun).mockRejectedValue(missingConfigError) + + const result = runThemeCheck(path, 'text', config) + + await expect(result).rejects.toBeInstanceOf(AbortError) + await expect(result).rejects.toThrowError(`Theme Check config file not found: ${config}`) + }) + + test('does not treat a missing extended config as a missing explicitly configured file', async () => { + const config = '/my-theme/.theme-check.yml' + const missingExtendedConfigError = Object.assign(new Error('ENOENT'), { + code: 'ENOENT', + path: '/my-theme/missing-extended.yml', + }) + vi.mocked(themeCheckRun).mockRejectedValue(missingExtendedConfigError) + + await expect(runThemeCheck(path, 'text', config)).rejects.toBe(missingExtendedConfigError) + }) + + test('does not treat other config failures as a missing explicitly configured file', async () => { + const config = '/my-theme/.theme-check.yml' + const permissionError = Object.assign(new Error('EACCES'), {code: 'EACCES', path: config}) + vi.mocked(themeCheckRun).mockRejectedValue(permissionError) + + await expect(runThemeCheck(path, 'text', config)).rejects.toBe(permissionError) + }) + + test('does not treat missing files as config errors when no config was explicitly provided', async () => { + const missingThemeFileError = Object.assign(new Error('ENOENT'), { + code: 'ENOENT', + path: '/my-theme/templates/index.liquid', + }) + vi.mocked(themeCheckRun).mockRejectedValue(missingThemeFileError) + + await expect(runThemeCheck(path, 'text')).rejects.toBe(missingThemeFileError) + }) }) }) diff --git a/packages/theme/src/cli/commands/theme/check.ts b/packages/theme/src/cli/commands/theme/check.ts index a7e0c7c4480..8a98b38ef16 100644 --- a/packages/theme/src/cli/commands/theme/check.ts +++ b/packages/theme/src/cli/commands/theme/check.ts @@ -10,6 +10,7 @@ import { sortOffenses, isExtendedWriteStream, handleExit, + withThemeCheckConfigErrorHandling, type FailLevel, } from '../../services/check.js' import {themeFlags} from '../../flags.js' @@ -151,11 +152,13 @@ export default class Check extends ThemeCommand { } export async function runThemeCheck(path: string, outputFormat: string, config?: string, environment?: string) { - const {offenses, theme} = await themeCheckRun(path, config, (message) => { - if (process.env.SHOPIFY_TMP_FLAG_DEBUG) { - outputDebug(message) - } - }) + const {offenses, theme} = await withThemeCheckConfigErrorHandling(config, () => + themeCheckRun(path, config, (message) => { + if (process.env.SHOPIFY_TMP_FLAG_DEBUG) { + outputDebug(message) + } + }), + ) const offensesByFile = sortOffenses(offenses) diff --git a/packages/theme/src/cli/services/check.test.ts b/packages/theme/src/cli/services/check.test.ts index 4a430ca582d..2654de705a0 100644 --- a/packages/theme/src/cli/services/check.test.ts +++ b/packages/theme/src/cli/services/check.test.ts @@ -4,10 +4,13 @@ import { formatSummary, handleExit, initConfig, + outputActiveChecks, + outputActiveConfig, renderOffensesText, sortOffenses, } from './check.js' import {fileExists, writeFile} from '@shopify/cli-kit/node/fs' +import {AbortError} from '@shopify/cli-kit/node/error' import {outputInfo, outputSuccess} from '@shopify/cli-kit/node/output' import {renderInfo} from '@shopify/cli-kit/node/ui' import { @@ -25,10 +28,14 @@ vi.mock('@shopify/cli-kit/node/fs', async () => ({ writeFile: vi.fn(), })) -vi.mock('@shopify/cli-kit/node/output', async () => ({ - outputInfo: vi.fn(), - outputSuccess: vi.fn(), -})) +vi.mock('@shopify/cli-kit/node/output', async () => { + const actual = await vi.importActual('@shopify/cli-kit/node/output') + return { + ...actual, + outputInfo: vi.fn(), + outputSuccess: vi.fn(), + } +}) vi.mock('@shopify/theme-check-node', async () => { const actual: any = await vi.importActual('@shopify/theme-check-node') @@ -427,3 +434,35 @@ describe('initConfig', () => { expect(outputSuccess).toHaveBeenCalledWith('Created .theme-check.yml at /path/to/root') }) }) + +describe('outputActiveConfig', () => { + test('reports a missing explicitly configured file as an expected error', async () => { + const configPath = './.theme-check.yml' + const missingConfigError = Object.assign(new Error(`ENOENT: no such file or directory, open '${configPath}'`), { + code: 'ENOENT', + path: configPath, + }) + vi.mocked(loadConfig).mockRejectedValue(missingConfigError) + + const result = outputActiveConfig('/my-theme', configPath) + + await expect(result).rejects.toBeInstanceOf(AbortError) + await expect(result).rejects.toThrowError(`Theme Check config file not found: ${configPath}`) + }) +}) + +describe('outputActiveChecks', () => { + test('reports a missing explicitly configured file as an expected error', async () => { + const configPath = './.theme-check.yml' + const missingConfigError = Object.assign(new Error(`ENOENT: no such file or directory, open '${configPath}'`), { + code: 'ENOENT', + path: configPath, + }) + vi.mocked(loadConfig).mockRejectedValue(missingConfigError) + + const result = outputActiveChecks('/my-theme', configPath) + + await expect(result).rejects.toBeInstanceOf(AbortError) + await expect(result).rejects.toThrowError(`Theme Check config file not found: ${configPath}`) + }) +}) diff --git a/packages/theme/src/cli/services/check.ts b/packages/theme/src/cli/services/check.ts index 44952c63aa3..af3e994c0e1 100644 --- a/packages/theme/src/cli/services/check.ts +++ b/packages/theme/src/cli/services/check.ts @@ -1,8 +1,9 @@ import {fileExists, writeFile} from '@shopify/cli-kit/node/fs' import {outputResult, outputInfo, outputSuccess} from '@shopify/cli-kit/node/output' -import {joinPath} from '@shopify/cli-kit/node/path' +import {joinPath, resolvePath} from '@shopify/cli-kit/node/path' import {renderInfo} from '@shopify/cli-kit/node/ui' import {uniq} from '@shopify/cli-kit/common/array' +import {AbortError} from '@shopify/cli-kit/node/error' import { Severity, applyFixToString, @@ -46,6 +47,31 @@ type SeverityCounts = Partial<{ export type FailLevel = 'error' | 'suggestion' | 'style' | 'warning' | 'info' | 'crash' +function isMissingThemeCheckConfigFile(error: unknown, configPath: string) { + return ( + error instanceof Error && + 'code' in error && + error.code === 'ENOENT' && + 'path' in error && + typeof error.path === 'string' && + resolvePath(error.path) === resolvePath(configPath) + ) +} + +export async function withThemeCheckConfigErrorHandling( + configPath: string | undefined, + operation: () => Promise, +): Promise { + try { + return await operation() + } catch (error) { + if (configPath !== undefined && isMissingThemeCheckConfigFile(error, configPath)) { + throw new AbortError(`Theme Check config file not found: ${configPath}`) + } + throw error + } +} + function failLevelToSeverity(failLevel: FailLevel): Severity | undefined { switch (failLevel) { case 'error': @@ -308,7 +334,9 @@ export async function performAutoFixes(sourceCodes: Theme, offenses: Offense[]) } export async function outputActiveConfig(themeRoot: string, configPath?: string, environment?: string) { - const {ignore, settings, rootUri} = await loadConfig(configPath, themeRoot) + const {ignore, settings, rootUri} = await withThemeCheckConfigErrorHandling(configPath, () => + loadConfig(configPath, themeRoot), + ) const config = { // loadConfig flattens all configs, it doesn't extend anything @@ -328,7 +356,9 @@ export async function outputActiveConfig(themeRoot: string, configPath?: string, } export async function outputActiveChecks(root: string, configPath?: string, environment?: string) { - const {settings, ignore, checks} = await loadConfig(configPath, root) + const {settings, ignore, checks} = await withThemeCheckConfigErrorHandling(configPath, () => + loadConfig(configPath, root), + ) // Depending on how the configs were merged during loadConfig, there may be // duplicate patterns to ignore. We can clean them before outputting. const ignorePatterns = uniq(ignore ?? [])