Skip to content

Commit

Permalink
[Content collections] Better error formatting (#6508)
Browse files Browse the repository at this point in the history
* feat: new error map with unions, literal, and fbs

* fix: bad type casting on type error

* refactor: add comments, clean up edge cases

* refactor: JSON.stringify, add colon for follow ups

* refactor: bold file name in msg

* fix: prefix union errors

* test: new error mapping

* refactor: clean up Required handling

* test: required, fallthrough

* chore: changeset

* fix: out-of-date error msg

* chore: stray console log

* docs: revert to old error msg
  • Loading branch information
bholmesdev authored Mar 13, 2023
1 parent f2dfdb7 commit c638740
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 16 deletions.
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 type { AstroConfig, AstroSettings } from '../@types/astro.js';
import { emitESMImage } from '../assets/utils/emitAsset.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;
}

0 comments on commit c638740

Please sign in to comment.