From edbc94abf9cc2edbc0f3806ebd2e6283aeac92f2 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Thu, 30 Apr 2026 15:13:51 +0200 Subject: [PATCH 1/5] fix(hono): Do not capture 3xx and 4xx errors and add tests --- .../hono-4/src/route-groups/test-errors.ts | 56 +++ .../test-applications/hono-4/src/routes.ts | 40 ++ .../hono-4/tests/errors.test.ts | 387 ++++++++++++++++-- .../hono-4/tests/middleware.test.ts | 24 +- .../hono-4/tests/route-patterns.test.ts | 62 +-- .../hono-4/tests/tracing.test.ts | 22 +- packages/hono/src/shared/isExpectedError.ts | 17 + .../hono/src/shared/middlewareHandlers.ts | 3 +- .../hono/src/shared/wrapMiddlewareSpan.ts | 11 +- .../hono/test/shared/isExpectedError.test.ts | 78 ++++ .../test/shared/middlewareHandlers.test.ts | 4 +- 11 files changed, 622 insertions(+), 82 deletions(-) create mode 100644 dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-errors.ts create mode 100644 packages/hono/src/shared/isExpectedError.ts create mode 100644 packages/hono/test/shared/isExpectedError.test.ts diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-errors.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-errors.ts new file mode 100644 index 000000000000..cf00c8d8df87 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-errors.ts @@ -0,0 +1,56 @@ +import { Hono } from 'hono'; +import { HTTPException } from 'hono/http-exception'; + +const errorRoutes = new Hono(); + +// Middleware that throws a 5xx HTTPException (should be captured) +errorRoutes.use('/middleware-http-exception/*', async (_c, _next) => { + throw new HTTPException(503, { message: 'Service Unavailable from middleware' }); +}); + +errorRoutes.get('/middleware-http-exception', c => c.text('should not reach')); + +// Middleware that throws a 4xx HTTPException (should NOT be captured) +errorRoutes.use('/middleware-http-exception-4xx/*', async (_c, _next) => { + throw new HTTPException(401, { message: 'Unauthorized from middleware' }); +}); + +errorRoutes.get('/middleware-http-exception-4xx', c => c.text('should not reach')); + +// Sub-app with a custom onError handler that swallows errors +const subAppWithOnError = new Hono(); + +subAppWithOnError.onError((err, c) => { + return c.text(`Handled by onError: ${err.message}`, 500); +}); + +subAppWithOnError.get('/fail', () => { + throw new Error('Error caught by custom onError'); +}); + +errorRoutes.route('/custom-on-error', subAppWithOnError); + +// Nested sub-apps: parent mounts child, child route throws +const childApp = new Hono(); + +childApp.get('/error', () => { + throw new Error('Nested child app error'); +}); + +childApp.get('/deep/error', () => { + throw new Error('Deeply nested child app error'); +}); + +const parentApp = new Hono(); +parentApp.route('/child', childApp); + +errorRoutes.route('/nested', parentApp); + +// Route that throws after partial response setup +errorRoutes.get('/partial-response-error', c => { + c.header('X-Custom-Header', 'partial'); + c.status(200); + throw new Error('Error after partial response setup'); +}); + +export { errorRoutes }; diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts index f6efc6dde03c..abcc9e1887df 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts @@ -1,6 +1,7 @@ import type { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; import { failingMiddleware, middlewareA, middlewareB } from './middleware'; +import { errorRoutes } from './route-groups/test-errors'; import { middlewareRoutes, subAppWithInlineMiddleware, subAppWithMiddleware } from './route-groups/test-middleware'; import { routePatterns } from './route-groups/test-route-patterns'; @@ -13,6 +14,22 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v return c.json({ paramId: c.req.param('paramId') }); }); + app.get('/error/async', async () => { + await new Promise(resolve => setTimeout(resolve, 10)); + throw new Error('Async route error'); + }); + + app.get('/error/non-error-throw', () => { + // eslint-disable-next-line no-throw-literal + throw 'Non-Error thrown value'; + }); + + app.get('/error/nested-cause', () => { + const rootCause = new Error('Database connection failed'); + const intermediateCause = new Error('Query execution failed', { cause: rootCause }); + throw new Error('Request handler failed', { cause: intermediateCause }); + }); + app.get('/error/:cause', c => { throw new Error('This is a test error for Sentry!', { cause: c.req.param('cause'), @@ -25,6 +42,26 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v throw new HTTPException(code, { message: `HTTPException ${code}` }); }); + app.get('/redirect/301', c => { + return c.redirect('/', 301); + }); + + app.get('/redirect/302', c => { + return c.redirect('/', 302); + }); + + app.get('/status/400', c => { + return c.text('Bad Request', 400); + }); + + app.get('/status/403', c => { + return c.text('Forbidden', 403); + }); + + app.get('/status/404', c => { + return c.text('Not Found', 404); + }); + // Root-app middleware: registered on the patched main app instance app.use('/test-middleware/named/*', middlewareA); app.use('/test-middleware/anonymous/*', async (c, next) => { @@ -43,4 +80,7 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v // Route patterns: HTTP methods, .all(), .on(), sync/async, errors app.route('/test-routes', routePatterns); + + // Error-specific routes: onError handler, nested sub-apps, middleware HTTPException + app.route('/test-errors', errorRoutes); } diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts index 832204237946..ec14f65ce087 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts @@ -1,53 +1,378 @@ import { expect, test } from '@playwright/test'; -import { waitForError } from '@sentry-internal/test-utils'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; import { APP_NAME } from './constants'; -test('captures error thrown in route handler', async ({ baseURL }) => { - const errorWaiter = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'This is a test error for Sentry!'; +test.describe('route handler errors', () => { + test('captures error with full event shape and trace correlation', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'This is a test error for Sentry!'; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/error/'); + }); + + const response = await fetch(`${baseURL}/error/test-cause`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + const transactionEvent = await transactionPromise; + + expect(transactionEvent.transaction).toBe('GET /error/:cause'); + + expect(errorEvent.exception?.values).toHaveLength(1); + + const exception = errorEvent.exception?.values?.[0]; + expect(exception?.value).toBe('This is a test error for Sentry!'); + expect(exception?.mechanism).toEqual({ + handled: false, + type: 'auto.http.hono.context_error', + }); + + expect(errorEvent.transaction).toBe('GET /error/:cause'); + expect(errorEvent.request?.method).toBe('GET'); + expect(errorEvent.request?.url).toContain('/error/test-cause'); + + expect(errorEvent.contexts?.trace?.trace_id).toBe(transactionEvent.contexts?.trace?.trace_id); + }); + + test('captures async route handler error', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Async route error'; + }); + + const response = await fetch(`${baseURL}/error/async`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toBe('Async route error'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ + handled: false, + type: 'auto.http.hono.context_error', + }); + }); + + test('captures non-Error thrown value', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Non-Error thrown value'; + }); + + const response = await fetch(`${baseURL}/error/non-error-throw`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toBe('Non-Error thrown value'); + }); + + test('captures error with nested cause chain', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.some(v => v.value === 'Request handler failed') ?? false; + }); + + const response = await fetch(`${baseURL}/error/nested-cause`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + const values = errorEvent.exception?.values ?? []; + expect(values.length).toBeGreaterThanOrEqual(1); + + const topError = values.find(v => v.value === 'Request handler failed'); + expect(topError).toBeDefined(); }); - const response = await fetch(`${baseURL}/error/test-cause`); - expect(response.status).toBe(500); + test('captures error thrown after partial response setup', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Error after partial response setup'; + }); - const event = await errorWaiter; - expect(event.exception?.values?.[0]?.value).toBe('This is a test error for Sentry!'); + const response = await fetch(`${baseURL}/test-errors/partial-response-error`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toBe('Error after partial response setup'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ + handled: false, + type: 'auto.http.hono.context_error', + }); + }); }); -test('captures HTTPException with 502 status', async ({ baseURL }) => { - const errorWaiter = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'HTTPException 502'; +test.describe('HTTPException errors', () => { + test('captures HTTPException with 500 status', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'HTTPException 500'; + }); + + const response = await fetch(`${baseURL}/http-exception/500`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toBe('HTTPException 500'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ + handled: false, + type: 'auto.http.hono.context_error', + }); }); - const response = await fetch(`${baseURL}/http-exception/502`); - expect(response.status).toBe(502); + test('captures HTTPException with 502 status', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'HTTPException 502'; + }); + + const response = await fetch(`${baseURL}/http-exception/502`); + expect(response.status).toBe(502); + + const errorEvent = await errorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toBe('HTTPException 502'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ + handled: false, + type: 'auto.http.hono.context_error', + }); + }); + + [401, 403, 404].forEach(code => { + test(`does not capture ${code} HTTPException`, async ({ baseURL }) => { + let errorEventOccurred = false; + + waitForError(APP_NAME, event => { + if (event.exception?.values?.[0]?.value === `HTTPException ${code}`) { + errorEventOccurred = true; + } + return false; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); + }); + + const response = await fetch(`${baseURL}/http-exception/${code}`); + expect(response.status).toBe(code); - const event = await errorWaiter; - expect(event.exception?.values?.[0]?.value).toBe('HTTPException 502'); + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /http-exception/:code'); + expect(errorEventOccurred).toBe(false); + }); + }); }); -// TODO: 401 and 404 HTTPExceptions should not be captured by Sentry by default, -// but currently they are. Fix the filtering and update these tests accordingly. -test('captures HTTPException with 401 status', async ({ baseURL }) => { - const errorWaiter = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'HTTPException 401'; +test.describe('middleware errors', () => { + test('captures 5xx HTTPException thrown in middleware', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Service Unavailable from middleware'; + }); + + const response = await fetch(`${baseURL}/test-errors/middleware-http-exception`); + expect(response.status).toBe(503); + + const errorEvent = await errorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toBe('Service Unavailable from middleware'); + expect(errorEvent.exception?.values?.[0]?.mechanism?.type).toBe('auto.middleware.hono'); + expect(errorEvent.exception?.values?.[0]?.mechanism?.handled).toBe(false); }); - const response = await fetch(`${baseURL}/http-exception/401`); - expect(response.status).toBe(401); + test('does not capture 4xx HTTPException thrown in middleware', async ({ baseURL }) => { + let errorEventOccurred = false; + + waitForError(APP_NAME, event => { + if (event.exception?.values?.[0]?.value === 'Unauthorized from middleware') { + errorEventOccurred = true; + } + return false; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + !!event.transaction?.includes('/test-errors/middleware-http-exception-4xx') + ); + }); - const event = await errorWaiter; - expect(event.exception?.values?.[0]?.value).toBe('HTTPException 401'); + const response = await fetch(`${baseURL}/test-errors/middleware-http-exception-4xx`); + expect(response.status).toBe(401); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /test-errors/middleware-http-exception-4xx/*'); + expect(errorEventOccurred).toBe(false); + }); }); -test('captures HTTPException with 404 status', async ({ baseURL }) => { - const errorWaiter = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'HTTPException 404'; +test.describe('nested sub-app errors', () => { + test('captures error from nested child sub-app', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Nested child app error'; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/nested/child/error'); + }); + + const response = await fetch(`${baseURL}/test-errors/nested/child/error`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + const transaction = await transactionPromise; + + expect(transaction.transaction).toBe('GET /test-errors/nested/child/error'); + + expect(errorEvent.exception?.values?.[0]?.value).toBe('Nested child app error'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ + handled: false, + type: 'auto.http.hono.context_error', + }); + expect(errorEvent.request?.url).toContain('/test-errors/nested/child/error'); }); - const response = await fetch(`${baseURL}/http-exception/404`); - expect(response.status).toBe(404); + test('captures error from deeply nested sub-app route', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Deeply nested child app error'; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/nested/child/deep/error'); + }); + + const response = await fetch(`${baseURL}/test-errors/nested/child/deep/error`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + const transaction = await transactionPromise; + + expect(transaction.transaction).toBe('GET /test-errors/nested/child/deep/error'); + + expect(errorEvent.exception?.values?.[0]?.value).toBe('Deeply nested child app error'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ + handled: false, + type: 'auto.http.hono.context_error', + }); + }); +}); + +test.describe('custom onError handler', () => { + test('captures error even when onError handles the response', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Error caught by custom onError'; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/custom-on-error/fail'); + }); + + const response = await fetch(`${baseURL}/test-errors/custom-on-error/fail`); + expect(response.status).toBe(500); + + const body = await response.text(); + expect(body).toContain('Handled by onError'); + + const errorEvent = await errorPromise; + const transaction = await transactionPromise; + + expect(transaction.transaction).toBe('GET /test-errors/custom-on-error/fail'); + + expect(errorEvent.exception?.values?.[0]?.value).toBe('Error caught by custom onError'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ + handled: false, + type: 'auto.http.hono.context_error', + }); + }); +}); + +test.describe('no error capture for non-error responses', () => { + [ + { description: '301 redirect', path: '/redirect/301', expectedStatus: 301 }, + { description: '302 redirect', path: '/redirect/302', expectedStatus: 302 }, + ].forEach(({ description, path, expectedStatus }) => { + test(`does not capture error for ${description}`, async ({ baseURL }) => { + let errorEventOccurred = false; + + waitForError(APP_NAME, event => { + if (event.request?.url?.includes(path)) { + errorEventOccurred = true; + } + return false; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(path); + }); - const event = await errorWaiter; - expect(event.exception?.values?.[0]?.value).toBe('HTTPException 404'); + const response = await fetch(`${baseURL}${path}`, { redirect: 'manual' }); + expect(response.status).toBe(expectedStatus); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${path}`); + expect(errorEventOccurred).toBe(false); + }); + }); + + [ + { description: '400 status without throw', path: '/status/400', expectedStatus: 400 }, + { description: '403 status without throw', path: '/status/403', expectedStatus: 403 }, + { description: '404 status without throw', path: '/status/404', expectedStatus: 404 }, + ].forEach(({ description, path, expectedStatus }) => { + test(`does not capture error for ${description}`, async ({ baseURL }) => { + let errorEventOccurred = false; + + waitForError(APP_NAME, event => { + if (event.request?.url?.includes(path)) { + errorEventOccurred = true; + } + return false; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(path); + }); + + const response = await fetch(`${baseURL}${path}`); + expect(response.status).toBe(expectedStatus); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${path}`); + expect(errorEventOccurred).toBe(false); + }); + }); + + test('does not capture error for non-existent route (404)', async ({ baseURL }) => { + let errorEventOccurred = false; + + waitForError(APP_NAME, event => { + if (event.request?.url?.includes('/this-route-does-not-exist')) { + errorEventOccurred = true; + } + return false; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/this-route-does-not-exist'); + }); + + const response = await fetch(`${baseURL}/this-route-does-not-exist`); + expect(response.status).toBe(404); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /this-route-does-not-exist'); + expect(errorEventOccurred).toBe(false); + }); + + test('does not capture error for successful 200 response', async ({ baseURL }) => { + let errorEventOccurred = false; + + waitForError(APP_NAME, event => { + if (event.request?.url?.includes('/?sentry-test-no-error')) { + errorEventOccurred = true; + } + return false; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('GET /'); + }); + + const response = await fetch(`${baseURL}/?sentry-test-no-error`); + expect(response.status).toBe(200); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /'); + expect(errorEventOccurred).toBe(false); + }); }); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts index e8431bed67ce..613ffc4af772 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts @@ -18,13 +18,15 @@ for (const { name, prefix } of SCENARIOS) { test.describe(name, () => { test('creates a span for named middleware', async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${prefix}/named`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${prefix}/named`); }); const response = await fetch(`${baseURL}${prefix}/named`); expect(response.status).toBe(200); const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${prefix}/named`); + const spans = transaction.spans || []; const middlewareSpan = spans.find( @@ -48,13 +50,15 @@ for (const { name, prefix } of SCENARIOS) { test('creates a span for anonymous middleware', async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${prefix}/anonymous`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${prefix}/anonymous`); }); const response = await fetch(`${baseURL}${prefix}/anonymous`); expect(response.status).toBe(200); const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${prefix}/anonymous`); + const spans = transaction.spans || []; expect(spans).toContainEqual( @@ -69,13 +73,15 @@ for (const { name, prefix } of SCENARIOS) { test('multiple middleware are sibling spans under the same parent', async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${prefix}/multi`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${prefix}/multi`); }); const response = await fetch(`${baseURL}${prefix}/multi`); expect(response.status).toBe(200); const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${prefix}/multi`); + const spans = transaction.spans || []; const middlewareSpans = spans.sort((a, b) => (a.start_timestamp ?? 0) - (b.start_timestamp ?? 0)); @@ -115,12 +121,14 @@ for (const { name, prefix } of SCENARIOS) { test('sets error status on middleware span when middleware throws', async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${prefix}/error/*`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${prefix}/error`); }); await fetch(`${baseURL}${prefix}/error`); const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${prefix}/error/*`); + const spans = transaction.spans || []; const failingSpan = spans.find( @@ -153,7 +161,8 @@ test.describe('.all() handler in sub-app', () => { test('does not create middleware span for .all() route handler', async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { return ( - event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /test-subapp-middleware/all-handler' + event.contexts?.trace?.op === 'http.server' && + !!event.transaction?.includes('/test-subapp-middleware/all-handler') ); }); @@ -164,6 +173,8 @@ test.describe('.all() handler in sub-app', () => { expect(body).toEqual({ handler: 'all' }); const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /test-subapp-middleware/all-handler'); + const spans = transaction.spans || []; // No middleware is called for this route, so there should be no spans. @@ -191,13 +202,14 @@ test.describe('inline middleware spans (sub-app)', () => { const fullPath = `${INLINE_PREFIX}${regPath}${mwPath}`; const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${fullPath}`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(fullPath); }); const response = await fetch(`${baseURL}${fullPath}`); expect(response.status).toBe(200); const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${fullPath}`); const EXPECTED_DESCRIPTIONS: Record> = { '/direct': { '': 'inlineMiddleware', '/separately': 'inlineSeparateMiddleware' }, diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts index fd6579fe3b17..15c6869d91d8 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts @@ -11,45 +11,45 @@ const REGISTRATION_STYLES = [ ] as const; test.describe('HTTP methods', () => { - for (const method of ['POST', 'PUT', 'DELETE', 'PATCH']) { + ['POST', 'PUT', 'DELETE', 'PATCH'].forEach(method => { test(`sends transaction for ${method}`, async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `${method} ${PREFIX}`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(PREFIX); }); const response = await fetch(`${baseURL}${PREFIX}`, { method }); expect(response.status).toBe(200); const transaction = await transactionPromise; - expect(transaction.contexts?.trace?.op).toBe('http.server'); expect(transaction.transaction).toBe(`${method} ${PREFIX}`); + expect(transaction.contexts?.trace?.op).toBe('http.server'); }); - } + }); }); test.describe('route registration styles', () => { - for (const { name, path } of REGISTRATION_STYLES) { + REGISTRATION_STYLES.forEach(({ name, path }) => { test(`${name} sends transaction`, async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${PREFIX}${path}`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${PREFIX}${path}`); }); const response = await fetch(`${baseURL}${PREFIX}${path}`); expect(response.status).toBe(200); const transaction = await transactionPromise; - expect(transaction.contexts?.trace?.op).toBe('http.server'); expect(transaction.transaction).toBe(`GET ${PREFIX}${path}`); + expect(transaction.contexts?.trace?.op).toBe('http.server'); }); - } + }); - for (const { name, path } of [ + [ { name: '.all()', path: '/all' }, { name: '.on()', path: '/on' }, - ]) { + ].forEach(({ name, path }) => { test(`${name} responds to POST`, async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `POST ${PREFIX}${path}`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${PREFIX}${path}`); }); const response = await fetch(`${baseURL}${PREFIX}${path}`, { method: 'POST' }); @@ -58,23 +58,24 @@ test.describe('route registration styles', () => { const transaction = await transactionPromise; expect(transaction.transaction).toBe(`POST ${PREFIX}${path}`); }); - } + }); }); test('async handler sends transaction', async ({ baseURL }) => { const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && event.transaction === `GET ${PREFIX}/async`; + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${PREFIX}/async`); }); const response = await fetch(`${baseURL}${PREFIX}/async`); expect(response.status).toBe(200); const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${PREFIX}/async`); expect(transaction.contexts?.trace?.op).toBe('http.server'); }); test.describe('500 HTTPException capture', () => { - for (const { name, path } of REGISTRATION_STYLES) { + REGISTRATION_STYLES.forEach(({ name, path }) => { test(`captures 500 from ${name} route with correct mechanism`, async ({ baseURL }) => { const fullPath = `${PREFIX}${path}/500`; @@ -94,7 +95,7 @@ test.describe('500 HTTPException capture', () => { }), ); }); - } + }); test('captures 500 error with POST method', async ({ baseURL }) => { const errorPromise = waitForError(APP_NAME, event => { @@ -119,26 +120,29 @@ test.describe('500 HTTPException capture', () => { }); }); -test.describe('4xx HTTPException capture', () => { - for (const code of [401, 402, 403]) { - test(`captures ${code} HTTPException`, async ({ baseURL }) => { +test.describe('4xx HTTPException filtering', () => { + [401, 402, 403].forEach(code => { + test(`does not capture ${code} HTTPException`, async ({ baseURL }) => { const fullPath = `${PREFIX}/${code}`; + let errorEventOccurred = false; - const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === `response ${code}` && !!event.request?.url?.includes(fullPath); + waitForError(APP_NAME, event => { + if (event.exception?.values?.[0]?.value === `response ${code}` && event.request?.url?.includes(fullPath)) { + errorEventOccurred = true; + } + return false; + }); + + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${PREFIX}/${code}`); }); const response = await fetch(`${baseURL}${fullPath}`); expect(response.status).toBe(code); - const errorEvent = await errorPromise; - expect(errorEvent.exception?.values?.[0]?.value).toBe(`response ${code}`); - expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual( - expect.objectContaining({ - handled: false, - type: 'auto.http.hono.context_error', - }), - ); + const transaction = await transactionPromise; + expect(transaction.transaction).toBe(`GET ${PREFIX}/${code}`); + expect(errorEventOccurred).toBe(false); }); - } + }); }); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts index 1c33943f38f8..4d9644312913 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/tracing.test.ts @@ -3,38 +3,40 @@ import { waitForTransaction } from '@sentry-internal/test-utils'; import { APP_NAME } from './constants'; test('sends a transaction for the index route', async ({ baseURL }) => { - const transactionWaiter = waitForTransaction(APP_NAME, event => { - return event.transaction === 'GET /'; + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /'; }); const response = await fetch(`${baseURL}/`); expect(response.status).toBe(200); - const transaction = await transactionWaiter; + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /'); expect(transaction.contexts?.trace?.op).toBe('http.server'); }); test('sends a transaction for a parameterized route', async ({ baseURL }) => { - const transactionWaiter = waitForTransaction(APP_NAME, event => { - return event.transaction === 'GET /test-param/:paramId'; + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/test-param/'); }); const response = await fetch(`${baseURL}/test-param/123`); expect(response.status).toBe(200); - const transaction = await transactionWaiter; - expect(transaction.contexts?.trace?.op).toBe('http.server'); + const transaction = await transactionPromise; expect(transaction.transaction).toBe('GET /test-param/:paramId'); + expect(transaction.contexts?.trace?.op).toBe('http.server'); }); test('sends a transaction for a route that throws', async ({ baseURL }) => { - const transactionWaiter = waitForTransaction(APP_NAME, event => { - return event.transaction === 'GET /error/:cause'; + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/error/'); }); await fetch(`${baseURL}/error/test-cause`); - const transaction = await transactionWaiter; + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /error/:cause'); expect(transaction.contexts?.trace?.op).toBe('http.server'); expect(transaction.contexts?.trace?.status).toBe('internal_error'); }); diff --git a/packages/hono/src/shared/isExpectedError.ts b/packages/hono/src/shared/isExpectedError.ts new file mode 100644 index 000000000000..f3014be0fb5e --- /dev/null +++ b/packages/hono/src/shared/isExpectedError.ts @@ -0,0 +1,17 @@ +/** + * 3xx and 4xx errors are expected (redirects, auth failures, not found, bad + * request) and should not be captured as Sentry error events. + * + * Checks any error-like value that carries a numeric `status` property — this + * covers Hono's `HTTPException`, third-party middleware errors, and custom + * error subclasses. + */ +export function isExpectedError(error: unknown): boolean { + if (typeof error !== 'object' || error === null) { + return false; + } + + const status = (error as { status?: unknown }).status; + + return typeof status === 'number' && status >= 300 && status < 500; +} diff --git a/packages/hono/src/shared/middlewareHandlers.ts b/packages/hono/src/shared/middlewareHandlers.ts index a470733b47a8..41902d90f84f 100644 --- a/packages/hono/src/shared/middlewareHandlers.ts +++ b/packages/hono/src/shared/middlewareHandlers.ts @@ -11,6 +11,7 @@ import { import type { Context } from 'hono'; import { routePath } from 'hono/route'; import { hasFetchEvent } from '../utils/hono-context'; +import { isExpectedError } from './isExpectedError'; /** * Request handler for Hono framework @@ -42,7 +43,7 @@ export function responseHandler(context: Context): void { getIsolationScope().setTransactionName(`${context.req.method} ${routePath(context)}`); - if (context.error) { + if (context.error && !isExpectedError(context.error)) { getClient()?.captureException(context.error, { mechanism: { handled: false, type: 'auto.http.hono.context_error' }, }); diff --git a/packages/hono/src/shared/wrapMiddlewareSpan.ts b/packages/hono/src/shared/wrapMiddlewareSpan.ts index b93e5de0bded..7f2ff5ac6d94 100644 --- a/packages/hono/src/shared/wrapMiddlewareSpan.ts +++ b/packages/hono/src/shared/wrapMiddlewareSpan.ts @@ -12,6 +12,7 @@ import { type WrappedFunction, } from '@sentry/core'; import { type MiddlewareHandler } from 'hono'; +import { isExpectedError } from './isExpectedError'; const MIDDLEWARE_ORIGIN = 'auto.middleware.hono'; @@ -46,9 +47,13 @@ export function wrapMiddlewareWithSpan(handler: MiddlewareHandler): MiddlewareHa return result; } catch (error) { span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - captureException(error, { - mechanism: { handled: false, type: MIDDLEWARE_ORIGIN }, - }); + + if (!isExpectedError(error)) { + captureException(error, { + mechanism: { handled: false, type: MIDDLEWARE_ORIGIN }, + }); + } + throw error; } finally { span.end(); diff --git a/packages/hono/test/shared/isExpectedError.test.ts b/packages/hono/test/shared/isExpectedError.test.ts new file mode 100644 index 000000000000..235db79e1357 --- /dev/null +++ b/packages/hono/test/shared/isExpectedError.test.ts @@ -0,0 +1,78 @@ +import { HTTPException } from 'hono/http-exception'; +import { describe, expect, it } from 'vitest'; +import { isExpectedError } from '../../src/shared/isExpectedError'; + +describe('isExpectedError', () => { + describe('HTTPException', () => { + it('returns true for 4xx HTTPException', () => { + expect(isExpectedError(new HTTPException(400, { message: 'Bad Request' }))).toBe(true); + expect(isExpectedError(new HTTPException(401, { message: 'Unauthorized' }))).toBe(true); + expect(isExpectedError(new HTTPException(403, { message: 'Forbidden' }))).toBe(true); + expect(isExpectedError(new HTTPException(404, { message: 'Not Found' }))).toBe(true); + expect(isExpectedError(new HTTPException(422, { message: 'Unprocessable Entity' }))).toBe(true); + expect(isExpectedError(new HTTPException(499))).toBe(true); + }); + + it('returns false for 5xx HTTPException', () => { + expect(isExpectedError(new HTTPException(500, { message: 'Internal Server Error' }))).toBe(false); + expect(isExpectedError(new HTTPException(502, { message: 'Bad Gateway' }))).toBe(false); + expect(isExpectedError(new HTTPException(503, { message: 'Service Unavailable' }))).toBe(false); + }); + }); + + describe('custom error classes with status property', () => { + it('returns true for custom Error subclass with 4xx status', () => { + class AuthError extends Error { + status = 401; + } + expect(isExpectedError(new AuthError('unauthorized'))).toBe(true); + }); + + it('returns false for custom Error subclass with 5xx status', () => { + class DbError extends Error { + status = 500; + } + expect(isExpectedError(new DbError('connection lost'))).toBe(false); + }); + + it('returns true for plain object with 4xx status', () => { + expect(isExpectedError({ status: 404, message: 'Not Found' })).toBe(true); + expect(isExpectedError({ status: 400 })).toBe(true); + }); + + it('returns false for plain object with 5xx status', () => { + expect(isExpectedError({ status: 500, message: 'Internal Server Error' })).toBe(false); + }); + }); + + describe('non-expected errors', () => { + it('returns false for plain Error without status', () => { + expect(isExpectedError(new Error('something broke'))).toBe(false); + }); + + it('returns false for non-object values', () => { + expect(isExpectedError('string error')).toBe(false); + expect(isExpectedError(42)).toBe(false); + expect(isExpectedError(null)).toBe(false); + expect(isExpectedError(undefined)).toBe(false); + expect(isExpectedError(true)).toBe(false); + }); + + it('returns false when status is not a number', () => { + expect(isExpectedError({ status: '404' })).toBe(false); + expect(isExpectedError({ status: null })).toBe(false); + expect(isExpectedError({ status: undefined })).toBe(false); + }); + + it('returns true for 3xx status', () => { + expect(isExpectedError({ status: 301 })).toBe(true); + expect(isExpectedError({ status: 302 })).toBe(true); + expect(isExpectedError({ status: 399 })).toBe(true); + }); + + it('returns false for 2xx status', () => { + expect(isExpectedError({ status: 200 })).toBe(false); + expect(isExpectedError({ status: 299 })).toBe(false); + }); + }); +}); diff --git a/packages/hono/test/shared/middlewareHandlers.test.ts b/packages/hono/test/shared/middlewareHandlers.test.ts index 83099370320c..4d166da7854f 100644 --- a/packages/hono/test/shared/middlewareHandlers.test.ts +++ b/packages/hono/test/shared/middlewareHandlers.test.ts @@ -1,4 +1,5 @@ import * as SentryCore from '@sentry/core'; +import { HTTPException } from 'hono/http-exception'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { responseHandler } from '../../src/shared/middlewareHandlers'; @@ -55,7 +56,7 @@ describe('responseHandler', () => { }); }); - it('captures error regardless of status code', () => { + it('captures 5xx HTTPException', () => { const mockCaptureException = vi.fn(); getClientMock.mockReturnValue({ captureException: mockCaptureException, @@ -101,7 +102,6 @@ describe('responseHandler', () => { // oxlint-disable-next-line typescript/no-explicit-any responseHandler(createMockContext(500, error) as any); - // captureException is called — it handles deduplication internally via checkOrSetAlreadyCaught expect(mockCaptureException).toHaveBeenCalledWith(error, { mechanism: { handled: false, type: 'auto.http.hono.context_error' }, }); From d5b31ccb1d6a10ad2d02af2179d73a385f26372e Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 4 May 2026 11:59:51 +0200 Subject: [PATCH 2/5] remove duplicate tests --- .../hono-4/src/route-groups/test-errors.ts | 11 - .../test-applications/hono-4/src/routes.ts | 36 --- .../hono-4/tests/errors.test.ts | 251 +++--------------- .../test/shared/middlewareHandlers.test.ts | 1 - 4 files changed, 35 insertions(+), 264 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-errors.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-errors.ts index cf00c8d8df87..b8f2fd96fe93 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-errors.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-errors.ts @@ -37,20 +37,9 @@ childApp.get('/error', () => { throw new Error('Nested child app error'); }); -childApp.get('/deep/error', () => { - throw new Error('Deeply nested child app error'); -}); - const parentApp = new Hono(); parentApp.route('/child', childApp); errorRoutes.route('/nested', parentApp); -// Route that throws after partial response setup -errorRoutes.get('/partial-response-error', c => { - c.header('X-Custom-Header', 'partial'); - c.status(200); - throw new Error('Error after partial response setup'); -}); - export { errorRoutes }; diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts index abcc9e1887df..cfb13146b6f7 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts @@ -14,22 +14,6 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v return c.json({ paramId: c.req.param('paramId') }); }); - app.get('/error/async', async () => { - await new Promise(resolve => setTimeout(resolve, 10)); - throw new Error('Async route error'); - }); - - app.get('/error/non-error-throw', () => { - // eslint-disable-next-line no-throw-literal - throw 'Non-Error thrown value'; - }); - - app.get('/error/nested-cause', () => { - const rootCause = new Error('Database connection failed'); - const intermediateCause = new Error('Query execution failed', { cause: rootCause }); - throw new Error('Request handler failed', { cause: intermediateCause }); - }); - app.get('/error/:cause', c => { throw new Error('This is a test error for Sentry!', { cause: c.req.param('cause'), @@ -42,26 +26,6 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v throw new HTTPException(code, { message: `HTTPException ${code}` }); }); - app.get('/redirect/301', c => { - return c.redirect('/', 301); - }); - - app.get('/redirect/302', c => { - return c.redirect('/', 302); - }); - - app.get('/status/400', c => { - return c.text('Bad Request', 400); - }); - - app.get('/status/403', c => { - return c.text('Forbidden', 403); - }); - - app.get('/status/404', c => { - return c.text('Not Found', 404); - }); - // Root-app middleware: registered on the patched main app instance app.use('/test-middleware/named/*', middlewareA); app.use('/test-middleware/anonymous/*', async (c, next) => { diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts index ec14f65ce087..3e406814a509 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts @@ -3,7 +3,7 @@ import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; import { APP_NAME } from './constants'; test.describe('route handler errors', () => { - test('captures error with full event shape and trace correlation', async ({ baseURL }) => { + test('captures error with mechanism and trace correlation', async ({ baseURL }) => { const errorPromise = waitForError(APP_NAME, event => { return event.exception?.values?.[0]?.value === 'This is a test error for Sentry!'; }); @@ -35,70 +35,10 @@ test.describe('route handler errors', () => { expect(errorEvent.contexts?.trace?.trace_id).toBe(transactionEvent.contexts?.trace?.trace_id); }); - - test('captures async route handler error', async ({ baseURL }) => { - const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'Async route error'; - }); - - const response = await fetch(`${baseURL}/error/async`); - expect(response.status).toBe(500); - - const errorEvent = await errorPromise; - expect(errorEvent.exception?.values?.[0]?.value).toBe('Async route error'); - expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ - handled: false, - type: 'auto.http.hono.context_error', - }); - }); - - test('captures non-Error thrown value', async ({ baseURL }) => { - const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'Non-Error thrown value'; - }); - - const response = await fetch(`${baseURL}/error/non-error-throw`); - expect(response.status).toBe(500); - - const errorEvent = await errorPromise; - expect(errorEvent.exception?.values?.[0]?.value).toBe('Non-Error thrown value'); - }); - - test('captures error with nested cause chain', async ({ baseURL }) => { - const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.some(v => v.value === 'Request handler failed') ?? false; - }); - - const response = await fetch(`${baseURL}/error/nested-cause`); - expect(response.status).toBe(500); - - const errorEvent = await errorPromise; - const values = errorEvent.exception?.values ?? []; - expect(values.length).toBeGreaterThanOrEqual(1); - - const topError = values.find(v => v.value === 'Request handler failed'); - expect(topError).toBeDefined(); - }); - - test('captures error thrown after partial response setup', async ({ baseURL }) => { - const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'Error after partial response setup'; - }); - - const response = await fetch(`${baseURL}/test-errors/partial-response-error`); - expect(response.status).toBe(500); - - const errorEvent = await errorPromise; - expect(errorEvent.exception?.values?.[0]?.value).toBe('Error after partial response setup'); - expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ - handled: false, - type: 'auto.http.hono.context_error', - }); - }); }); test.describe('HTTPException errors', () => { - test('captures HTTPException with 500 status', async ({ baseURL }) => { + test('captures 5xx HTTPException', async ({ baseURL }) => { const errorPromise = waitForError(APP_NAME, event => { return event.exception?.values?.[0]?.value === 'HTTPException 500'; }); @@ -114,44 +54,48 @@ test.describe('HTTPException errors', () => { }); }); - test('captures HTTPException with 502 status', async ({ baseURL }) => { - const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'HTTPException 502'; - }); + test('does not capture 3xx HTTPException', async ({ baseURL }) => { + let errorEventOccurred = false; - const response = await fetch(`${baseURL}/http-exception/502`); - expect(response.status).toBe(502); + waitForError(APP_NAME, event => { + if (event.exception?.values?.[0]?.value === 'HTTPException 301') { + errorEventOccurred = true; + } + return false; + }); - const errorEvent = await errorPromise; - expect(errorEvent.exception?.values?.[0]?.value).toBe('HTTPException 502'); - expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ - handled: false, - type: 'auto.http.hono.context_error', + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); }); - }); - [401, 403, 404].forEach(code => { - test(`does not capture ${code} HTTPException`, async ({ baseURL }) => { - let errorEventOccurred = false; + const response = await fetch(`${baseURL}/http-exception/301`, { redirect: 'manual' }); + expect(response.status).toBe(301); - waitForError(APP_NAME, event => { - if (event.exception?.values?.[0]?.value === `HTTPException ${code}`) { - errorEventOccurred = true; - } - return false; - }); + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /http-exception/:code'); + expect(errorEventOccurred).toBe(false); + }); - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); - }); + test('does not capture 4xx HTTPException', async ({ baseURL }) => { + let errorEventOccurred = false; - const response = await fetch(`${baseURL}/http-exception/${code}`); - expect(response.status).toBe(code); + waitForError(APP_NAME, event => { + if (event.exception?.values?.[0]?.value === 'HTTPException 404') { + errorEventOccurred = true; + } + return false; + }); - const transaction = await transactionPromise; - expect(transaction.transaction).toBe('GET /http-exception/:code'); - expect(errorEventOccurred).toBe(false); + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); }); + + const response = await fetch(`${baseURL}/http-exception/404`); + expect(response.status).toBe(404); + + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /http-exception/:code'); + expect(errorEventOccurred).toBe(false); }); }); @@ -221,30 +165,6 @@ test.describe('nested sub-app errors', () => { }); expect(errorEvent.request?.url).toContain('/test-errors/nested/child/error'); }); - - test('captures error from deeply nested sub-app route', async ({ baseURL }) => { - const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'Deeply nested child app error'; - }); - - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/nested/child/deep/error'); - }); - - const response = await fetch(`${baseURL}/test-errors/nested/child/deep/error`); - expect(response.status).toBe(500); - - const errorEvent = await errorPromise; - const transaction = await transactionPromise; - - expect(transaction.transaction).toBe('GET /test-errors/nested/child/deep/error'); - - expect(errorEvent.exception?.values?.[0]?.value).toBe('Deeply nested child app error'); - expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual({ - handled: false, - type: 'auto.http.hono.context_error', - }); - }); }); test.describe('custom onError handler', () => { @@ -275,104 +195,3 @@ test.describe('custom onError handler', () => { }); }); }); - -test.describe('no error capture for non-error responses', () => { - [ - { description: '301 redirect', path: '/redirect/301', expectedStatus: 301 }, - { description: '302 redirect', path: '/redirect/302', expectedStatus: 302 }, - ].forEach(({ description, path, expectedStatus }) => { - test(`does not capture error for ${description}`, async ({ baseURL }) => { - let errorEventOccurred = false; - - waitForError(APP_NAME, event => { - if (event.request?.url?.includes(path)) { - errorEventOccurred = true; - } - return false; - }); - - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(path); - }); - - const response = await fetch(`${baseURL}${path}`, { redirect: 'manual' }); - expect(response.status).toBe(expectedStatus); - - const transaction = await transactionPromise; - expect(transaction.transaction).toBe(`GET ${path}`); - expect(errorEventOccurred).toBe(false); - }); - }); - - [ - { description: '400 status without throw', path: '/status/400', expectedStatus: 400 }, - { description: '403 status without throw', path: '/status/403', expectedStatus: 403 }, - { description: '404 status without throw', path: '/status/404', expectedStatus: 404 }, - ].forEach(({ description, path, expectedStatus }) => { - test(`does not capture error for ${description}`, async ({ baseURL }) => { - let errorEventOccurred = false; - - waitForError(APP_NAME, event => { - if (event.request?.url?.includes(path)) { - errorEventOccurred = true; - } - return false; - }); - - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(path); - }); - - const response = await fetch(`${baseURL}${path}`); - expect(response.status).toBe(expectedStatus); - - const transaction = await transactionPromise; - expect(transaction.transaction).toBe(`GET ${path}`); - expect(errorEventOccurred).toBe(false); - }); - }); - - test('does not capture error for non-existent route (404)', async ({ baseURL }) => { - let errorEventOccurred = false; - - waitForError(APP_NAME, event => { - if (event.request?.url?.includes('/this-route-does-not-exist')) { - errorEventOccurred = true; - } - return false; - }); - - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/this-route-does-not-exist'); - }); - - const response = await fetch(`${baseURL}/this-route-does-not-exist`); - expect(response.status).toBe(404); - - const transaction = await transactionPromise; - expect(transaction.transaction).toBe('GET /this-route-does-not-exist'); - expect(errorEventOccurred).toBe(false); - }); - - test('does not capture error for successful 200 response', async ({ baseURL }) => { - let errorEventOccurred = false; - - waitForError(APP_NAME, event => { - if (event.request?.url?.includes('/?sentry-test-no-error')) { - errorEventOccurred = true; - } - return false; - }); - - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('GET /'); - }); - - const response = await fetch(`${baseURL}/?sentry-test-no-error`); - expect(response.status).toBe(200); - - const transaction = await transactionPromise; - expect(transaction.transaction).toBe('GET /'); - expect(errorEventOccurred).toBe(false); - }); -}); diff --git a/packages/hono/test/shared/middlewareHandlers.test.ts b/packages/hono/test/shared/middlewareHandlers.test.ts index 4d166da7854f..b8e4cdef1062 100644 --- a/packages/hono/test/shared/middlewareHandlers.test.ts +++ b/packages/hono/test/shared/middlewareHandlers.test.ts @@ -1,5 +1,4 @@ import * as SentryCore from '@sentry/core'; -import { HTTPException } from 'hono/http-exception'; import { beforeEach, describe, expect, it, vi } from 'vitest'; import { responseHandler } from '../../src/shared/middlewareHandlers'; From ae281577170e906883495a90fe9549392cbae856 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 4 May 2026 12:55:32 +0200 Subject: [PATCH 3/5] move error tests from route-patterns to errors --- .../src/route-groups/test-route-patterns.ts | 29 ------- .../hono-4/tests/errors.test.ts | 68 +++++++++-------- .../hono-4/tests/route-patterns.test.ts | 75 +------------------ 3 files changed, 37 insertions(+), 135 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-route-patterns.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-route-patterns.ts index e32662fb3b18..598943cad868 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-route-patterns.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-route-patterns.ts @@ -1,5 +1,4 @@ import { Hono } from 'hono'; -import { HTTPException } from 'hono/http-exception'; const routePatterns = new Hono(); @@ -24,32 +23,4 @@ METHODS.forEach(method => { routePatterns.on(method, '/on', c => c.text(`${method} on response`)); }); -// Error routes for direct method registration -METHODS.forEach(method => { - routePatterns[method]('/500', () => { - throw new HTTPException(500, { message: 'response 500' }); - }); - routePatterns[method]('/401', () => { - throw new HTTPException(401, { message: 'response 401' }); - }); - routePatterns[method]('/402', () => { - throw new HTTPException(402, { message: 'response 402' }); - }); - routePatterns[method]('/403', () => { - throw new HTTPException(403, { message: 'response 403' }); - }); -}); - -// Error routes for .all() -routePatterns.all('/all/500', () => { - throw new HTTPException(500, { message: 'response 500' }); -}); - -// Error routes for .on() -METHODS.forEach(method => { - routePatterns.on(method, '/on/500', () => { - throw new HTTPException(500, { message: 'response 500' }); - }); -}); - export { routePatterns }; diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts index 3e406814a509..e41cab852fe6 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts @@ -54,48 +54,52 @@ test.describe('HTTPException errors', () => { }); }); - test('does not capture 3xx HTTPException', async ({ baseURL }) => { - let errorEventOccurred = false; + [301, 302].forEach(code => { + test(`does not capture ${code} HTTPException`, async ({ baseURL }) => { + let errorEventOccurred = false; - waitForError(APP_NAME, event => { - if (event.exception?.values?.[0]?.value === 'HTTPException 301') { - errorEventOccurred = true; - } - return false; - }); + waitForError(APP_NAME, event => { + if (event.exception?.values?.[0]?.value === `HTTPException ${code}`) { + errorEventOccurred = true; + } + return false; + }); - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); - }); + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); + }); - const response = await fetch(`${baseURL}/http-exception/301`, { redirect: 'manual' }); - expect(response.status).toBe(301); + const response = await fetch(`${baseURL}/http-exception/${code}`, { redirect: 'manual' }); + expect(response.status).toBe(code); - const transaction = await transactionPromise; - expect(transaction.transaction).toBe('GET /http-exception/:code'); - expect(errorEventOccurred).toBe(false); + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /http-exception/:code'); + expect(errorEventOccurred).toBe(false); + }); }); - test('does not capture 4xx HTTPException', async ({ baseURL }) => { - let errorEventOccurred = false; + [401, 403, 404].forEach(code => { + test(`does not capture ${code} HTTPException`, async ({ baseURL }) => { + let errorEventOccurred = false; - waitForError(APP_NAME, event => { - if (event.exception?.values?.[0]?.value === 'HTTPException 404') { - errorEventOccurred = true; - } - return false; - }); + waitForError(APP_NAME, event => { + if (event.exception?.values?.[0]?.value === `HTTPException ${code}`) { + errorEventOccurred = true; + } + return false; + }); - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); - }); + const transactionPromise = waitForTransaction(APP_NAME, event => { + return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); + }); - const response = await fetch(`${baseURL}/http-exception/404`); - expect(response.status).toBe(404); + const response = await fetch(`${baseURL}/http-exception/${code}`); + expect(response.status).toBe(code); - const transaction = await transactionPromise; - expect(transaction.transaction).toBe('GET /http-exception/:code'); - expect(errorEventOccurred).toBe(false); + const transaction = await transactionPromise; + expect(transaction.transaction).toBe('GET /http-exception/:code'); + expect(errorEventOccurred).toBe(false); + }); }); }); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts index 15c6869d91d8..decd1049b6c9 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/route-patterns.test.ts @@ -1,5 +1,5 @@ import { expect, test } from '@playwright/test'; -import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; +import { waitForTransaction } from '@sentry-internal/test-utils'; import { APP_NAME } from './constants'; const PREFIX = '/test-routes'; @@ -73,76 +73,3 @@ test('async handler sends transaction', async ({ baseURL }) => { expect(transaction.transaction).toBe(`GET ${PREFIX}/async`); expect(transaction.contexts?.trace?.op).toBe('http.server'); }); - -test.describe('500 HTTPException capture', () => { - REGISTRATION_STYLES.forEach(({ name, path }) => { - test(`captures 500 from ${name} route with correct mechanism`, async ({ baseURL }) => { - const fullPath = `${PREFIX}${path}/500`; - - const errorPromise = waitForError(APP_NAME, event => { - return event.exception?.values?.[0]?.value === 'response 500' && !!event.request?.url?.includes(fullPath); - }); - - const response = await fetch(`${baseURL}${fullPath}`); - expect(response.status).toBe(500); - - const errorEvent = await errorPromise; - expect(errorEvent.exception?.values?.[0]?.value).toBe('response 500'); - expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual( - expect.objectContaining({ - handled: false, - type: 'auto.http.hono.context_error', - }), - ); - }); - }); - - test('captures 500 error with POST method', async ({ baseURL }) => { - const errorPromise = waitForError(APP_NAME, event => { - return ( - event.exception?.values?.[0]?.value === 'response 500' && - !!event.request?.url?.includes(`${PREFIX}/500`) && - event.request?.method === 'POST' - ); - }); - - const response = await fetch(`${baseURL}${PREFIX}/500`, { method: 'POST' }); - expect(response.status).toBe(500); - - const errorEvent = await errorPromise; - expect(errorEvent.exception?.values?.[0]?.value).toBe('response 500'); - expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual( - expect.objectContaining({ - handled: false, - type: 'auto.http.hono.context_error', - }), - ); - }); -}); - -test.describe('4xx HTTPException filtering', () => { - [401, 402, 403].forEach(code => { - test(`does not capture ${code} HTTPException`, async ({ baseURL }) => { - const fullPath = `${PREFIX}/${code}`; - let errorEventOccurred = false; - - waitForError(APP_NAME, event => { - if (event.exception?.values?.[0]?.value === `response ${code}` && event.request?.url?.includes(fullPath)) { - errorEventOccurred = true; - } - return false; - }); - - const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${PREFIX}/${code}`); - }); - - const response = await fetch(`${baseURL}${fullPath}`); - expect(response.status).toBe(code); - - const transaction = await transactionPromise; - expect(transaction.transaction).toBe(`GET ${PREFIX}/${code}`); - expect(errorEventOccurred).toBe(false); - }); - }); -}); From cfc56d09e401c95bf1a6e04d9ce46953ef381392 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 4 May 2026 14:16:37 +0200 Subject: [PATCH 4/5] fix bun and node tests --- .../hono-4/tests/errors.test.ts | 55 +++++++++++++++---- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts index e41cab852fe6..d737b462d994 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts @@ -1,6 +1,6 @@ import { expect, test } from '@playwright/test'; import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; -import { APP_NAME } from './constants'; +import { APP_NAME, RUNTIME } from './constants'; test.describe('route handler errors', () => { test('captures error with mechanism and trace correlation', async ({ baseURL }) => { @@ -54,6 +54,8 @@ test.describe('HTTPException errors', () => { }); }); + // On Node/Bun, httpServerSpansIntegration drops transactions for 3xx/4xx responses (ignoreStatusCodes), so we just use a request guard. + // On Cloudflare the transaction is available, and we additionally verify its name. [301, 302].forEach(code => { test(`does not capture ${code} HTTPException`, async ({ baseURL }) => { let errorEventOccurred = false; @@ -66,14 +68,25 @@ test.describe('HTTPException errors', () => { }); const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); + return RUNTIME === 'cloudflare' + ? event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/') + : event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /'; }); const response = await fetch(`${baseURL}/http-exception/${code}`, { redirect: 'manual' }); expect(response.status).toBe(code); + if (RUNTIME !== 'cloudflare') { + // Simple request guard for non-Cloudflare runtimes since the other transaction is dropped for 4xx responses + await fetch(`${baseURL}/`); + } + const transaction = await transactionPromise; - expect(transaction.transaction).toBe('GET /http-exception/:code'); + + if (RUNTIME === 'cloudflare') { + expect(transaction.transaction).toBe('GET /http-exception/:code'); + } + expect(errorEventOccurred).toBe(false); }); }); @@ -90,14 +103,25 @@ test.describe('HTTPException errors', () => { }); const transactionPromise = waitForTransaction(APP_NAME, event => { - return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/'); + return RUNTIME === 'cloudflare' + ? event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes('/http-exception/') + : event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /'; }); const response = await fetch(`${baseURL}/http-exception/${code}`); expect(response.status).toBe(code); + if (RUNTIME !== 'cloudflare') { + // Simple request guard for non-Cloudflare runtimes since the other transaction is dropped for 4xx responses + await fetch(`${baseURL}/`); + } + const transaction = await transactionPromise; - expect(transaction.transaction).toBe('GET /http-exception/:code'); + + if (RUNTIME === 'cloudflare') { + expect(transaction.transaction).toBe('GET /http-exception/:code'); + } + expect(errorEventOccurred).toBe(false); }); }); @@ -129,17 +153,28 @@ test.describe('middleware errors', () => { }); const transactionPromise = waitForTransaction(APP_NAME, event => { - return ( - event.contexts?.trace?.op === 'http.server' && - !!event.transaction?.includes('/test-errors/middleware-http-exception-4xx') - ); + if (RUNTIME === 'cloudflare') { + return ( + event.contexts?.trace?.op === 'http.server' && + !!event.transaction?.includes('/test-errors/middleware-http-exception-4xx') + ); + } + return event.contexts?.trace?.op === 'http.server' && event.transaction === 'GET /'; }); const response = await fetch(`${baseURL}/test-errors/middleware-http-exception-4xx`); expect(response.status).toBe(401); + if (RUNTIME !== 'cloudflare') { + await fetch(`${baseURL}/`); + } + const transaction = await transactionPromise; - expect(transaction.transaction).toBe('GET /test-errors/middleware-http-exception-4xx/*'); + + if (RUNTIME === 'cloudflare') { + expect(transaction.transaction).toBe('GET /test-errors/middleware-http-exception-4xx/*'); + } + expect(errorEventOccurred).toBe(false); }); }); From c522c31e10041a42c46d1b928940464d7483acb7 Mon Sep 17 00:00:00 2001 From: s1gr1d <32902192+s1gr1d@users.noreply.github.com> Date: Mon, 4 May 2026 15:54:55 +0200 Subject: [PATCH 5/5] fix setting span status --- .../hono-4/tests/errors.test.ts | 16 ++++++++++- .../hono-4/tests/middleware.test.ts | 28 ++++++++----------- .../hono/src/shared/wrapMiddlewareSpan.ts | 10 ++----- 3 files changed, 30 insertions(+), 24 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts index d737b462d994..98c81d30afeb 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts @@ -128,11 +128,18 @@ test.describe('HTTPException errors', () => { }); test.describe('middleware errors', () => { - test('captures 5xx HTTPException thrown in middleware', async ({ baseURL }) => { + test('captures 5xx HTTPException thrown in middleware with error span status', async ({ baseURL }) => { const errorPromise = waitForError(APP_NAME, event => { return event.exception?.values?.[0]?.value === 'Service Unavailable from middleware'; }); + const transactionPromise = waitForTransaction(APP_NAME, event => { + return ( + event.contexts?.trace?.op === 'http.server' && + !!event.transaction?.includes('/test-errors/middleware-http-exception') + ); + }); + const response = await fetch(`${baseURL}/test-errors/middleware-http-exception`); expect(response.status).toBe(503); @@ -140,6 +147,10 @@ test.describe('middleware errors', () => { expect(errorEvent.exception?.values?.[0]?.value).toBe('Service Unavailable from middleware'); expect(errorEvent.exception?.values?.[0]?.mechanism?.type).toBe('auto.middleware.hono'); expect(errorEvent.exception?.values?.[0]?.mechanism?.handled).toBe(false); + + const transaction = await transactionPromise; + const middlewareSpan = (transaction.spans || []).find(s => s.op === 'middleware.hono'); + expect(middlewareSpan?.status).toBe('internal_error'); }); test('does not capture 4xx HTTPException thrown in middleware', async ({ baseURL }) => { @@ -173,6 +184,9 @@ test.describe('middleware errors', () => { if (RUNTIME === 'cloudflare') { expect(transaction.transaction).toBe('GET /test-errors/middleware-http-exception-4xx/*'); + + const middlewareSpan = (transaction.spans || []).find(s => s.op === 'middleware.hono'); + expect(middlewareSpan?.status).not.toBe('internal_error'); } expect(errorEventOccurred).toBe(false); diff --git a/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts b/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts index 613ffc4af772..d984ac0d38a8 100644 --- a/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts @@ -39,9 +39,9 @@ for (const { name, prefix } of SCENARIOS) { description: 'middlewareA', op: 'middleware.hono', origin: 'auto.middleware.hono', - status: 'ok', }), ); + expect(middlewareSpan?.status).not.toBe('internal_error'); // @ts-expect-error timestamp is defined const durationMs = (middlewareSpan?.timestamp - middlewareSpan?.start_timestamp) * 1000; @@ -61,14 +61,13 @@ for (const { name, prefix } of SCENARIOS) { const spans = transaction.spans || []; - expect(spans).toContainEqual( - expect.objectContaining({ - description: '', - op: 'middleware.hono', - origin: 'auto.middleware.hono', - status: 'ok', - }), + const anonymousSpan = spans.find( + (span: { description?: string; op?: string }) => + span.op === 'middleware.hono' && span.description === '', ); + expect(anonymousSpan).toBeDefined(); + expect(anonymousSpan?.origin).toBe('auto.middleware.hono'); + expect(anonymousSpan?.status).not.toBe('internal_error'); }); test('multiple middleware are sibling spans under the same parent', async ({ baseURL }) => { @@ -218,14 +217,11 @@ test.describe('inline middleware spans (sub-app)', () => { }; const expectedDescription = EXPECTED_DESCRIPTIONS[regPath]![mwPath]!; - expect(transaction.spans).toContainEqual( - expect.objectContaining({ - description: expectedDescription, - op: 'middleware.hono', - origin: 'auto.middleware.hono', - status: 'ok', - }), - ); + const inlineSpan = (transaction.spans || []).find(s => s.description === expectedDescription); + expect(inlineSpan).toBeDefined(); + expect(inlineSpan?.op).toBe('middleware.hono'); + expect(inlineSpan?.origin).toBe('auto.middleware.hono'); + expect(inlineSpan?.status).not.toBe('internal_error'); }); } } diff --git a/packages/hono/src/shared/wrapMiddlewareSpan.ts b/packages/hono/src/shared/wrapMiddlewareSpan.ts index 7f2ff5ac6d94..13668e129c66 100644 --- a/packages/hono/src/shared/wrapMiddlewareSpan.ts +++ b/packages/hono/src/shared/wrapMiddlewareSpan.ts @@ -1,13 +1,12 @@ import { captureException, getActiveSpan, - getRootSpan, getOriginalFunction, + getRootSpan, markFunctionWrapped, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, - SPAN_STATUS_OK, startInactiveSpan, type WrappedFunction, } from '@sentry/core'; @@ -42,13 +41,10 @@ export function wrapMiddlewareWithSpan(handler: MiddlewareHandler): MiddlewareHa }); try { - const result = await handler(context, next); - span.setStatus({ code: SPAN_STATUS_OK }); - return result; + return await handler(context, next); } catch (error) { - span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - if (!isExpectedError(error)) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); captureException(error, { mechanism: { handled: false, type: MIDDLEWARE_ORIGIN }, });