Improve Zod error messages and user config error messages (#12634)

* 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
withastro · Dec 5, 2024 · 03958d9 · 03958d9
1 parent 8704c54
commit 03958d9
Show file tree

Hide file tree

Showing 9 changed files with 101 additions and 53 deletions.
diff --git a/.changeset/heavy-buttons-compare.md 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
@@ -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
@@ -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
@@ -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<unknown>) =>
-	[...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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 () => {