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

[Content collections] Better error formatting #6508

Merged
merged 13 commits into from
Mar 13, 2023
99 changes: 99 additions & 0 deletions packages/astro/src/content/error-map.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import type { ZodErrorMap } from 'zod';

type TypeOrLiteralErrByPathEntry = {
code: 'invalid_type' | 'invalid_literal';
received: unknown;
expected: unknown[];
};

export const errorMap: ZodErrorMap = (baseError, ctx) => {
const baseErrorPath = flattenErrorPath(baseError.path);
if (baseError.code === 'invalid_union') {
// Optimization: Combine type and literal errors for keys that are common across ALL union types
// Ex. a union between `{ key: z.literal('tutorial') }` and `{ key: z.literal('blog') }` will
// raise a single error when `key` does not match:
// > Did not match union.
// > key: Expected `'tutorial' | 'blog'`, received 'foo'
let typeOrLiteralErrByPath: Map<string, TypeOrLiteralErrByPathEntry> = new Map();
for (const unionError of baseError.unionErrors.map((e) => e.errors).flat()) {
if (unionError.code === 'invalid_type' || unionError.code === 'invalid_literal') {
const flattenedErrorPath = flattenErrorPath(unionError.path);
if (typeOrLiteralErrByPath.has(flattenedErrorPath)) {
typeOrLiteralErrByPath.get(flattenedErrorPath)!.expected.push(unionError.expected);
} else {
typeOrLiteralErrByPath.set(flattenedErrorPath, {
code: unionError.code,
received: unionError.received,
expected: [unionError.expected],
});
}
}
}
let messages: string[] = [
prefix(
baseErrorPath,
typeOrLiteralErrByPath.size ? 'Did not match union:' : 'Did not match union.'
),
];
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'),
};
}
if (baseError.code === 'invalid_literal' || baseError.code === 'invalid_type') {
return {
message: prefix(
baseErrorPath,
getTypeOrLiteralMsg({
code: baseError.code,
received: baseError.received,
expected: [baseError.expected],
})
),
};
} else if (baseError.message) {
return { message: prefix(baseErrorPath, baseError.message) };
} else {
return { message: prefix(baseErrorPath, ctx.defaultError) };
}
};

const getTypeOrLiteralMsg = (error: TypeOrLiteralErrByPathEntry): string => {
if (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(
error.received
)}`;
case 'invalid_literal':
return `Expected \`${unionExpectedVals(expectedDeduped)}\`, received ${JSON.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('');

const flattenErrorPath = (errorPath: (string | number)[]) => errorPath.join('.');
1 change: 1 addition & 0 deletions packages/astro/src/content/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@ export { contentObservable, getContentPaths, getDotAstroTypeReference } from './
export { astroContentAssetPropagationPlugin } from './vite-plugin-content-assets.js';
export { astroContentImportPlugin } from './vite-plugin-content-imports.js';
export { astroContentVirtualModPlugin } from './vite-plugin-content-virtual-mod.js';
export { errorMap } from './error-map.js';
15 changes: 1 addition & 14 deletions packages/astro/src/content/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { AstroConfig, AstroSettings } from '../@types/astro.js';
import { emitESMImage } from '../assets/internal.js';
import { AstroError, AstroErrorData } from '../core/errors/index.js';
import { CONTENT_TYPES_FILE } from './consts.js';
import { errorMap } from './error-map.js';

export const collectionConfigParser = z.object({
schema: z.any().optional(),
Expand Down Expand Up @@ -221,20 +222,6 @@ function hasUnderscoreBelowContentDirectoryPath(
return false;
}

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

const errorMap: z.ZodErrorMap = (error, ctx) => {
if (error.code === 'invalid_type') {
const badKeyPath = JSON.stringify(flattenErrorPath(error.path));
if (error.received === 'undefined') {
return { message: `${badKeyPath} is required.` };
} else {
return { message: `${badKeyPath} should be ${error.expected}, not ${error.received}.` };
}
}
return { message: ctx.defaultError };
};

function getFrontmatterErrorLine(rawFrontmatter: string, frontmatterKey: string) {
const indexOfFrontmatterKey = rawFrontmatter.indexOf(`\n${frontmatterKey}`);
if (indexOfFrontmatterKey === -1) return 0;
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ See https://docs.astro.build/en/guides/server-side-rendering/ for more informati
code: 9001,
message: (collection: string, entryId: string, error: ZodError) => {
return [
`${String(collection)} → ${String(entryId)} frontmatter does not match collection schema.`,
`**${String(collection)} → ${String(entryId)}** frontmatter is invalid.`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Flagging for docs review @sarah11918 👀

Copy link
Member

Choose a reason for hiding this comment

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

@bholmesdev I love the change overall! And, I know you wanted something shorter here, but I don't love "invalid" vs "does not match what you said you wanted here." (which is why it's invalid)

If you feel the need to shorten, something like ...

"unexpected frontmatter" feels a little more user friendly?

Your call! Non-blocking!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood! Didn't feel too strongly about this change. I think I'll revert to the old wording, with the bolding added 👍

...error.errors.map((zodError) => zodError.message),
].join('\n');
},
Expand Down
107 changes: 107 additions & 0 deletions packages/astro/test/units/content-collections/error-map.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { z } from '../../../zod.mjs';
import { errorMap } from '../../../dist/content/index.js';
import { fixLineEndings } from '../../test-utils.js';
import { expect } from 'chai';

describe('Content Collections - error map', () => {
it('Prefixes messages with object key', () => {
const error = getParseError(
z.object({
base: z.string(),
nested: z.object({
key: z.string(),
}),
union: z.union([z.string(), z.number()]),
}),
{ base: 1, nested: { key: 2 }, union: true }
);
const msgs = messages(error).sort();
expect(msgs).to.have.length(3);
// expect "**" for bolding
expect(msgs[0].startsWith('**base**')).to.equal(true);
expect(msgs[1].startsWith('**nested.key**')).to.equal(true);
expect(msgs[2].startsWith('**union**')).to.equal(true);
});
it('Returns formatted error for type mismatch', () => {
const error = getParseError(
z.object({
foo: z.string(),
}),
{ foo: 1 }
);
expect(messages(error)).to.deep.equal(['**foo**: Expected type `"string"`, received "number"']);
});
it('Returns formatted error for literal mismatch', () => {
const error = getParseError(
z.object({
lang: z.literal('en'),
}),
{ lang: 'es' }
);
expect(messages(error)).to.deep.equal(['**lang**: Expected `"en"`, received "es"']);
});
it('Replaces undefined errors with "Required"', () => {
const error = getParseError(
z.object({
foo: z.string(),
bar: z.string(),
}),
{ foo: 'foo' }
);
expect(messages(error)).to.deep.equal(['**bar**: Required']);
});
it('Returns formatted error for basic union mismatch', () => {
const error = getParseError(
z.union([z.boolean(), z.number()]),
'not a boolean or a number, oops!'
);
expect(messages(error)).to.deep.equal([
fixLineEndings(
'Did not match union:\n> Expected type `"boolean" | "number"`, received "string"'
),
]);
});
it('Returns formatted error for union mismatch on nested object properties', () => {
const error = getParseError(
z.union([
z.object({
type: z.literal('tutorial'),
}),
z.object({
type: z.literal('article'),
}),
]),
{ type: 'integration-guide' }
);
expect(messages(error)).to.deep.equal([
fixLineEndings(
'Did not match union:\n> **type**: Expected `"tutorial" | "article"`, received "integration-guide"'
),
]);
});
it('Lets unhandled errors fall through', () => {
const error = getParseError(
z.object({
lang: z.enum(['en', 'fr']),
}),
{ lang: 'jp' }
);
expect(messages(error)).to.deep.equal([
"**lang**: Invalid enum value. Expected 'en' | 'fr', received 'jp'",
]);
});
});

/**
* @param {z.ZodError} error
* @returns string[]
*/
function messages(error) {
return error.errors.map((e) => e.message);
}

function getParseError(schema, entry, parseOpts = { errorMap }) {
const res = schema.safeParse(entry, parseOpts);
expect(res.success).to.equal(false, 'Schema should raise error');
return res.error;
}