Skip to content

Commit a7c1ed0

Browse files
mydeaclaude
andcommitted
fix(server-utils): clear hono request markers after the response
The dedup markers live on the isolation scope. When that scope outlives the request, e.g. forked once around the whole server on a runtime without per-request isolation, every later request was treated as a duplicate and lost its route name and request data, and could pick up a stale shouldHandleError. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 99d7176 commit a7c1ed0

2 files changed

Lines changed: 44 additions & 18 deletions

File tree

‎packages/server-utils/src/integrations/hono/createHonoMiddleware.ts‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,20 @@ export function createHonoRequestMiddleware(options: CreateHonoRequestMiddleware
9191
}
9292
addNonEnumerableProperty(scope, HONO_REQUEST_HANDLED, true);
9393

94-
requestHandler(context, options.getConnInfo);
94+
try {
95+
requestHandler(context, options.getConnInfo);
9596

96-
await next(); // Handler runs in between Request above ⤴ and Response below ⤵
97+
await next(); // Handler runs in between Request above ⤴ and Response below ⤵
9798

98-
const effectiveShouldHandleError =
99-
(scope[HONO_SHOULD_HANDLE_ERROR] as SentryHonoMiddlewareOptions['shouldHandleError']) ?? shouldHandleError;
100-
responseHandler(context, effectiveShouldHandleError);
99+
const effectiveShouldHandleError =
100+
(scope[HONO_SHOULD_HANDLE_ERROR] as SentryHonoMiddlewareOptions['shouldHandleError']) ?? shouldHandleError;
101+
responseHandler(context, effectiveShouldHandleError);
102+
} finally {
103+
// An isolation scope that outlives the request (e.g. forked once around the whole server on a
104+
// runtime without per-request isolation) must not dedupe the next request against this one.
105+
addNonEnumerableProperty(scope, HONO_REQUEST_HANDLED, undefined);
106+
addNonEnumerableProperty(scope, HONO_SHOULD_HANDLE_ERROR, undefined);
107+
}
101108
};
102109

103110
addNonEnumerableProperty(middleware, SENTRY_HONO_MIDDLEWARE, true);

‎packages/server-utils/test/integrations/hono/createHonoMiddleware.test.ts‎

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { beforeEach, describe, expect, it, vi } from 'vitest';
1+
import { setAsyncContextStrategy, withIsolationScope } from '@sentry/core';
2+
import { beforeEach, describe, expect, it, onTestFinished, vi } from 'vitest';
23

34
// Spy on the request/response handlers so we can assert exactly how many times they run.
45
const requestHandler = vi.fn();
@@ -10,6 +11,8 @@ vi.mock('../../../src/integrations/hono/middlewareHandlers', () => ({
1011
captureContextError: (...args: unknown[]) => captureContextError(...args),
1112
}));
1213

14+
// eslint-disable-next-line import/first
15+
import { setAsyncLocalStorageAsyncContextStrategy } from '../../../src/async-context';
1316
// eslint-disable-next-line import/first
1417
import { createHonoRequestMiddleware } from '../../../src/integrations/hono/createHonoMiddleware';
1518

@@ -38,23 +41,39 @@ describe('createHonoRequestMiddleware — duplicate registration handling', () =
3841
expect(responseHandler).toHaveBeenCalledTimes(1);
3942
});
4043

41-
it('passes an already-handled context straight through to next()', async () => {
44+
it('passes a deduplicated request straight through to next()', async () => {
4245
const context = fakeContext();
46+
const handler = vi.fn();
4347

44-
// First middleware handles the request and marks the context.
45-
await createHonoRequestMiddleware()(context, async () => {});
46-
requestHandler.mockClear();
47-
responseHandler.mockClear();
48-
49-
// A second (duplicate) middleware on the same context must not re-run the handlers.
50-
let nextCalled = false;
5148
await createHonoRequestMiddleware()(context, async () => {
52-
nextCalled = true;
49+
await createHonoRequestMiddleware()(context, handler);
5350
});
5451

55-
expect(nextCalled).toBe(true);
56-
expect(requestHandler).not.toHaveBeenCalled();
57-
expect(responseHandler).not.toHaveBeenCalled();
52+
expect(handler).toHaveBeenCalledTimes(1);
53+
expect(requestHandler).toHaveBeenCalledTimes(1);
54+
expect(responseHandler).toHaveBeenCalledTimes(1);
55+
});
56+
57+
it('handles sequential requests sharing one isolation scope independently', async () => {
58+
const userShouldHandleError = (): boolean => true;
59+
const auto = createHonoRequestMiddleware();
60+
const manual = createHonoRequestMiddleware({ shouldHandleError: userShouldHandleError });
61+
setAsyncLocalStorageAsyncContextStrategy();
62+
onTestFinished(() => setAsyncContextStrategy(undefined));
63+
64+
await withIsolationScope(async () => {
65+
const firstContext = fakeContext();
66+
await auto(firstContext, async () => {
67+
await manual(firstContext, async () => {});
68+
});
69+
70+
const secondContext = fakeContext();
71+
await auto(secondContext, async () => {});
72+
73+
expect(requestHandler).toHaveBeenCalledTimes(2);
74+
expect(responseHandler).toHaveBeenNthCalledWith(1, firstContext, userShouldHandleError);
75+
expect(responseHandler).toHaveBeenNthCalledWith(2, secondContext, undefined);
76+
});
5877
});
5978

6079
it('handles independent requests independently (marker is per-context)', async () => {

0 commit comments

Comments
 (0)