-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(core): Improve error formatting in ZodErrors integration #15111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
06232f3
1ffaf4b
f134135
d92072d
f1cae8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,33 +5,45 @@ import { truncate } from '../utils-hoist/string'; | |
|
||
interface ZodErrorsOptions { | ||
key?: string; | ||
/** | ||
* Limits the number of Zod errors inlined in each Sentry event. | ||
* | ||
* @default 10 | ||
*/ | ||
limit?: number; | ||
/** | ||
* Save full list of Zod issues as an attachment in Sentry | ||
* | ||
* @default false | ||
*/ | ||
saveZodIssuesAsAttachment?: boolean; | ||
} | ||
|
||
const DEFAULT_LIMIT = 10; | ||
const INTEGRATION_NAME = 'ZodErrors'; | ||
|
||
// Simplified ZodIssue type definition | ||
/** | ||
* Simplified ZodIssue type definition | ||
*/ | ||
interface ZodIssue { | ||
path: (string | number)[]; | ||
message?: string; | ||
expected?: string | number; | ||
received?: string | number; | ||
expected?: unknown; | ||
received?: unknown; | ||
unionErrors?: unknown[]; | ||
keys?: unknown[]; | ||
invalid_literal?: unknown; | ||
} | ||
|
||
interface ZodError extends Error { | ||
issues: ZodIssue[]; | ||
|
||
get errors(): ZodError['issues']; | ||
} | ||
|
||
function originalExceptionIsZodError(originalException: unknown): originalException is ZodError { | ||
return ( | ||
isError(originalException) && | ||
originalException.name === 'ZodError' && | ||
Array.isArray((originalException as ZodError).errors) | ||
Array.isArray((originalException as ZodError).issues) | ||
); | ||
} | ||
|
||
|
@@ -45,9 +57,18 @@ type SingleLevelZodIssue<T extends ZodIssue> = { | |
|
||
/** | ||
* Formats child objects or arrays to a string | ||
* That is preserved when sent to Sentry | ||
* that is preserved when sent to Sentry. | ||
* | ||
* Without this, we end up with something like this in Sentry: | ||
* | ||
* [ | ||
* [Object], | ||
* [Object], | ||
* [Object], | ||
* [Object] | ||
* ] | ||
*/ | ||
function formatIssueTitle(issue: ZodIssue): SingleLevelZodIssue<ZodIssue> { | ||
export function flattenIssue(issue: ZodIssue): SingleLevelZodIssue<ZodIssue> { | ||
return { | ||
...issue, | ||
path: 'path' in issue && Array.isArray(issue.path) ? issue.path.join('.') : undefined, | ||
|
@@ -56,64 +77,145 @@ function formatIssueTitle(issue: ZodIssue): SingleLevelZodIssue<ZodIssue> { | |
}; | ||
} | ||
|
||
/** | ||
* Takes ZodError issue path array and returns a flattened version as a string. | ||
* This makes it easier to display paths within a Sentry error message. | ||
* | ||
* Array indexes are normalized to reduce duplicate entries | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure we want to duplicate entries here, I think What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only applied to the issue message, not In practice, we found that having the indexes in the message is more noise than helpful for understanding the issue at a glance, which was the primary motivation for me making this improvement in our internal fork. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good to me, let's keep what is here then |
||
* | ||
* @param path ZodError issue path | ||
* @returns flattened path | ||
* | ||
* @example | ||
* flattenIssuePath([0, 'foo', 1, 'bar']) // -> '<array>.foo.<array>.bar' | ||
*/ | ||
export function flattenIssuePath(path: Array<string | number>): string { | ||
return path | ||
.map(p => { | ||
if (typeof p === 'number') { | ||
return '<array>'; | ||
} else { | ||
return p; | ||
} | ||
}) | ||
.join('.'); | ||
} | ||
|
||
/** | ||
* Zod error message is a stringified version of ZodError.issues | ||
* This doesn't display well in the Sentry UI. Replace it with something shorter. | ||
*/ | ||
function formatIssueMessage(zodError: ZodError): string { | ||
export function formatIssueMessage(zodError: ZodError): string { | ||
const errorKeyMap = new Set<string | number | symbol>(); | ||
for (const iss of zodError.issues) { | ||
if (iss.path?.[0]) { | ||
errorKeyMap.add(iss.path[0]); | ||
const issuePath = flattenIssuePath(iss.path); | ||
if (issuePath.length > 0) { | ||
errorKeyMap.add(issuePath); | ||
} | ||
} | ||
const errorKeys = Array.from(errorKeyMap); | ||
|
||
const errorKeys = Array.from(errorKeyMap); | ||
if (errorKeys.length === 0) { | ||
// If there are no keys, then we're likely validating the root | ||
// variable rather than a key within an object. This attempts | ||
// to extract what type it was that failed to validate. | ||
// For example, z.string().parse(123) would return "string" here. | ||
let rootExpectedType = 'variable'; | ||
if (zodError.issues.length > 0) { | ||
const iss = zodError.issues[0]; | ||
if (iss !== undefined && 'expected' in iss && typeof iss.expected === 'string') { | ||
rootExpectedType = iss.expected; | ||
} | ||
} | ||
return `Failed to validate ${rootExpectedType}`; | ||
} | ||
return `Failed to validate keys: ${truncate(errorKeys.join(', '), 100)}`; | ||
} | ||
|
||
/** | ||
* Applies ZodError issues to an event extras and replaces the error message | ||
* Applies ZodError issues to an event extra and replaces the error message | ||
*/ | ||
export function applyZodErrorsToEvent(limit: number, event: Event, hint?: EventHint): Event { | ||
export function applyZodErrorsToEvent( | ||
limit: number, | ||
saveZodIssuesAsAttachment: boolean = false, | ||
event: Event, | ||
hint: EventHint, | ||
): Event { | ||
if ( | ||
!event.exception?.values || | ||
!hint?.originalException || | ||
!hint.originalException || | ||
!originalExceptionIsZodError(hint.originalException) || | ||
hint.originalException.issues.length === 0 | ||
) { | ||
return event; | ||
} | ||
|
||
return { | ||
...event, | ||
exception: { | ||
...event.exception, | ||
values: [ | ||
{ | ||
...event.exception.values[0], | ||
value: formatIssueMessage(hint.originalException), | ||
try { | ||
const issuesToFlatten = saveZodIssuesAsAttachment | ||
? hint.originalException.issues | ||
: hint.originalException.issues.slice(0, limit); | ||
const flattenedIssues = issuesToFlatten.map(flattenIssue); | ||
|
||
if (saveZodIssuesAsAttachment) { | ||
// Sometimes having the full error details can be helpful. | ||
// Attachments have much higher limits, so we can include the full list of issues. | ||
if (!Array.isArray(hint.attachments)) { | ||
hint.attachments = []; | ||
} | ||
hint.attachments.push({ | ||
filename: 'zod_issues.json', | ||
data: JSON.stringify({ | ||
issues: flattenedIssues, | ||
}), | ||
}); | ||
} | ||
|
||
return { | ||
...event, | ||
exception: { | ||
...event.exception, | ||
values: [ | ||
{ | ||
...event.exception.values[0], | ||
value: formatIssueMessage(hint.originalException), | ||
}, | ||
...event.exception.values.slice(1), | ||
], | ||
}, | ||
extra: { | ||
...event.extra, | ||
'zoderror.issues': flattenedIssues.slice(0, limit), | ||
}, | ||
}; | ||
} catch (e) { | ||
// Hopefully we never throw errors here, but record it | ||
// with the event just in case. | ||
return { | ||
...event, | ||
extra: { | ||
...event.extra, | ||
'zoderrors sentry integration parse error': { | ||
message: 'an exception was thrown while processing ZodError within applyZodErrorsToEvent()', | ||
error: e instanceof Error ? `${e.name}: ${e.message}\n${e.stack}` : 'unknown', | ||
}, | ||
...event.exception.values.slice(1), | ||
], | ||
}, | ||
extra: { | ||
...event.extra, | ||
'zoderror.issues': hint.originalException.errors.slice(0, limit).map(formatIssueTitle), | ||
}, | ||
}; | ||
}, | ||
}; | ||
} | ||
} | ||
|
||
const _zodErrorsIntegration = ((options: ZodErrorsOptions = {}) => { | ||
const limit = options.limit || DEFAULT_LIMIT; | ||
const limit = options.limit ?? DEFAULT_LIMIT; | ||
|
||
return { | ||
name: INTEGRATION_NAME, | ||
processEvent(originalEvent, hint) { | ||
const processedEvent = applyZodErrorsToEvent(limit, originalEvent, hint); | ||
processEvent(originalEvent, hint): Event { | ||
const processedEvent = applyZodErrorsToEvent(limit, options.saveZodIssuesAsAttachment, originalEvent, hint); | ||
return processedEvent; | ||
}, | ||
}; | ||
}) satisfies IntegrationFn; | ||
|
||
/** | ||
* Sentry integration to process Zod errors, making them easier to work with in Sentry. | ||
*/ | ||
export const zodErrorsIntegration = defineIntegration(_zodErrorsIntegration); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure if adding deps was okay, but it made adding tests a lot easier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine for testing