Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Zod error messages and user config error messages #12634

Merged
merged 13 commits into from
Dec 5, 2024
5 changes: 5 additions & 0 deletions .changeset/heavy-buttons-compare.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Improves error message formatting for user config and content collection frontmatter
2 changes: 1 addition & 1 deletion packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ export const AstroConfigSchema = z.object({
}),
})
.strict(
`Invalid or outdated experimental feature.\nCheck for incorrect spelling or outdated Astro version.\nSee https://docs.astro.build/en/reference/configuration-reference/#experimental-flags for a list of all current experiments.`,
`Invalid or outdated experimental feature.\nCheck for incorrect spelling or outdated Astro version.\nSee https://docs.astro.build/en/reference/experimental-flags/ for a list of all current experiments.`,
)
.default({}),
legacy: z
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/core/config/validate.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import type { AstroConfig } from '../../types/public/config.js';
import { errorMap } from '../errors/index.js';
import { createRelativeSchema } from './schema.js';

/** Turn raw config values into normalized values */
Expand All @@ -10,5 +11,5 @@ export async function validateConfig(
const AstroConfigRelativeSchema = createRelativeSchema(cmd, root);

// First-Pass Validation
return await AstroConfigRelativeSchema.parseAsync(userConfig);
return await AstroConfigRelativeSchema.parseAsync(userConfig, { errorMap });
}
95 changes: 61 additions & 34 deletions packages/astro/src/core/errors/zod-error-map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,30 +29,56 @@ export const errorMap: ZodErrorMap = (baseError, ctx) => {
}
}
}
let messages: string[] = [
prefix(
baseErrorPath,
typeOrLiteralErrByPath.size ? 'Did not match union:' : 'Did not match union.',
),
];
const messages: string[] = [prefix(baseErrorPath, 'Did not match union.')];
const details: string[] = [...typeOrLiteralErrByPath.entries()]
// If type or literal error isn't common to ALL union types,
// filter it out. Can lead to confusing noise.
.filter(([, error]) => error.expected.length === baseError.unionErrors.length)
.map(([key, error]) =>
key === baseErrorPath
? // Avoid printing the key again if it's a base error
`> ${getTypeOrLiteralMsg(error)}`
: `> ${prefix(key, getTypeOrLiteralMsg(error))}`,
);

if (details.length === 0) {
const expectedShapes: string[] = [];
for (const unionError of baseError.unionErrors) {
const expectedShape: string[] = [];
for (const issue of unionError.issues) {
// If the issue is a nested union error, show the associated error message instead of the
// base error message.
if (issue.code === 'invalid_union') {
return errorMap(issue, ctx);
}
const relativePath = flattenErrorPath(issue.path)
.replace(baseErrorPath, '')
.replace(leadingPeriod, '');
if ('expected' in issue && typeof issue.expected === 'string') {
expectedShape.push(
relativePath ? `${relativePath}: ${issue.expected}` : issue.expected,
);
} else {
expectedShape.push(relativePath);
}
}
if (expectedShape.length === 1 && !expectedShape[0]?.includes(':')) {
// In this case the expected shape is not an object, but probably a literal type, e.g. `['string']`.
expectedShapes.push(expectedShape.join(''));
} else {
expectedShapes.push(`{ ${expectedShape.join('; ')} }`);
}
}
if (expectedShapes.length) {
details.push('> Expected type `' + expectedShapes.join(' | ') + '`');
details.push('> Received `' + stringify(ctx.data) + '`');
}
}

return {
message: messages
.concat(
[...typeOrLiteralErrByPath.entries()]
// If type or literal error isn't common to ALL union types,
// filter it out. Can lead to confusing noise.
.filter(([, error]) => error.expected.length === baseError.unionErrors.length)
.map(([key, error]) =>
key === baseErrorPath
? // Avoid printing the key again if it's a base error
`> ${getTypeOrLiteralMsg(error)}`
: `> ${prefix(key, getTypeOrLiteralMsg(error))}`,
),
)
.join('\n'),
message: messages.concat(details).join('\n'),
};
}
if (baseError.code === 'invalid_literal' || baseError.code === 'invalid_type') {
} else if (baseError.code === 'invalid_literal' || baseError.code === 'invalid_type') {
return {
message: prefix(
baseErrorPath,
Expand All @@ -71,29 +97,30 @@ export const errorMap: ZodErrorMap = (baseError, ctx) => {
};

const getTypeOrLiteralMsg = (error: TypeOrLiteralErrByPathEntry): string => {
if (error.received === 'undefined') return 'Required';
// received could be `undefined` or the string `'undefined'`
if (typeof error.received === 'undefined' || error.received === 'undefined') return 'Required';
const expectedDeduped = new Set(error.expected);
switch (error.code) {
case 'invalid_type':
return `Expected type \`${unionExpectedVals(expectedDeduped)}\`, received ${JSON.stringify(
return `Expected type \`${unionExpectedVals(expectedDeduped)}\`, received \`${stringify(
error.received,
)}`;
)}\``;
case 'invalid_literal':
return `Expected \`${unionExpectedVals(expectedDeduped)}\`, received ${JSON.stringify(
return `Expected \`${unionExpectedVals(expectedDeduped)}\`, received \`${stringify(
error.received,
)}`;
)}\``;
}
};

const prefix = (key: string, msg: string) => (key.length ? `**${key}**: ${msg}` : msg);

const unionExpectedVals = (expectedVals: Set<unknown>) =>
[...expectedVals]
.map((expectedVal, idx) => {
if (idx === 0) return JSON.stringify(expectedVal);
const sep = ' | ';
return `${sep}${JSON.stringify(expectedVal)}`;
})
.join('');
[...expectedVals].map((expectedVal) => stringify(expectedVal)).join(' | ');

const flattenErrorPath = (errorPath: (string | number)[]) => errorPath.join('.');

/** `JSON.stringify()` a value with spaces around object/array entries. */
const stringify = (val: unknown) =>
JSON.stringify(val, null, 1).split(newlinePlusWhitespace).join(' ');
const newlinePlusWhitespace = /\n\s*/;
const leadingPeriod = /^\./;
18 changes: 14 additions & 4 deletions packages/astro/src/core/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,12 +231,22 @@ function getNetworkLogging(host: string | boolean): 'none' | 'host-to-expose' |
}
}

const codeRegex = /`([^`]+)`/g;

export function formatConfigErrorMessage(err: ZodError) {
const errorList = err.issues.map(
(issue) => ` ! ${bold(issue.path.join('.'))} ${red(issue.message + '.')}`,
const errorList = err.issues.map((issue) =>
`! ${renderErrorMarkdown(issue.message, 'cli')}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My highly-featured markdown renderer keep seeing usage 🫡

// Make text wrapped in backticks blue.
.replaceAll(codeRegex, cyan('$1'))
.split('\n')
// Dim all lines in an issue except for the first which should be red.
.map((line, index) => (index > 0 ? dim(line) : red(line)))
// Indent all lines.
.map((line) => ' ' + line)
.join('\n'),
);
return `${red('[config]')} Astro found issue(s) with your configuration:\n${errorList.join(
'\n',
return `${red('[config]')} Astro found issue(s) with your configuration:\n\n${errorList.join(
'\n\n',
)}`;
}

Expand Down
6 changes: 3 additions & 3 deletions packages/astro/test/content-collections.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ describe('Content Collections', () => {
} catch (e) {
error = e.message;
}
assert.equal(error.includes('**title**: Expected type `"string"`, received "number"'), true);
assert.match(error, /\*\*title\*\*: Expected type `"string"`, received `"number"`/);
});
});
describe('With config.mts', () => {
Expand All @@ -281,7 +281,7 @@ describe('Content Collections', () => {
} catch (e) {
error = e.message;
}
assert.equal(error.includes('**title**: Expected type `"string"`, received "number"'), true);
assert.match(error, /\*\*title\*\*: Expected type `"string"`, received `"number"`/);
});
});

Expand All @@ -296,7 +296,7 @@ describe('Content Collections', () => {
} catch (e) {
error = e.message;
}
assert.equal(error.includes('**title**: Required'), true);
assert.match(error, /\*\*title\*\*: Required/);
});
});

Expand Down
8 changes: 4 additions & 4 deletions packages/astro/test/error-map.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe('Content Collections - error map', () => {
}),
{ foo: 1 },
);
assert.deepEqual(messages(error), ['**foo**: Expected type `"string"`, received "number"']);
assert.deepEqual(messages(error), ['**foo**: Expected type `"string"`, received `"number"`']);
});
it('Returns formatted error for literal mismatch', () => {
const error = getParseError(
Expand All @@ -39,7 +39,7 @@ describe('Content Collections - error map', () => {
}),
{ lang: 'es' },
);
assert.deepEqual(messages(error), ['**lang**: Expected `"en"`, received "es"']);
assert.deepEqual(messages(error), ['**lang**: Expected `"en"`, received `"es"`']);
});
it('Replaces undefined errors with "Required"', () => {
const error = getParseError(
Expand All @@ -58,7 +58,7 @@ describe('Content Collections - error map', () => {
);
assert.deepEqual(messages(error), [
fixLineEndings(
'Did not match union:\n> Expected type `"boolean" | "number"`, received "string"',
'Did not match union.\n> Expected type `"boolean" | "number"`, received `"string"`',
),
]);
});
Expand All @@ -76,7 +76,7 @@ describe('Content Collections - error map', () => {
);
assert.deepEqual(messages(error), [
fixLineEndings(
'Did not match union:\n> **type**: Expected `"tutorial" | "article"`, received "integration-guide"',
'Did not match union.\n> **type**: Expected `"tutorial" | "article"`, received `"integration-guide"`',
),
]);
});
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/legacy-content-collections.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ describe('Legacy Content Collections', () => {
} catch (e) {
error = e.message;
}
assert.equal(error.includes('**title**: Expected type `"string"`, received "number"'), true);
assert.match(error, /\*\*title\*\*: Expected type `"string"`, received `"number"`/);
});
});
describe('With config.mts', () => {
Expand All @@ -302,7 +302,7 @@ describe('Legacy Content Collections', () => {
} catch (e) {
error = e.message;
}
assert.equal(error.includes('**title**: Expected type `"string"`, received "number"'), true);
assert.match(error, /\*\*title\*\*: Expected type `"string"`, received `"number"`/);
});
});

Expand Down
15 changes: 11 additions & 4 deletions packages/astro/test/units/config/config-validate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ describe('Config Validation', () => {
assert.equal(
formattedError,
`[config] Astro found issue(s) with your configuration:
! site Expected string, received number.`,

! site: Expected type "string", received "number"`,
);
});

Expand All @@ -38,8 +39,11 @@ describe('Config Validation', () => {
assert.equal(
formattedError,
`[config] Astro found issue(s) with your configuration:
! integrations.0 Expected object, received number.
! build.format Invalid input.`,

! integrations.0: Expected type "object", received "number"

! build.format: Did not match union.
> Expected "file" | "directory" | "preserve", received "invalid"`,
);
});

Expand Down Expand Up @@ -118,7 +122,10 @@ describe('Config Validation', () => {
process.cwd(),
).catch((err) => err);
assert.equal(configError instanceof z.ZodError, true);
assert.equal(configError.errors[0].message, 'Array must contain at least 1 element(s)');
assert.equal(
configError.errors[0].message,
'**i18n.locales.1.codes**: Array must contain at least 1 element(s)',
);
});

it('errors if the default locale is not in path', async () => {
Expand Down
Loading