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
8 changes: 8 additions & 0 deletions .changeset/quick-turtles-decide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'astro': patch
---

Improve content collection error formatting:
- Bold the collection and entry that failed
- Consistently list the frontmatter key at the start of every error
- Rich errors for union types
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
4 changes: 3 additions & 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,9 @@ 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 does not match collection schema.`,
...error.errors.map((zodError) => zodError.message),
].join('\n');
},
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/content-collections.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ describe('Content Collections', () => {
} catch (e) {
error = e.message;
}
expect(error).to.include('"title" should be string, not number.');
expect(error).to.include('**title**: Expected type `"string"`, received "number"');
});
});

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;
}