feat(core): Improve error formatting in ZodErrors integration #15111

jahands · 2025-01-21T15:00:00Z

Include full key path rather than the top level key in title
Improve message for validation issues with no path
Add option to include extended issue information as an attachment

Background

I tried using this integration, but found the error format in Sentry to be lacking.
I added tests to demonstrate how this will format issues.

Screenshots

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

If you've added code that should be tested, please add tests.
Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

jahands · 2025-01-21T15:02:34Z

packages/core/package.json

-  "sideEffects": false
+  "sideEffects": false,
+  "devDependencies": {
+    "zod": "^3.24.1"


Wasn't sure if adding deps was okay, but it made adding tests a lot easier

This is fine for testing

- Include full key path rather than the top level key in title - Improve message for validation issues with no path - Add option to include extended issue information as an attachment

AbhiPrasad · 2025-01-21T15:30:14Z

@jahands thanks for the PR! could you share a link to a sentry event / screenshot of a sentry event that shows the changes from this PR?

jahands · 2025-01-21T16:38:16Z

@AbhiPrasad done!

AbhiPrasad · 2025-01-21T17:30:02Z

Assigning myself so I can help review and merge this in! On my todo to review tomorrow morning :)

packages/core/src/integrations/zoderrors.ts

AbhiPrasad · 2025-01-22T14:17:56Z

packages/core/package.json

-  "sideEffects": false
+  "sideEffects": false,
+  "devDependencies": {
+    "zod": "^3.24.1"


This is fine for testing

AbhiPrasad · 2025-01-22T14:22:06Z

packages/core/src/integrations/zoderrors.ts

+ * 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


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?

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.

Sounds good to me, let's keep what is here then

packages/core/src/integrations/zoderrors.ts

AbhiPrasad · 2025-01-23T14:50:40Z

Hmm tests seem to be failing because the attachment doesn't match the issues being sent anymore. I wonder why 🤔

 FAIL  test/lib/integrations/zoderrrors.test.ts
  ● applyZodErrorsToEvent() › should add all ZodError issues as attachment

    expect(received).toMatchInlineSnapshot(snapshot)

    Snapshot name: `applyZodErrorsToEvent() should add all ZodError issues as attachment 1`

    - Snapshot  - 8
    + Received  + 0

    @@ -6,15 +6,7 @@
            "keys": "[\"extra\"]",
            "message": "Invalid input: expected string, received number",
            "path": "names.1",
            "received": "number",
          },
    -     Object {
    -       "code": "invalid_type",
    -       "expected": "string",
    -       "keys": "[\"extra2\"]",
    -       "message": "Invalid input: expected string, received number",
    -       "path": "foo.1",
    -       "received": "number",
    -     },
        ],
      }

      168 |     }
      169 |     expect(attachment.filename).toBe('zod_issues.json');
    > 170 |     expect(JSON.parse(attachment.data.toString())).toMatchInlineSnapshot(`
          |                                                    ^
      171 | Object {
      172 |   "issues": Array [
      173 |     Object {

      at Object.<anonymous> (test/lib/integrations/zoderrrors.test.ts:170:52)

jahands · 2025-01-23T15:30:37Z

Hmm tests seem to be failing because the attachment doesn't match the issues being sent anymore. I wonder why 🤔

 FAIL  test/lib/integrations/zoderrrors.test.ts
  ● applyZodErrorsToEvent() › should add all ZodError issues as attachment

    expect(received).toMatchInlineSnapshot(snapshot)

    Snapshot name: `applyZodErrorsToEvent() should add all ZodError issues as attachment 1`

    - Snapshot  - 8
    + Received  + 0

    @@ -6,15 +6,7 @@
            "keys": "[\"extra\"]",
            "message": "Invalid input: expected string, received number",
            "path": "names.1",
            "received": "number",
          },
    -     Object {
    -       "code": "invalid_type",
    -       "expected": "string",
    -       "keys": "[\"extra2\"]",
    -       "message": "Invalid input: expected string, received number",
    -       "path": "foo.1",
    -       "received": "number",
    -     },
        ],
      }

      168 |     }
      169 |     expect(attachment.filename).toBe('zod_issues.json');
    > 170 |     expect(JSON.parse(attachment.data.toString())).toMatchInlineSnapshot(`
          |                                                    ^
      171 | Object {
      172 |   "issues": Array [
      173 |     Object {

      at Object.<anonymous> (test/lib/integrations/zoderrrors.test.ts:170:52)

Oh I see, we need to skip slicing the list of issues before flattening if save attachment is enabled so that the full list is included in the attachment. Will fix

jahands · 2025-01-23T15:58:42Z

@AbhiPrasad fixed that bug, tests pass for me locally now

This PR adds the external contributor to the CHANGELOG.md file, so that they are credited for their contribution. See #15111 Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>

- Include full key path rather than the top level key in title - Improve message for validation issues with no path - Add option to include extended issue information as an attachment

- Adds improvements based on feedback I got while PR'ing this to sentry-javascript: getsentry/sentry-javascript#15111 - Exports zodErrorsIntegration in the root index.ts (missed this in the original PR)

jahands commented Jan 21, 2025

View reviewed changes

jahands changed the title ~~feat(core): Improve error formatting in ZodErrors integration~~ feat(integrations): Improve error formatting in ZodErrors integration Jan 21, 2025

feat(integrations): Improve error formatting in ZodErrors integration

06232f3

- Include full key path rather than the top level key in title - Improve message for validation issues with no path - Add option to include extended issue information as an attachment

jahands force-pushed the zod-sentry-improvements branch from faa1518 to 06232f3 Compare January 21, 2025 15:03

AbhiPrasad requested review from AbhiPrasad, a team and chargome and removed request for a team January 21, 2025 17:29

AbhiPrasad self-assigned this Jan 21, 2025

AbhiPrasad changed the title ~~feat(integrations): Improve error formatting in ZodErrors integration~~ feat(core): Improve error formatting in ZodErrors integration Jan 22, 2025

AbhiPrasad reviewed Jan 22, 2025

View reviewed changes

jahands added 2 commits January 23, 2025 04:41

fixup! Improve saveAttachments var name

1ffaf4b

fixup! slice issues before mapping

f134135

AbhiPrasad approved these changes Jan 23, 2025

View reviewed changes

fixup! Don't truncate issues when saveZodIssuesAsAttachment=true

d92072d

fixup! formatting

f1cae8f

AbhiPrasad merged commit 965185c into getsentry:develop Jan 23, 2025
131 checks passed

HazAT mentioned this pull request Jan 23, 2025

chore: Add external contributor to CHANGELOG.md #15152

Merged

AbhiPrasad mentioned this pull request Jan 24, 2025

feat(v8/core): Improve error formatting in ZodErrors integration #15155

Merged

jahands mentioned this pull request Feb 11, 2025

chore: Export Zod errors integration and add upstream improvements robertcepa/toucan-js#255

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(core): Improve error formatting in ZodErrors integration #15111

feat(core): Improve error formatting in ZodErrors integration #15111

jahands commented Jan 21, 2025 •

edited

Loading

jahands Jan 21, 2025

AbhiPrasad Jan 22, 2025

AbhiPrasad commented Jan 21, 2025

jahands commented Jan 21, 2025

AbhiPrasad commented Jan 21, 2025

AbhiPrasad Jan 22, 2025

AbhiPrasad Jan 22, 2025

jahands Jan 23, 2025 •

edited

Loading

AbhiPrasad Jan 23, 2025

AbhiPrasad commented Jan 23, 2025

jahands commented Jan 23, 2025

jahands commented Jan 23, 2025

feat(core): Improve error formatting in ZodErrors integration #15111

feat(core): Improve error formatting in ZodErrors integration #15111

Conversation

jahands commented Jan 21, 2025 • edited Loading

Background

Screenshots

jahands Jan 21, 2025

Choose a reason for hiding this comment

AbhiPrasad Jan 22, 2025

Choose a reason for hiding this comment

AbhiPrasad commented Jan 21, 2025

jahands commented Jan 21, 2025

AbhiPrasad commented Jan 21, 2025

AbhiPrasad Jan 22, 2025

Choose a reason for hiding this comment

AbhiPrasad Jan 22, 2025

Choose a reason for hiding this comment

jahands Jan 23, 2025 • edited Loading

Choose a reason for hiding this comment

AbhiPrasad Jan 23, 2025

Choose a reason for hiding this comment

AbhiPrasad commented Jan 23, 2025

jahands commented Jan 23, 2025

jahands commented Jan 23, 2025

jahands commented Jan 21, 2025 •

edited

Loading

jahands Jan 23, 2025 •

edited

Loading