Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/surface-config-source-errors.md
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 25 additions & 0 deletions docs/guide/extending.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions packages/b2c-tooling-sdk/src/cli/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions packages/b2c-tooling-sdk/src/config/dw-json.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
2 changes: 1 addition & 1 deletion packages/b2c-tooling-sdk/src/config/mapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
22 changes: 20 additions & 2 deletions packages/b2c-tooling-sdk/src/config/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -155,6 +157,7 @@ export class ConfigResolver {
*/
resolve(overrides: Partial<NormalizedConfig> = {}, options: ResolveConfigOptions = {}): ConfigResolutionResult {
const sourceInfos: ConfigSourceInfo[] = [];
const sourceWarnings: ConfigWarning[] = [];
const baseConfig: NormalizedConfig = {};

// Create enriched options that will be updated with accumulated config values.
Expand All @@ -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);
Expand Down Expand Up @@ -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};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
5 changes: 2 additions & 3 deletions packages/b2c-tooling-sdk/test/config/dw-json.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
66 changes: 66 additions & 0 deletions packages/b2c-tooling-sdk/test/config/resolver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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([]);

Expand Down
9 changes: 7 additions & 2 deletions packages/b2c-tooling-sdk/test/config/sources.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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', {
Expand Down
Loading