Skip to content

fix(hono): Capture transaction name on request for correct culprit #20801

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.

Sign up for GitHub

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

Merged
s1gr1d merged 2 commits into from
May 11, 2026
Merged

fix(hono): Capture transaction name on request for correct culprit #20801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c77dd51
fix(hono): Capture transaction name on request for correct culprit
s1gr1d May 11, 2026
6b16217
fix(hono): Capture transaction name on request for correct culprit
s1gr1d May 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dev-packages/e2e-tests/test-applications/hono-4/src/route-groups/test-middleware.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ middlewareRoutes.get('/named', c => c.json({ middleware: 'named' }));
middlewareRoutes.get('/anonymous', c => c.json({ middleware: 'anonymous' }));
middlewareRoutes.get('/multi', c => c.json({ middleware: 'multi' }));
middlewareRoutes.get('/error', c => c.text('should not reach'));
middlewareRoutes.get('/param/:id', c => c.json({ paramId: c.req.param('id') }));

// Self-contained sub-app registering its own middleware via .use()
const subAppWithMiddleware = new Hono();
Expand All @@ -18,6 +19,7 @@ subAppWithMiddleware.use('/anonymous/*', async (c, next) => {
});
subAppWithMiddleware.use('/multi/*', middlewareA, middlewareB);
subAppWithMiddleware.use('/error/*', failingMiddleware);
subAppWithMiddleware.use('/param/*', middlewareA);

// .all() handler (1 parameter) — should NOT be wrapped as middleware by patchRoute.
subAppWithMiddleware.all('/all-handler', async function allCatchAll(c) {
Expand Down
1 change: 1 addition & 0 deletions dev-packages/e2e-tests/test-applications/hono-4/src/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export function addRoutes(app: Hono<{ Bindings?: { E2E_TEST_DSN: string } }>): v
});
app.use('/test-middleware/multi/*', middlewareA, middlewareB);
app.use('/test-middleware/error/*', failingMiddleware);
app.use('/test-middleware/param/*', middlewareA);
app.route('/test-middleware', middlewareRoutes);

// Sub-app middleware: registered on the sub-app, wrapped at mount time by route() patching
Expand Down
3 changes: 2 additions & 1 deletion dev-packages/e2e-tests/test-applications/hono-4/tests/errors.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ 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);
expect(errorEvent.transaction).toBe('GET /test-errors/middleware-http-exception');

const transaction = await transactionPromise;
const middlewareSpan = (transaction.spans || []).find(s => s.op === 'middleware.hono');
Expand Down Expand Up @@ -183,7 +184,7 @@ test.describe('middleware errors', () => {
const transaction = await transactionPromise;

if (RUNTIME === 'cloudflare') {
expect(transaction.transaction).toBe('GET /test-errors/middleware-http-exception-4xx/*');
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');
Expand Down
24 changes: 23 additions & 1 deletion dev-packages/e2e-tests/test-applications/hono-4/tests/middleware.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ for (const { name, prefix } of SCENARIOS) {
type: 'auto.middleware.hono',
}),
);

// The transaction name on the error event determines the culprit shown in Sentry.
expect(errorEvent.transaction).toBe(`GET ${prefix}/error`);
});

test('sets error status on middleware span when middleware throws', async ({ baseURL }) => {
Expand All @@ -126,7 +129,7 @@ for (const { name, prefix } of SCENARIOS) {
await fetch(`${baseURL}${prefix}/error`);

const transaction = await transactionPromise;
expect(transaction.transaction).toBe(`GET ${prefix}/error/*`);
expect(transaction.transaction).toBe(`GET ${prefix}/error`);

const spans = transaction.spans || [];

Expand All @@ -138,6 +141,25 @@ for (const { name, prefix } of SCENARIOS) {
expect(failingSpan?.status).toBe('internal_error');
});

test('uses parameterized route in transaction name', async ({ baseURL }) => {
const transactionPromise = waitForTransaction(APP_NAME, event => {
return event.contexts?.trace?.op === 'http.server' && !!event.transaction?.includes(`${prefix}/param/`);
});

const response = await fetch(`${baseURL}${prefix}/param/42`);
expect(response.status).toBe(200);

const transaction = await transactionPromise;
expect(transaction.transaction).toBe(`GET ${prefix}/param/:id`);

const spans = transaction.spans || [];
const middlewareSpan = spans.find(
(span: { description?: string; op?: string }) =>
span.op === 'middleware.hono' && span.description === 'middlewareA',
);
expect(middlewareSpan).toBeDefined();
});

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' && !!event.request?.url?.includes(prefix);
Expand Down
25 changes: 16 additions & 9 deletions packages/hono/src/shared/middlewareHandlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
getRootSpan,
SEMANTIC_ATTRIBUTE_SENTRY_SOURCE,
updateSpanName,
type Scope,
winterCGRequestToRequestData,
} from '@sentry/core';
import type { Context } from 'hono';
Expand All @@ -22,6 +23,8 @@ export function requestHandler(context: Context): void {

const isolationScope = defaultScope === currentIsolationScope ? defaultScope : currentIsolationScope;

updateSpanRouteName(isolationScope, context);

isolationScope.setSDKProcessingMetadata({
normalizedRequest: winterCGRequestToRequestData(hasFetchEvent(context) ? context.event.request : context.req.raw),
});
Expand All @@ -31,21 +34,25 @@ export function requestHandler(context: Context): void {
* Response handler for Hono framework
*/
export function responseHandler(context: Context): void {
if (context.error && !isExpectedError(context.error)) {
getClient()?.captureException(context.error, {
mechanism: { handled: false, type: 'auto.http.hono.context_error' },
});
}
}

function updateSpanRouteName(isolationScope: Scope, context: Context): void {
const activeSpan = getActiveSpan();
const lastMatchedRoute = routePath(context, -1);
Copy link
Copy Markdown
Member

@chargome chargome May 11, 2026

Choose a reason for hiding this comment

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

nit: A comment explaining what this actually does would be helpful, but not worth a CI rerun

Copy link
Copy Markdown
Member

@chargome chargome May 11, 2026

Choose a reason for hiding this comment

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

Also, is this always the correct one?

Copy link
Copy Markdown
Member Author

@s1gr1d s1gr1d May 11, 2026

Choose a reason for hiding this comment

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

I can include a comment in another PR.

And yes, -1 gets the last matched path which is the one where it's routed to.
https://hono.dev/docs/helpers/route#using-with-index-parameter


if (activeSpan) {
activeSpan.updateName(`${context.req.method} ${routePath(context)}`);
activeSpan.updateName(`${context.req.method} ${lastMatchedRoute}`);
activeSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');

const rootSpan = getRootSpan(activeSpan);
updateSpanName(rootSpan, `${context.req.method} ${routePath(context)}`);
updateSpanName(rootSpan, `${context.req.method} ${lastMatchedRoute}`);
rootSpan.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, 'route');
}

getIsolationScope().setTransactionName(`${context.req.method} ${routePath(context)}`);

if (context.error && !isExpectedError(context.error)) {
getClient()?.captureException(context.error, {
mechanism: { handled: false, type: 'auto.http.hono.context_error' },
});
}
isolationScope.setTransactionName(`${context.req.method} ${lastMatchedRoute}`);
}
Comment thread
s1gr1d marked this conversation as resolved.
6 changes: 4 additions & 2 deletions packages/hono/test/shared/middlewareHandlers.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as SentryCore from '@sentry/core';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { responseHandler } from '../../src/shared/middlewareHandlers';
import { requestHandler, responseHandler } from '../../src/shared/middlewareHandlers';

vi.mock('hono/route', () => ({
routePath: () => '/test',
Expand All @@ -11,6 +11,7 @@ vi.mock('../../src/utils/hono-context', () => ({
}));

const mockSetTransactionName = vi.fn();
const mockSetSDKProcessingMetadata = vi.fn();

vi.mock('@sentry/core', async () => {
const actual = await vi.importActual('@sentry/core');
Expand All @@ -19,6 +20,7 @@ vi.mock('@sentry/core', async () => {
getActiveSpan: vi.fn(() => null),
getIsolationScope: vi.fn(() => ({
setTransactionName: mockSetTransactionName,
setSDKProcessingMetadata: mockSetSDKProcessingMetadata,
})),
getClient: vi.fn(() => undefined),
};
Expand Down Expand Up @@ -110,7 +112,7 @@ describe('responseHandler', () => {
describe('transaction name', () => {
it('sets transaction name on isolation scope', () => {
// oxlint-disable-next-line typescript/no-explicit-any
responseHandler(createMockContext(200) as any);
requestHandler(createMockContext(200) as any);

expect(mockSetTransactionName).toHaveBeenCalledWith('GET /test');
});
Expand Down
Loading