diff --git a/.changeset/surface-config-source-errors.md b/.changeset/surface-config-source-errors.md new file mode 100644 index 00000000..e0786f24 --- /dev/null +++ b/.changeset/surface-config-source-errors.md @@ -0,0 +1,5 @@ +--- +'@salesforce/b2c-tooling-sdk': minor +--- + +Surface config source errors as warnings. When a config source (like dw.json) has malformed content, the error is now displayed as a warning instead of being silently ignored. diff --git a/docs/guide/extending.md b/docs/guide/extending.md index f4b9db7c..21439b09 100644 --- a/docs/guide/extending.md +++ b/docs/guide/extending.md @@ -205,6 +205,31 @@ export class MyCustomSource implements ConfigSource { } ``` +### Error Handling + +If your `ConfigSource` encounters an error (e.g., malformed config file, network failure), you can: + +1. **Return `undefined`** - Source is silently skipped (use for "source not available") +2. **Throw an exception** - Error is surfaced as a warning to the user + +```typescript +load(options: ResolveConfigOptions): ConfigLoadResult | undefined { + const configPath = '/path/to/config'; + + if (!fs.existsSync(configPath)) { + return undefined; // Source not available - skip silently + } + + // Let JSON.parse errors propagate - they'll become warnings + const content = fs.readFileSync(configPath, 'utf8'); + const config = JSON.parse(content); // Throws if malformed + + return { config, location: configPath }; +} +``` + +When a source throws, the CLI displays a warning and continues with other sources. This helps users identify configuration problems without blocking execution. + ### Plugin Configuration Plugins cannot add flags to commands they don't own (this is an oclif limitation). Instead, plugins should accept configuration via environment variables: diff --git a/packages/b2c-tooling-sdk/src/cli/config.ts b/packages/b2c-tooling-sdk/src/cli/config.ts index 2f619312..b0091256 100644 --- a/packages/b2c-tooling-sdk/src/cli/config.ts +++ b/packages/b2c-tooling-sdk/src/cli/config.ts @@ -245,9 +245,9 @@ export function loadConfig( ); } - // Log warnings + // Log warnings (at warn level so users can see configuration issues) for (const warning of resolved.warnings) { - logger.trace({warning}, `[Config] ${warning.message}`); + logger.warn({warning}, `[Config] ${warning.message}`); } return resolved; diff --git a/packages/b2c-tooling-sdk/src/config/dw-json.ts b/packages/b2c-tooling-sdk/src/config/dw-json.ts index 5bc31047..6b1f7541 100644 --- a/packages/b2c-tooling-sdk/src/config/dw-json.ts +++ b/packages/b2c-tooling-sdk/src/config/dw-json.ts @@ -239,9 +239,10 @@ export function loadDwJson(options: LoadDwJsonOptions = {}): LoadDwJsonResult | path: dwJsonPath, }; } catch (error) { - // Invalid JSON or read error + // Invalid JSON or read error - log at trace level and re-throw + // The resolver will catch this and create a SOURCE_ERROR warning const message = error instanceof Error ? error.message : String(error); logger.trace({path: dwJsonPath, error: message}, '[DwJsonSource] Failed to parse config file'); - return undefined; + throw error; } } diff --git a/packages/b2c-tooling-sdk/src/config/mapping.ts b/packages/b2c-tooling-sdk/src/config/mapping.ts index fe9749f2..71676e7a 100644 --- a/packages/b2c-tooling-sdk/src/config/mapping.ts +++ b/packages/b2c-tooling-sdk/src/config/mapping.ts @@ -125,7 +125,7 @@ export function mergeConfigsWithProtection( if (hostnameMismatch && hostnameProtection) { warnings.push({ code: 'HOSTNAME_MISMATCH', - message: `Hostname override "${overrides.hostname}" differs from config file "${base.hostname}". Config file values ignored.`, + message: `Server override "${overrides.hostname}" differs from config file "${base.hostname}". Config file values ignored.`, details: { providedHostname: overrides.hostname, configHostname: base.hostname, diff --git a/packages/b2c-tooling-sdk/src/config/resolver.ts b/packages/b2c-tooling-sdk/src/config/resolver.ts index 9f5cddad..1870a3c4 100644 --- a/packages/b2c-tooling-sdk/src/config/resolver.ts +++ b/packages/b2c-tooling-sdk/src/config/resolver.ts @@ -16,9 +16,11 @@ import type {B2CInstance} from '../instance/index.js'; import {mergeConfigsWithProtection, getPopulatedFields, createInstanceFromConfig} from './mapping.js'; import {DwJsonSource, MobifySource, PackageJsonSource} from './sources/index.js'; import type { + ConfigLoadResult, ConfigSource, ConfigSourceInfo, ConfigResolutionResult, + ConfigWarning, NormalizedConfig, ResolveConfigOptions, ResolvedB2CConfig, @@ -155,6 +157,7 @@ export class ConfigResolver { */ resolve(overrides: Partial = {}, options: ResolveConfigOptions = {}): ConfigResolutionResult { const sourceInfos: ConfigSourceInfo[] = []; + const sourceWarnings: ConfigWarning[] = []; const baseConfig: NormalizedConfig = {}; // Create enriched options that will be updated with accumulated config values. @@ -165,7 +168,19 @@ export class ConfigResolver { // Load from each source in order, merging results // Earlier sources have higher priority - later sources only fill in missing values for (const source of this.sources) { - const result = source.load(enrichedOptions); + let result: ConfigLoadResult | undefined; + try { + result = source.load(enrichedOptions); + } catch (error) { + // Source threw an error (e.g., malformed config file) - create warning and continue + const message = error instanceof Error ? error.message : String(error); + sourceWarnings.push({ + code: 'SOURCE_ERROR', + message: `Failed to load configuration from ${source.name}: ${message}`, + details: {source: source.name, error: message}, + }); + continue; + } if (result && result.config) { const {config: sourceConfig, location} = result; const fields = getPopulatedFields(sourceConfig); @@ -219,10 +234,13 @@ export class ConfigResolver { } // Apply overrides with hostname mismatch protection - const {config, warnings} = mergeConfigsWithProtection(overrides, baseConfig, { + const {config, warnings: mergeWarnings} = mergeConfigsWithProtection(overrides, baseConfig, { hostnameProtection: options.hostnameProtection, }); + // Combine source warnings with merge warnings + const warnings = [...sourceWarnings, ...mergeWarnings]; + return {config, warnings, sources: sourceInfos}; } diff --git a/packages/b2c-tooling-sdk/src/config/sources/mobify-source.ts b/packages/b2c-tooling-sdk/src/config/sources/mobify-source.ts index 221e11ad..01b6cb1f 100644 --- a/packages/b2c-tooling-sdk/src/config/sources/mobify-source.ts +++ b/packages/b2c-tooling-sdk/src/config/sources/mobify-source.ts @@ -70,10 +70,11 @@ export class MobifySource implements ConfigSource { location: mobifyPath, }; } catch (error) { - // Invalid JSON or read error + // Invalid JSON or read error - log at trace level and re-throw + // The resolver will catch this and create a SOURCE_ERROR warning const message = error instanceof Error ? error.message : String(error); logger.trace({location: mobifyPath, error: message}, '[MobifySource] Failed to parse credentials file'); - return undefined; + throw error; } } diff --git a/packages/b2c-tooling-sdk/src/config/sources/package-json-source.ts b/packages/b2c-tooling-sdk/src/config/sources/package-json-source.ts index 229ad62c..06ba5f7e 100644 --- a/packages/b2c-tooling-sdk/src/config/sources/package-json-source.ts +++ b/packages/b2c-tooling-sdk/src/config/sources/package-json-source.ts @@ -108,9 +108,11 @@ export class PackageJsonSource implements ConfigSource { return {config, location: packageJsonPath}; } catch (error) { + // Invalid JSON or read error - log at trace level and re-throw + // The resolver will catch this and create a SOURCE_ERROR warning const message = error instanceof Error ? error.message : String(error); logger.trace({location: packageJsonPath, error: message}, '[PackageJsonSource] Failed to parse package.json'); - return undefined; + throw error; } } } diff --git a/packages/b2c-tooling-sdk/test/config/dw-json.test.ts b/packages/b2c-tooling-sdk/test/config/dw-json.test.ts index e73fc5ee..639ae4aa 100644 --- a/packages/b2c-tooling-sdk/test/config/dw-json.test.ts +++ b/packages/b2c-tooling-sdk/test/config/dw-json.test.ts @@ -151,12 +151,11 @@ describe('config/dw-json', () => { expect(result?.config.hostname).to.equal('root.demandware.net'); }); - it('returns undefined for invalid JSON', () => { + it('throws for invalid JSON', () => { const dwJsonPath = path.join(tempDir, 'dw.json'); fs.writeFileSync(dwJsonPath, 'invalid json'); - const result = loadDwJson(); - expect(result).to.be.undefined; + expect(() => loadDwJson()).to.throw(SyntaxError); }); it('returns undefined for non-existent explicit path', () => { diff --git a/packages/b2c-tooling-sdk/test/config/resolver.test.ts b/packages/b2c-tooling-sdk/test/config/resolver.test.ts index bda0cc85..795bc907 100644 --- a/packages/b2c-tooling-sdk/test/config/resolver.test.ts +++ b/packages/b2c-tooling-sdk/test/config/resolver.test.ts @@ -217,6 +217,72 @@ describe('config/resolver', () => { expect(warnings[0].code).to.equal('HOSTNAME_MISMATCH'); }); + it('creates SOURCE_ERROR warning when source throws', () => { + // Create a source that throws an error + const throwingSource: ConfigSource = { + name: 'throwing-source', + load() { + throw new Error('Malformed config file'); + }, + }; + const validSource = new MockSource('valid', { + hostname: 'example.demandware.net', + clientId: 'valid-client', + }); + const resolver = new ConfigResolver([throwingSource, validSource]); + + const {config, warnings, sources} = resolver.resolve(); + + // Should have one SOURCE_ERROR warning + expect(warnings).to.have.length(1); + expect(warnings[0].code).to.equal('SOURCE_ERROR'); + expect(warnings[0].message).to.include('throwing-source'); + expect(warnings[0].message).to.include('Malformed config file'); + expect(warnings[0].details).to.deep.equal({ + source: 'throwing-source', + error: 'Malformed config file', + }); + + // Valid source should still contribute config + expect(config.hostname).to.equal('example.demandware.net'); + expect(config.clientId).to.equal('valid-client'); + expect(sources).to.have.length(1); + expect(sources[0].name).to.equal('valid'); + }); + + it('continues with remaining sources after SOURCE_ERROR', () => { + // First source throws, second succeeds, third also throws + const throwingSource1: ConfigSource = { + name: 'bad-source-1', + priority: -1, + load() { + throw new Error('Error 1'); + }, + }; + const validSource = new MockSource('valid', {hostname: 'example.com'}, undefined, 0); + const throwingSource2: ConfigSource = { + name: 'bad-source-2', + priority: 1, + load() { + throw new Error('Error 2'); + }, + }; + const resolver = new ConfigResolver([throwingSource1, validSource, throwingSource2]); + + const {config, warnings, sources} = resolver.resolve(); + + // Should have two SOURCE_ERROR warnings + expect(warnings).to.have.length(2); + expect(warnings[0].code).to.equal('SOURCE_ERROR'); + expect(warnings[0].message).to.include('bad-source-1'); + expect(warnings[1].code).to.equal('SOURCE_ERROR'); + expect(warnings[1].message).to.include('bad-source-2'); + + // Valid source contributes config + expect(config.hostname).to.equal('example.com'); + expect(sources).to.have.length(1); + }); + it('returns empty config when no sources have data', () => { const resolver = new ConfigResolver([]); diff --git a/packages/b2c-tooling-sdk/test/config/sources.test.ts b/packages/b2c-tooling-sdk/test/config/sources.test.ts index 6e279165..fda8662f 100644 --- a/packages/b2c-tooling-sdk/test/config/sources.test.ts +++ b/packages/b2c-tooling-sdk/test/config/sources.test.ts @@ -300,7 +300,7 @@ describe('config/sources', () => { } }); - it('returns undefined for invalid JSON in ~/.mobify', function () { + it('creates SOURCE_ERROR warning for invalid JSON in ~/.mobify', function () { const originalHomedir = os.homedir; let canMock = false; try { @@ -320,9 +320,14 @@ describe('config/sources', () => { fs.writeFileSync(mobifyPath, 'invalid json'); const resolver = new ConfigResolver(); - const {config} = resolver.resolve(); + const {config, warnings} = resolver.resolve(); + // Config should not have the API key expect(config.mrtApiKey).to.be.undefined; + // Should have a SOURCE_ERROR warning for MobifySource + const sourceError = warnings.find((w) => w.code === 'SOURCE_ERROR' && w.message.includes('MobifySource')); + expect(sourceError).to.not.be.undefined; + expect(sourceError?.message).to.include('Failed to load configuration'); // Restore Object.defineProperty(os, 'homedir', {