Skip to content
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

feat(integrations): Add zod integration #11144

Merged
merged 9 commits into from May 2, 2024
Merged

Conversation

scttcper
Copy link
Member

This adds a Zod integration to sentry that adds better support for ZodError issues. Currently, the ZodError message is a formatted json string that gets truncated and the full list of issues are lost.

  • Adds the full list of issues to extras['zoderror.issues'].
  • Replaces the error message with a simple string.

before
image
image

after
image
image
image

This adds a [Zod](https://github.com/colinhacks/zod) integration to sentry that adds better support for ZodError issues. Currently, the ZodError message is a formatted json string that gets truncated and the full list of issues are lost.

- Adds the full list of issues to `extras['zoderror.issues']`.
- Replaces the error message with a simple string.
Copy link
Contributor

github-actions bot commented Mar 15, 2024

size-limit report 📦

Path Size
@sentry/browser 21.64 KB (0%)
@sentry/browser (incl. Tracing) 32.68 KB (0%)
@sentry/browser (incl. Tracing, Replay) 68.03 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 61.43 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 72.07 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 84.19 KB (-0.11% 🔽)
@sentry/browser (incl. Feedback) 37.74 KB (-0.11% 🔽)
@sentry/browser (incl. sendFeedback) 26.43 KB (+0.01% 🔺)
@sentry/browser (incl. FeedbackAsync) 30.87 KB (-0.19% 🔽)
@sentry/react 24.33 KB (0%)
@sentry/react (incl. Tracing) 35.64 KB (0%)
@sentry/vue 25.47 KB (0%)
@sentry/vue (incl. Tracing) 34.47 KB (0%)
@sentry/svelte 21.77 KB (0%)
CDN Bundle 23.95 KB (0%)
CDN Bundle (incl. Tracing) 33.98 KB (0%)
CDN Bundle (incl. Tracing, Replay) 67.67 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 83.55 KB (+0.01% 🔺)
CDN Bundle - uncompressed 70.58 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 100.94 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 210.55 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 256.94 KB (0%)
@sentry/nextjs (client) 34.86 KB (0%)
@sentry/sveltekit (client) 33.24 KB (0%)
@sentry/node 138.47 KB (0%)

@scttcper scttcper marked this pull request as ready for review March 15, 2024 23:27
@mydea
Copy link
Member

mydea commented Apr 17, 2024

Hey @scttcper do you mind resolving merge conflicts (rebasing on develop), then I'll take a look - thank you! (and sorry for late response, got a bit lost there...)

# Conflicts:
#	packages/node/src/index.ts
@scttcper scttcper requested a review from mydea April 18, 2024 18:13
Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

Looks good to me!

We need to remember also adding this to the docs as a non-default integration, once v8 ships.

* This doesn't display well in the Sentry UI. Replace it with something shorter.
*/
function formatIssueMessage(zodError: ZodError): string {
const formError = zodError.flatten();

Choose a reason for hiding this comment

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

I'd recommend against relying on .flatten here as it's liable to change in Zod 4. Instead consider iterating over the .issues array.

declare let err: z.ZodError;
const errorKeyMap = new Set<string | number | symbol>();
for (const iss of err.issues) {
  if (iss.path) errorKeyMap.add(iss.path[0]);
}
const errorKeys = Array.from(errorKeyMap);

Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

If you could do these two changes, I'd be happy to merge:

  1. Just move all the code in the integration, no need to split this across core & utils.
  2. Apply the feedback that was given about flatten, sounds good to me to be future proof there.

Thanks a lot!

@scttcper scttcper requested a review from mydea April 24, 2024 21:27
@scttcper
Copy link
Member Author

@mydea i think this is ready to go. I assume you'll merge it? I'm not up to date on what's going on with the sdk v8

@scttcper scttcper requested a review from mydea April 29, 2024 18:45
Copy link
Member

@mydea mydea left a comment

Choose a reason for hiding this comment

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

thanks so much for this PR!

@mydea mydea merged commit eadcac5 into develop May 2, 2024
96 checks passed
@mydea mydea deleted the scttcper/zod-integration branch May 2, 2024 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants