From f837dfd149e866d11e44a88f9546b2b5e6951a61 Mon Sep 17 00:00:00 2001 From: Philippe Collin Date: Fri, 19 Jun 2026 17:35:08 -0400 Subject: [PATCH] Add typed ThemeCheckConfigError to theme-check-node loadConfig now wraps any failure while reading, parsing, or validating a Theme Check configuration in a ThemeCheckConfigError. The error attaches the configPath and preserves the underlying error (filesystem error, YAML parse error, validation error, etc.) on `cause`, and surfaces the original message inline. This lets callers recognize a configuration problem with a single `instanceof` check (or the stable `code`) instead of matching error-message text, while the original error's details remain available on `cause`. Assisted-By: devx/e547eed5-2a6b-4b20-972a-c6fe290bdf0c --- .changeset/theme-check-config-error.md | 12 +++++ .../theme-check-node/src/config/errors.ts | 48 +++++++++++++++++++ packages/theme-check-node/src/config/index.ts | 1 + .../src/config/load-config.spec.ts | 29 +++++++++++ .../src/config/load-config.ts | 33 ++++++++----- packages/theme-check-node/src/index.ts | 1 + 6 files changed, 112 insertions(+), 12 deletions(-) create mode 100644 .changeset/theme-check-config-error.md create mode 100644 packages/theme-check-node/src/config/errors.ts diff --git a/.changeset/theme-check-config-error.md b/.changeset/theme-check-config-error.md new file mode 100644 index 000000000..5ec2dd2b1 --- /dev/null +++ b/.changeset/theme-check-config-error.md @@ -0,0 +1,12 @@ +--- +'@shopify/theme-check-node': minor +--- + +Add `ThemeCheckConfigError` so configuration-resolution failures are catchable by type. + +`loadConfig` now wraps any failure while reading, parsing, or validating a Theme Check +configuration in a `ThemeCheckConfigError`. The error attaches the `configPath` and preserves +the underlying error (filesystem error, YAML parse error, validation error, etc.) on `cause`, +so callers can recognize a configuration problem with a single `instanceof` check — or the +stable `code` (`'THEME_CHECK_CONFIG_ERROR'`) — instead of matching error messages, while the +original error's details remain available on `cause`. diff --git a/packages/theme-check-node/src/config/errors.ts b/packages/theme-check-node/src/config/errors.ts new file mode 100644 index 000000000..7b4e8d8d9 --- /dev/null +++ b/packages/theme-check-node/src/config/errors.ts @@ -0,0 +1,48 @@ +/** + * Thrown by `loadConfig` when a Theme Check configuration cannot be resolved. + * + * Any failure while reading, parsing, or validating the configuration is + * surfaced as a `ThemeCheckConfigError`, so callers can recognize a + * configuration problem with a single `instanceof` check (or the stable + * `code`) instead of matching error messages. The underlying error — a + * filesystem error, a YAML parse error, a validation error, etc. — is + * preserved on `cause`, so its `code`, line/column, and message remain + * available to callers that want the detail. + */ +export class ThemeCheckConfigError extends Error { + /** + * Stable discriminator for recognizing this error across module/realm + * boundaries where `instanceof` can be unreliable (duplicate package copies). + */ + readonly code = 'THEME_CHECK_CONFIG_ERROR'; + /** The config path or modern identifier that failed to load, when known. */ + readonly configPath?: string; + + constructor(message: string, options: { cause?: unknown; configPath?: string } = {}) { + super(message, { cause: options.cause }); + this.name = 'ThemeCheckConfigError'; + this.configPath = options.configPath; + } +} + +/** + * Wraps any error thrown while resolving a configuration in a + * `ThemeCheckConfigError`, attaching the config path and preserving the + * original error on `cause`. The original message is surfaced inline so the + * default output stays useful without unwrapping `cause`. + */ +export function toThemeCheckConfigError( + error: unknown, + configPath?: string, +): ThemeCheckConfigError { + if (error instanceof ThemeCheckConfigError) return error; + + const errno = error instanceof Error ? (error as NodeJS.ErrnoException) : undefined; + const location = errno?.path ?? configPath; + const detail = errno?.message ?? String(error); + + return new ThemeCheckConfigError( + `Failed to load Theme Check configuration${location ? ` from ${location}` : ''}: ${detail}`, + { cause: error, configPath: location }, + ); +} diff --git a/packages/theme-check-node/src/config/index.ts b/packages/theme-check-node/src/config/index.ts index 5115b430c..e88848400 100644 --- a/packages/theme-check-node/src/config/index.ts +++ b/packages/theme-check-node/src/config/index.ts @@ -1,2 +1,3 @@ export { loadConfig } from './load-config'; export { findConfigPath } from './find-config-path'; +export { ThemeCheckConfigError } from './errors'; diff --git a/packages/theme-check-node/src/config/load-config.spec.ts b/packages/theme-check-node/src/config/load-config.spec.ts index bcd473bbd..9532c1f56 100644 --- a/packages/theme-check-node/src/config/load-config.spec.ts +++ b/packages/theme-check-node/src/config/load-config.spec.ts @@ -2,6 +2,7 @@ import fs from 'node:fs/promises'; import path from 'node:path'; import { describe, it, expect, afterEach, beforeEach, assert, vi } from 'vitest'; import { loadConfig } from './load-config'; +import { ThemeCheckConfigError } from './errors'; import { allChecks, CheckDefinition, @@ -252,6 +253,34 @@ SyntaxError: assert(config.rootUri.startsWith('file://')); }); + describe('when the configuration cannot be resolved', () => { + it('wraps a missing config file in a ThemeCheckConfigError, preserving the original error', async () => { + const missingPath = path.join(tempDir, 'does-not-exist.yml'); + const error = await loadConfig(missingPath, tempDir).catch((e) => e); + expect(error).to.be.an.instanceof(ThemeCheckConfigError); + expect(error.code).to.equal('THEME_CHECK_CONFIG_ERROR'); + expect(error.configPath).to.equal(missingPath); + // The original error is preserved for callers that want the detail. + expect((error.cause as NodeJS.ErrnoException)?.code).to.equal('ENOENT'); + }); + + it('wraps a config path that is a directory in a ThemeCheckConfigError', async () => { + const error = await loadConfig(tempDir, tempDir).catch((e) => e); + expect(error).to.be.an.instanceof(ThemeCheckConfigError); + expect((error.cause as NodeJS.ErrnoException)?.code).to.equal('EISDIR'); + }); + + it('wraps an invalid check setting shape (config description parse error) in a ThemeCheckConfigError', async () => { + const configPath = await createMockConfigFile( + tempDir, + `extends: nothing\nUnusedAssign: true`, + ); + const error = await loadConfig(configPath, tempDir).catch((e) => e); + expect(error).to.be.an.instanceof(ThemeCheckConfigError); + expect(error.cause).to.be.an.instanceof(Error); + }); + }); + function check(code: string) { return allChecks.find(isCheck(code)); } diff --git a/packages/theme-check-node/src/config/load-config.ts b/packages/theme-check-node/src/config/load-config.ts index 42361562f..a965fa88c 100644 --- a/packages/theme-check-node/src/config/load-config.ts +++ b/packages/theme-check-node/src/config/load-config.ts @@ -4,6 +4,7 @@ import { loadConfigDescription } from './load-config-description'; import { resolveConfig } from './resolve'; import { ModernIdentifier } from './types'; import { validateConfig } from './validation'; +import { toThemeCheckConfigError } from './errors'; import fs from 'fs/promises'; /** @@ -23,19 +24,27 @@ export async function loadConfig( root: AbsolutePath, ): Promise { if (!root) throw new Error('loadConfig cannot be called without a root argument'); - let defaultChecks = 'theme-check:recommended'; - if (!configPath) { - const files = await fs.readdir(root); - // *.extension.toml implies that we're already in the appropriate extensions - // directory. *.app.toml implies that we're inside the root of an app. - if (files.some((file) => file.endsWith('.extension.toml') || file.endsWith('.app.toml'))) { - defaultChecks = 'theme-check:theme-app-extension'; + // Config resolution is a single, well-defined operation: any failure while + // reading, parsing, or validating the configuration is a configuration + // problem, so we surface it as a typed `ThemeCheckConfigError`. + try { + let defaultChecks = 'theme-check:recommended'; + + if (!configPath) { + const files = await fs.readdir(root); + // *.extension.toml implies that we're already in the appropriate extensions + // directory. *.app.toml implies that we're inside the root of an app. + if (files.some((file) => file.endsWith('.extension.toml') || file.endsWith('.app.toml'))) { + defaultChecks = 'theme-check:theme-app-extension'; + } } - } - const configDescription = await resolveConfig(configPath ?? defaultChecks, true); - const config = await loadConfigDescription(configDescription, root); - validateConfig(config); - return config; + const configDescription = await resolveConfig(configPath ?? defaultChecks, true); + const config = await loadConfigDescription(configDescription, root); + validateConfig(config); + return config; + } catch (error) { + throw toThemeCheckConfigError(error, typeof configPath === 'string' ? configPath : undefined); + } } diff --git a/packages/theme-check-node/src/index.ts b/packages/theme-check-node/src/index.ts index 22cb13827..39cfe212a 100644 --- a/packages/theme-check-node/src/index.ts +++ b/packages/theme-check-node/src/index.ts @@ -41,6 +41,7 @@ const asyncGlob = promisify(glob); export * from '@shopify/theme-check-common'; export * from './config/types'; export { NodeFileSystem }; +export { ThemeCheckConfigError } from './config'; export const loadConfig: typeof resolveConfig = async (configPath, root) => { configPath ??= await findConfigPath(root);