Skip to content

Commit a4335cc

Browse files
dzehnderclaude
andcommitted
fix(llmo-config): polish LlmoConfigValidationError per review
Address minor polish items from PR #1574 review: - Pass { cause: zodError } to super() so the original ZodError stack is preserved on the cause chain. Improves debuggability without changing the public error shape. - Use issue `code` instead of `message` in the summary string. Zod's default messages can echo received values, and the LLMO config can carry user-curated content (brand names, competitor URLs) on the api-service write path. The full human-readable detail (including the values) remains on `this.issues` for trusted callers to inspect. - Lock down the message-format contract with a regex assertion so the path-shape (e.g. categories.<uuid>.region) is not silently dropped by a future maintainer. - Switch the test fixture UUID to a v4-conformant value so Zod's validation reaches the inner field rather than failing at the record key. Refs: SITES-43238 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 208d7dc commit a4335cc

2 files changed

Lines changed: 19 additions & 5 deletions

File tree

packages/spacecat-shared-utils/src/llmo-config.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,17 @@ import { llmoConfig } from './schemas.js';
2626
*/
2727
export class LlmoConfigValidationError extends Error {
2828
constructor(siteId, zodError) {
29+
// Use issue `code` rather than `message` in the summary: Zod's default
30+
// messages can echo received values, which may include user-supplied
31+
// content (brand names, competitor URLs) on the api-service write path.
32+
// The full message and value remain on `this.issues` for trusted callers.
2933
const summary = zodError.issues
30-
.map((i) => `${i.path.join('.')}: ${i.message}`)
34+
.map((i) => `${i.path.join('.')}: ${i.code}`)
3135
.join('; ');
32-
super(`LLMO config for site ${siteId} failed schema validation: ${summary}`);
36+
super(
37+
`LLMO config for site ${siteId} failed schema validation: ${summary}`,
38+
{ cause: zodError },
39+
);
3340
this.name = 'LlmoConfigValidationError';
3441
this.siteId = siteId;
3542
this.issues = zodError.issues;

packages/spacecat-shared-utils/test/llmo-config.test.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ describe('llmo-config utilities', () => {
206206
const invalidConfig = {
207207
...validConfig,
208208
categories: {
209-
'00000000-0000-0000-0000-000000000001': { name: 'brand', region: 'en-us' },
209+
'550e8400-e29b-41d4-a716-446655440000': { name: 'brand', region: 'en-us' },
210210
},
211211
};
212212

@@ -224,11 +224,11 @@ describe('llmo-config utilities', () => {
224224
expect(s3Client.send).not.called;
225225
});
226226

227-
it('LlmoConfigValidationError carries siteId, name, and Zod issues', async () => {
227+
it('LlmoConfigValidationError carries siteId, name, Zod issues, and cause', async () => {
228228
const invalidConfig = {
229229
...validConfig,
230230
categories: {
231-
'00000000-0000-0000-0000-000000000001': { name: 'brand', region: 'en-us' },
231+
'550e8400-e29b-41d4-a716-446655440000': { name: 'brand', region: 'en-us' },
232232
},
233233
};
234234

@@ -243,7 +243,14 @@ describe('llmo-config utilities', () => {
243243
expect(caught.name).equals('LlmoConfigValidationError');
244244
expect(caught.siteId).equals(siteId);
245245
expect(caught.issues).to.be.an('array').with.length.greaterThan(0);
246+
247+
// Message format: "...failed schema validation: <path>: <code>"
246248
expect(caught.message).to.include(siteId);
249+
expect(caught.message).to.match(/categories\..+\.region/);
250+
251+
// Original ZodError preserved as cause for stack-chain debugging.
252+
expect(caught.cause).to.exist;
253+
expect(caught.cause.issues).to.equal(caught.issues);
247254
});
248255
});
249256

0 commit comments

Comments
 (0)