-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[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
- Loading branch information
1 parent
f2dfdb7
commit c638740
Showing
7 changed files
with
220 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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('.'); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
107 changes: 107 additions & 0 deletions
107
packages/astro/test/units/content-collections/error-map.test.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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; | ||
} |