Skip to content

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

Merged
merged 5 commits into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,8 @@
"volta": {
"extends": "../../package.json"
},
"sideEffects": false
"sideEffects": false,
"devDependencies": {
"zod": "^3.24.1"
Copy link
Contributor Author

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

Copy link
Member

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

}
}
170 changes: 136 additions & 34 deletions packages/core/src/integrations/zoderrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}

Expand All @@ -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,
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want to duplicate entries here, I think [0].foo.[1].bar is more valuable than <array>.foo.<array>.bar, and we want to make sure that different array entries will lead to different issues.

What do you think?

Copy link
Contributor Author

@jahands jahands Jan 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only applied to the issue message, not event.extra or attachments which still include all unique issues with the full path including indexes.

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.

Copy link
Member

Choose a reason for hiding this comment

The 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);
Loading
Loading