Skip to content

fix(hono): Do not capture 3xx and 4xx errors and add tests#20640

Merged
s1gr1d merged 6 commits intodevelopfrom
sig/error-tests-hono
May 5, 2026
Merged

fix(hono): Do not capture 3xx and 4xx errors and add tests#20640
s1gr1d merged 6 commits intodevelopfrom
sig/error-tests-hono

Conversation

@s1gr1d
Copy link
Copy Markdown
Member

@s1gr1d s1gr1d commented May 4, 2026

Add tests around error capturing in Hono and adds condition to not capture 3xx and 4xx errors.

Also removes some redundant tests and moves all error-related tests to errors.test.ts

@s1gr1d s1gr1d requested a review from a team as a code owner May 4, 2026 10:56
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1dfe17a. Configure here.

});

it('captures error regardless of status code', () => {
it('captures 5xx HTTPException', () => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Test doesn't verify claimed 5xx HTTPException capture behavior

Medium Severity

The test was renamed from "captures error regardless of status code" to "captures 5xx HTTPException" but the body was not updated. It still uses a plain Error('not found') (no status property) with createMockContext(404, error). Since isExpectedError checks error.status (not context.res.status), a plain Error always returns false from isExpectedError, so captureException is called regardless. The test passes coincidentally and doesn't exercise the new isExpectedError filtering at all. There's also no unit test verifying that an error with a 4xx status property is NOT captured by responseHandler.

Fix in Cursor Fix in Web

Triggered by project rule: PR Review Guidelines for Cursor Bot

Reviewed by Cursor Bugbot for commit 1dfe17a. Configure here.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

size-limit report 📦

Path Size % Change Change
@sentry/browser 26.31 kB - -
@sentry/browser - with treeshaking flags 24.8 kB - -
@sentry/browser (incl. Tracing) 44.2 kB - -
@sentry/browser (incl. Tracing + Span Streaming) 46.42 kB - -
@sentry/browser (incl. Tracing, Profiling) 49.16 kB - -
@sentry/browser (incl. Tracing, Replay) 83.58 kB - -
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 73.04 kB - -
@sentry/browser (incl. Tracing, Replay with Canvas) 88.26 kB - -
@sentry/browser (incl. Tracing, Replay, Feedback) 100.87 kB - -
@sentry/browser (incl. Feedback) 43.47 kB - -
@sentry/browser (incl. sendFeedback) 31.12 kB - -
@sentry/browser (incl. FeedbackAsync) 36.21 kB - -
@sentry/browser (incl. Metrics) 27.62 kB - -
@sentry/browser (incl. Logs) 27.75 kB - -
@sentry/browser (incl. Metrics & Logs) 28.45 kB - -
@sentry/react 28.05 kB - -
@sentry/react (incl. Tracing) 46.42 kB - -
@sentry/vue 31.18 kB - -
@sentry/vue (incl. Tracing) 46.04 kB - -
@sentry/svelte 26.34 kB - -
CDN Bundle 28.91 kB - -
CDN Bundle (incl. Tracing) 46.95 kB - -
CDN Bundle (incl. Logs, Metrics) 30.34 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) 48.06 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) 69.41 kB - -
CDN Bundle (incl. Tracing, Replay) 84.11 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) 85.16 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) 89.91 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) 91.01 kB - -
CDN Bundle - uncompressed 84.72 kB - -
CDN Bundle (incl. Tracing) - uncompressed 140.31 kB - -
CDN Bundle (incl. Logs, Metrics) - uncompressed 88.92 kB - -
CDN Bundle (incl. Tracing, Logs, Metrics) - uncompressed 143.77 kB - -
CDN Bundle (incl. Replay, Logs, Metrics) - uncompressed 212.86 kB - -
CDN Bundle (incl. Tracing, Replay) - uncompressed 258.11 kB - -
CDN Bundle (incl. Tracing, Replay, Logs, Metrics) - uncompressed 261.56 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 271.81 kB - -
CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed 275.25 kB - -
@sentry/nextjs (client) 48.92 kB - -
@sentry/sveltekit (client) 44.67 kB - -
@sentry/node-core 59.13 kB +0.02% +9 B 🔺
@sentry/node 170.57 kB +0.01% +12 B 🔺
@sentry/node - without tracing 97.17 kB +0.01% +7 B 🔺
@sentry/aws-serverless 113.99 kB +0.03% +31 B 🔺
@sentry/cloudflare (withSentry) - minified 165.2 kB - -
@sentry/cloudflare (withSentry) 417.71 kB - -

View base workflow run

Comment thread packages/hono/src/shared/wrapMiddlewareSpan.ts

expect(errorEvent.exception?.values?.[0]?.value).toBe('Error caught by custom onError');
expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({
handled: false,
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not a 100% sure on this one, but according to our develop docs, it's not handled as this was captured by a global error handler (onError).
https://develop.sentry.dev/sdk/telemetry/errors/#concepts

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, should be unhandled 👍

Copy link
Copy Markdown
Member

@chargome chargome 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!

@s1gr1d s1gr1d merged commit 6d0ebc4 into develop May 5, 2026
256 checks passed
@s1gr1d s1gr1d deleted the sig/error-tests-hono branch May 5, 2026 07:38
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.

2 participants