diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/middleware.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/middleware.ts new file mode 100644 index 000000000000..cc7bfae9896d --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/middleware.ts @@ -0,0 +1,17 @@ +import type { MiddlewareHandler } from 'hono'; + +export const middlewareA: MiddlewareHandler = async function middlewareA(c, next) { + // Add some delay + await new Promise(resolve => setTimeout(resolve, 50)); + await next(); +}; + +export const middlewareB: MiddlewareHandler = async function middlewareB(_c, next) { + // Add some delay + await new Promise(resolve => setTimeout(resolve, 60)); + await next(); +}; + +export const failingMiddleware: MiddlewareHandler = async function failingMiddleware(_c, _next) { + throw new Error('Middleware error'); +}; diff --git a/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-middleware.ts b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-middleware.ts new file mode 100644 index 000000000000..656fea319579 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-middleware.ts @@ -0,0 +1,10 @@ +import { Hono } from 'hono'; + +const testMiddleware = new Hono(); + +testMiddleware.get('/named', c => c.json({ middleware: 'named' })); +testMiddleware.get('/anonymous', c => c.json({ middleware: 'anonymous' })); +testMiddleware.get('/multi', c => c.json({ middleware: 'multi' })); +testMiddleware.get('/error', c => c.text('should not reach')); + +export { testMiddleware }; 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 a3272bc45ca3..65d30787de64 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,5 +1,7 @@ import type { Hono } from 'hono'; import { HTTPException } from 'hono/http-exception'; +import { testMiddleware } from './route-groups/test-middleware'; +import { middlewareA, middlewareB, failingMiddleware } from './middleware'; export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): void { app.get('/', c => { @@ -21,4 +23,17 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v const code = Number(c.req.param('code')) as any; throw new HTTPException(code, { message: `HTTPException ${code}` }); }); + + // === Middleware === + // Middleware is registered on the main app (the patched instance) via `app.use()` + // TODO: In the future, we may want to support middleware registration on sub-apps (route groups) + app.use('/test-middleware/named/*', middlewareA); + app.use('/test-middleware/anonymous/*', async (c, next) => { + c.header('X-Custom', 'anonymous'); + await next(); + }); + app.use('/test-middleware/multi/*', middlewareA, middlewareB); + app.use('/test-middleware/error/*', failingMiddleware); + + app.route('/test-middleware', testMiddleware); } 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 new file mode 100644 index 000000000000..a03398798756 --- /dev/null +++ b/dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts @@ -0,0 +1,143 @@ +import { expect, test } from '@playwright/test'; +import { waitForError, waitForTransaction } from '@sentry-internal/test-utils'; +import { type SpanJSON } from '@sentry/core'; + +const APP_NAME = 'hono-4'; + +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 /test-middleware/named'; + }); + + const response = await fetch(`${baseURL}/test-middleware/named`); + expect(response.status).toBe(200); + + const transaction = await transactionPromise; + const spans = transaction.spans || []; + + const middlewareSpan = spans.find( + (span: { description?: string; op?: string }) => + span.op === 'middleware.hono' && span.description === 'middlewareA', + ); + + expect(middlewareSpan).toEqual( + expect.objectContaining({ + description: 'middlewareA', + op: 'middleware.hono', + origin: 'auto.middleware.hono', + status: 'ok', + }), + ); + + // The middleware has a 50ms delay, so the span duration should be at least 50ms (0.05s) + // @ts-expect-error timestamp is defined + const durationMs = (middlewareSpan?.timestamp - middlewareSpan?.start_timestamp) * 1000; + expect(durationMs).toBeGreaterThanOrEqual(49); +}); + +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 /test-middleware/anonymous'; + }); + + const response = await fetch(`${baseURL}/test-middleware/anonymous`); + expect(response.status).toBe(200); + + const transaction = await transactionPromise; + const spans = transaction.spans || []; + + expect(spans).toContainEqual( + expect.objectContaining({ + description: '', + op: 'middleware.hono', + origin: 'auto.middleware.hono', + status: 'ok', + }), + ); +}); + +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 /test-middleware/multi'; + }); + + const response = await fetch(`${baseURL}/test-middleware/multi`); + expect(response.status).toBe(200); + + const transaction = await transactionPromise; + const spans = transaction.spans || []; + + // Sort spans because they are in a different order in Node/Bun (OTel-based) + const middlewareSpans = spans + .filter((span: SpanJSON) => span.op === 'middleware.hono' && span.origin === 'auto.middleware.hono') + .sort((a, b) => (a.start_timestamp ?? 0) - (b.start_timestamp ?? 0)); + + expect(middlewareSpans).toHaveLength(2); + expect(middlewareSpans[0]?.description).toBe('middlewareA'); + expect(middlewareSpans[1]?.description).toBe('middlewareB'); + + // Both middleware spans share the same parent (siblings, not nested) + expect(middlewareSpans[0]?.parent_span_id).toBe(middlewareSpans[1]?.parent_span_id); + + // middlewareA has a 50ms delay, middlewareB has a 60ms delay + // @ts-expect-error timestamp is defined + const middlewareADuration = (middlewareSpans[0]?.timestamp - middlewareSpans[0]?.start_timestamp) * 1000; + // @ts-expect-error timestamp is defined + const middlewareBDuration = (middlewareSpans[1]?.timestamp - middlewareSpans[1]?.start_timestamp) * 1000; + expect(middlewareADuration).toBeGreaterThanOrEqual(49); + expect(middlewareBDuration).toBeGreaterThanOrEqual(59); +}); + +test('captures error thrown in middleware', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Middleware error'; + }); + + const response = await fetch(`${baseURL}/test-middleware/error`); + expect(response.status).toBe(500); + + const errorEvent = await errorPromise; + expect(errorEvent.exception?.values?.[0]?.value).toBe('Middleware error'); + expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual( + expect.objectContaining({ + handled: false, + type: 'auto.middleware.hono', + }), + ); +}); + +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 /test-middleware/error/*'; + }); + + await fetch(`${baseURL}/test-middleware/error`); + + const transaction = await transactionPromise; + const spans = transaction.spans || []; + + const failingSpan = spans.find( + (span: { description?: string; op?: string }) => + span.op === 'middleware.hono' && span.description === 'failingMiddleware', + ); + + expect(failingSpan).toBeDefined(); + expect(failingSpan?.status).toBe('internal_error'); + expect(failingSpan?.origin).toBe('auto.middleware.hono'); +}); + +test('includes request data on error events from middleware', async ({ baseURL }) => { + const errorPromise = waitForError(APP_NAME, event => { + return event.exception?.values?.[0]?.value === 'Middleware error'; + }); + + await fetch(`${baseURL}/test-middleware/error`); + + const errorEvent = await errorPromise; + expect(errorEvent.request).toEqual( + expect.objectContaining({ + method: 'GET', + url: expect.stringContaining('/test-middleware/error'), + }), + ); +}); diff --git a/packages/hono/src/shared/patchAppUse.ts b/packages/hono/src/shared/patchAppUse.ts index 28c3c49e7193..f4bb9205c0f6 100644 --- a/packages/hono/src/shared/patchAppUse.ts +++ b/packages/hono/src/shared/patchAppUse.ts @@ -1,5 +1,7 @@ import { captureException, + getActiveSpan, + getRootSpan, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, @@ -32,15 +34,20 @@ export function patchAppUse(app: Hono): void { /** * Wraps a Hono middleware handler so that its execution is traced as a Sentry span. - * Uses startInactiveSpan so that all middleware spans are siblings under the request/transaction - * (onion order: A → B → handler → B → A does not nest B under A in the trace). + * Explicitly parents each span under the root (transaction) span so that all middleware + * spans are siblings — even when OTel instrumentation introduces nested active contexts + * (onion order: A → B → handler → B → A would otherwise nest B under A). */ function wrapMiddlewareWithSpan(handler: MiddlewareHandler): MiddlewareHandler { return async function sentryTracedMiddleware(context, next) { + const activeSpan = getActiveSpan(); + const rootSpan = activeSpan ? getRootSpan(activeSpan) : undefined; + const span = startInactiveSpan({ name: handler.name || '', op: 'middleware.hono', onlyIfParent: true, + parentSpan: rootSpan, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'middleware.hono', [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: MIDDLEWARE_ORIGIN, diff --git a/packages/hono/test/shared/patchAppUse.test.ts b/packages/hono/test/shared/patchAppUse.test.ts index 8f4e3bc0cc6c..0482d3569c84 100644 --- a/packages/hono/test/shared/patchAppUse.test.ts +++ b/packages/hono/test/shared/patchAppUse.test.ts @@ -155,4 +155,23 @@ describe('patchAppUse (middleware spans)', () => { expect(fakeApp._capturedThis).toBe(fakeApp); }); + + // todo: support sub-app (Hono route groups) patching in the future + it('does not wrap middleware on sub-apps (instance-level patching limitation)', async () => { + const app = new Hono(); + patchAppUse(app); + + // Route Grouping: https://hono.dev/docs/api/routing#grouping + const subApp = new Hono(); + subApp.use(async function subMiddleware(_c: unknown, next: () => Promise) { + await next(); + }); + subApp.get('/', () => new Response('sub')); + + app.route('/sub', subApp); + + await app.fetch(new Request('http://localhost/sub')); + + expect(startInactiveSpanMock).not.toHaveBeenCalledWith(expect.objectContaining({ name: 'subMiddleware' })); + }); });