From 03958d939217e6acef25c0aa1af2de663b04c956 Mon Sep 17 00:00:00 2001 From: Chris Swithinbank Date: Thu, 5 Dec 2024 13:48:00 +0100 Subject: [PATCH] Improve Zod error messages and user config error messages (#12634) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Upstream Zod error map improvements from Starlight * Update tests * Use Zod error map when validating user config * Update user config error formatting for error map * Tweak colours * Separate colour and indentation steps for clarity * Fix broken link to experimental flag documentation * Add changeset * Update tests * Fix one more test * Use existing utility for bold and other Markdown formatting * Extract codeRegex to top level * Don’t dim text and tweak indentation --- .changeset/heavy-buttons-compare.md | 5 + packages/astro/src/core/config/schema.ts | 2 +- packages/astro/src/core/config/validate.ts | 3 +- .../astro/src/core/errors/zod-error-map.ts | 95 ++++++++++++------- packages/astro/src/core/messages.ts | 16 +++- .../astro/test/content-collections.test.js | 6 +- packages/astro/test/error-map.test.js | 8 +- .../test/legacy-content-collections.test.js | 4 +- .../test/units/config/config-validate.test.js | 15 ++- 9 files changed, 101 insertions(+), 53 deletions(-) create mode 100644 .changeset/heavy-buttons-compare.md diff --git a/.changeset/heavy-buttons-compare.md b/.changeset/heavy-buttons-compare.md new file mode 100644 index 0000000000..d59e7bd37f --- /dev/null +++ b/.changeset/heavy-buttons-compare.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Improves error message formatting for user config and content collection frontmatter diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 464ea3de1a..467cf137bc 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -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 diff --git a/packages/astro/src/core/config/validate.ts b/packages/astro/src/core/config/validate.ts index 75a64f1f7d..c293b5b58f 100644 --- a/packages/astro/src/core/config/validate.ts +++ b/packages/astro/src/core/config/validate.ts @@ -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 */ @@ -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 }); } diff --git a/packages/astro/src/core/errors/zod-error-map.ts b/packages/astro/src/core/errors/zod-error-map.ts index 647fc0db38..4137191e45 100644 --- a/packages/astro/src/core/errors/zod-error-map.ts +++ b/packages/astro/src/core/errors/zod-error-map.ts @@ -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, @@ -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) => - [...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 = /^\./; diff --git a/packages/astro/src/core/messages.ts b/packages/astro/src/core/messages.ts index 57c44b0650..4198e96c37 100644 --- a/packages/astro/src/core/messages.ts +++ b/packages/astro/src/core/messages.ts @@ -231,12 +231,20 @@ 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')}` + // Make text wrapped in backticks blue. + .replaceAll(codeRegex, blue('$1')) + // Make the first line red and indent the rest. + .split('\n') + .map((line, index) => (index === 0 ? red(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', )}`; } diff --git a/packages/astro/test/content-collections.test.js b/packages/astro/test/content-collections.test.js index 19a47f7e4c..43c2e22a6a 100644 --- a/packages/astro/test/content-collections.test.js +++ b/packages/astro/test/content-collections.test.js @@ -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', () => { @@ -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"`/); }); }); @@ -296,7 +296,7 @@ describe('Content Collections', () => { } catch (e) { error = e.message; } - assert.equal(error.includes('**title**: Required'), true); + assert.match(error, /\*\*title\*\*: Required/); }); }); diff --git a/packages/astro/test/error-map.test.js b/packages/astro/test/error-map.test.js index 1906aaa210..e3f13e7371 100644 --- a/packages/astro/test/error-map.test.js +++ b/packages/astro/test/error-map.test.js @@ -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( @@ -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( @@ -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"`', ), ]); }); @@ -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"`', ), ]); }); diff --git a/packages/astro/test/legacy-content-collections.test.js b/packages/astro/test/legacy-content-collections.test.js index 6c5335dde5..613d76e1ec 100644 --- a/packages/astro/test/legacy-content-collections.test.js +++ b/packages/astro/test/legacy-content-collections.test.js @@ -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', () => { @@ -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"`/); }); }); diff --git a/packages/astro/test/units/config/config-validate.test.js b/packages/astro/test/units/config/config-validate.test.js index 0fa67373a9..2ef08e4b6a 100644 --- a/packages/astro/test/units/config/config-validate.test.js +++ b/packages/astro/test/units/config/config-validate.test.js @@ -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"`, ); }); @@ -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"`, ); }); @@ -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 () => {