-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(hono): Instrument middlewares app.use()
#19611
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
abeef91
e4a35b8
4f0e13f
4b014ce
e540965
f7057ae
a09b775
58bb050
0f39821
0e032d6
429d418
8e7268c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| import { | ||
| captureException, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_OP, | ||
| SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, | ||
| SPAN_STATUS_ERROR, | ||
| SPAN_STATUS_OK, | ||
| startInactiveSpan, | ||
| } from '@sentry/core'; | ||
| import type { Hono, MiddlewareHandler } from 'hono'; | ||
|
|
||
| const MIDDLEWARE_ORIGIN = 'auto.middleware.hono'; | ||
|
|
||
| /** | ||
| * Patches `app.use` so that every middleware registered through it is automatically | ||
| * wrapped in a Sentry span. Supports both forms: `app.use(...handlers)` and `app.use(path, ...handlers)`. | ||
| */ | ||
| export function patchAppUse(app: Hono): void { | ||
| app.use = new Proxy(app.use, { | ||
| apply(target: typeof app.use, thisArg: typeof app, args: Parameters<typeof app.use>): ReturnType<typeof app.use> { | ||
| const [first, ...rest] = args as [unknown, ...MiddlewareHandler[]]; | ||
|
|
||
| if (typeof first === 'string') { | ||
| const wrappedHandlers = rest.map(handler => wrapMiddlewareWithSpan(handler)); | ||
| return Reflect.apply(target, thisArg, [first, ...wrappedHandlers]); | ||
| } | ||
|
|
||
| const allHandlers = [first as MiddlewareHandler, ...rest].map(handler => wrapMiddlewareWithSpan(handler)); | ||
| return Reflect.apply(target, thisArg, allHandlers); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * 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). | ||
| */ | ||
| function wrapMiddlewareWithSpan(handler: MiddlewareHandler): MiddlewareHandler { | ||
| return async function sentryTracedMiddleware(context, next) { | ||
| const span = startInactiveSpan({ | ||
| name: handler.name || '<anonymous>', | ||
| op: 'middleware.hono', | ||
| onlyIfParent: true, | ||
| attributes: { | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'middleware.hono', | ||
| [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: MIDDLEWARE_ORIGIN, | ||
| }, | ||
| }); | ||
|
|
||
| try { | ||
| const result = await handler(context, next); | ||
| span.setStatus({ code: SPAN_STATUS_OK }); | ||
| return result; | ||
| } catch (error) { | ||
| span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); | ||
| captureException(error, { | ||
| mechanism: { handled: false, type: MIDDLEWARE_ORIGIN }, | ||
| }); | ||
| throw error; | ||
s1gr1d marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } finally { | ||
| span.end(); | ||
| } | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| import * as SentryCore from '@sentry/core'; | ||
| import { Hono } from 'hono'; | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest'; | ||
| import { patchAppUse } from '../../src/shared/patchAppUse'; | ||
|
|
||
| vi.mock('@sentry/core', async () => { | ||
| const actual = await vi.importActual('@sentry/core'); | ||
| return { | ||
| ...actual, | ||
| startInactiveSpan: vi.fn((_opts: unknown) => ({ | ||
| setStatus: vi.fn(), | ||
| end: vi.fn(), | ||
| })), | ||
| captureException: vi.fn(), | ||
| }; | ||
| }); | ||
|
|
||
| const startInactiveSpanMock = SentryCore.startInactiveSpan as ReturnType<typeof vi.fn>; | ||
| const captureExceptionMock = SentryCore.captureException as ReturnType<typeof vi.fn>; | ||
|
|
||
| describe('patchAppUse (middleware spans)', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('wraps handlers in app.use(handler) so startInactiveSpan is called when middleware runs', async () => { | ||
| const app = new Hono(); | ||
| patchAppUse(app); | ||
|
|
||
| const userHandler = vi.fn(async (_c: unknown, next: () => Promise<void>) => { | ||
| await next(); | ||
| }); | ||
| app.use(userHandler); | ||
|
|
||
| expect(startInactiveSpanMock).not.toHaveBeenCalled(); | ||
|
|
||
| const fetchHandler = app.fetch; | ||
| const req = new Request('http://localhost/'); | ||
| await fetchHandler(req); | ||
|
|
||
| expect(startInactiveSpanMock).toHaveBeenCalledTimes(1); | ||
| expect(startInactiveSpanMock).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| op: 'middleware.hono', | ||
| onlyIfParent: true, | ||
| attributes: expect.objectContaining({ | ||
| 'sentry.op': 'middleware.hono', | ||
| 'sentry.origin': 'auto.middleware.hono', | ||
| }), | ||
| }), | ||
| ); | ||
| expect(userHandler).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| describe('span naming', () => { | ||
| it('uses handler.name for span when handler has a name', async () => { | ||
| const app = new Hono(); | ||
| patchAppUse(app); | ||
|
|
||
| async function myNamedMiddleware(_c: unknown, next: () => Promise<void>) { | ||
| await next(); | ||
| } | ||
| app.use(myNamedMiddleware); | ||
|
|
||
| await app.fetch(new Request('http://localhost/')); | ||
|
|
||
| expect(startInactiveSpanMock).toHaveBeenCalledWith(expect.objectContaining({ name: 'myNamedMiddleware' })); | ||
| }); | ||
|
|
||
| it('uses <anonymous.index> for span when handler is anonymous', async () => { | ||
| const app = new Hono(); | ||
| patchAppUse(app); | ||
|
|
||
| app.use(async (_c: unknown, next: () => Promise<void>) => next()); | ||
|
|
||
| await app.fetch(new Request('http://localhost/')); | ||
|
|
||
| expect(startInactiveSpanMock).toHaveBeenCalledTimes(1); | ||
| const name = startInactiveSpanMock.mock.calls[0][0].name; | ||
| expect(name).toMatch('<anonymous>'); | ||
| }); | ||
| }); | ||
|
|
||
| it('wraps each handler in app.use(path, ...handlers) and passes path through', async () => { | ||
| const app = new Hono(); | ||
| patchAppUse(app); | ||
|
|
||
| const handler = async (_c: unknown, next: () => Promise<void>) => next(); | ||
| app.use('/api', handler); | ||
| app.get('/api', () => new Response('ok')); | ||
|
|
||
| await app.fetch(new Request('http://localhost/api')); | ||
|
|
||
| expect(startInactiveSpanMock).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('calls captureException when middleware throws', async () => { | ||
| const app = new Hono(); | ||
| patchAppUse(app); | ||
|
|
||
| const err = new Error('middleware error'); | ||
| app.use(async () => { | ||
| throw err; | ||
| }); | ||
|
|
||
| const res = await app.fetch(new Request('http://localhost/')); | ||
| expect(res.status).toBe(500); | ||
|
|
||
| expect(captureExceptionMock).toHaveBeenCalledWith(err, { | ||
| mechanism: { handled: false, type: 'auto.middleware.hono' }, | ||
| }); | ||
| }); | ||
|
|
||
| it('creates sibling spans for multiple middlewares (onion order, not parent-child)', async () => { | ||
| const app = new Hono(); | ||
| patchAppUse(app); | ||
|
|
||
| app.use( | ||
| async (_c: unknown, next: () => Promise<void>) => next(), | ||
| async function namedMiddleware(_c: unknown, next: () => Promise<void>) { | ||
| await next(); | ||
| }, | ||
| async (_c: unknown, next: () => Promise<void>) => next(), | ||
| ); | ||
|
|
||
| await app.fetch(new Request('http://localhost/')); | ||
|
|
||
| expect(startInactiveSpanMock).toHaveBeenCalledTimes(3); | ||
| const [firstCall, secondCall, thirdCall] = startInactiveSpanMock.mock.calls; | ||
| expect(firstCall[0]).toMatchObject({ op: 'middleware.hono' }); | ||
| expect(secondCall[0]).toMatchObject({ op: 'middleware.hono' }); | ||
| expect(firstCall[0].name).toMatch('<anonymous>'); | ||
| expect(secondCall[0].name).toBe('namedMiddleware'); | ||
| expect(thirdCall[0].name).toBe('<anonymous>'); | ||
| expect(firstCall[0].name).not.toBe(secondCall[0].name); | ||
| }); | ||
|
|
||
| it('preserves this context when calling the original use (Proxy forwards thisArg)', () => { | ||
| type FakeApp = { | ||
| _capturedThis: unknown; | ||
| use: (...args: unknown[]) => FakeApp; | ||
| }; | ||
| const fakeApp: FakeApp = { | ||
| _capturedThis: null, | ||
| use(this: FakeApp, ..._args: unknown[]) { | ||
| this._capturedThis = this; | ||
| return this; | ||
| }, | ||
| }; | ||
|
|
||
| patchAppUse(fakeApp as unknown as Parameters<typeof patchAppUse>[0]); | ||
|
|
||
| const noop = async (_c: unknown, next: () => Promise<void>) => next(); | ||
| fakeApp.use(noop); | ||
|
|
||
| expect(fakeApp._capturedThis).toBe(fakeApp); | ||
| }); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing integration or E2E test for feat PRMedium Severity This Additional Locations (1)Triggered by project rule: PR Review Guidelines for Cursor Bot |
||


Uh oh!
There was an error while loading. Please reload this page.