From c6387409062f1d7c2afc93319748ad57086837c5 Mon Sep 17 00:00:00 2001 From: Ben Holmes Date: Mon, 13 Mar 2023 14:16:17 -0400 Subject: [PATCH] [Content collections] Better error formatting (#6508) * 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 --- .changeset/quick-turtles-decide.md | 8 ++ packages/astro/src/content/error-map.ts | 99 ++++++++++++++++ packages/astro/src/content/index.ts | 1 + packages/astro/src/content/utils.ts | 15 +-- packages/astro/src/core/errors/errors-data.ts | 4 +- .../astro/test/content-collections.test.js | 2 +- .../content-collections/error-map.test.js | 107 ++++++++++++++++++ 7 files changed, 220 insertions(+), 16 deletions(-) create mode 100644 .changeset/quick-turtles-decide.md create mode 100644 packages/astro/src/content/error-map.ts create mode 100644 packages/astro/test/units/content-collections/error-map.test.js diff --git a/.changeset/quick-turtles-decide.md b/.changeset/quick-turtles-decide.md new file mode 100644 index 0000000000..e3ce95beb3 --- /dev/null +++ b/.changeset/quick-turtles-decide.md @@ -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 diff --git a/packages/astro/src/content/error-map.ts b/packages/astro/src/content/error-map.ts new file mode 100644 index 0000000000..51c8cdde17 --- /dev/null +++ b/packages/astro/src/content/error-map.ts @@ -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 = 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) => + [...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('.'); diff --git a/packages/astro/src/content/index.ts b/packages/astro/src/content/index.ts index f12106b0aa..7504389e9d 100644 --- a/packages/astro/src/content/index.ts +++ b/packages/astro/src/content/index.ts @@ -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'; diff --git a/packages/astro/src/content/utils.ts b/packages/astro/src/content/utils.ts index 00d0f61e4b..2db053dd3f 100644 --- a/packages/astro/src/content/utils.ts +++ b/packages/astro/src/content/utils.ts @@ -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(), @@ -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; diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index a4db5ab3ef..44e210ad34 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -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'); }, diff --git a/packages/astro/test/content-collections.test.js b/packages/astro/test/content-collections.test.js index 67ef08235f..dd86288d1c 100644 --- a/packages/astro/test/content-collections.test.js +++ b/packages/astro/test/content-collections.test.js @@ -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"'); }); }); diff --git a/packages/astro/test/units/content-collections/error-map.test.js b/packages/astro/test/units/content-collections/error-map.test.js new file mode 100644 index 0000000000..4284ad7213 --- /dev/null +++ b/packages/astro/test/units/content-collections/error-map.test.js @@ -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; +}