Skip to content

Commit e704778

Browse files
committed
fix(hono): Use Proxy for patchAppRequest and patchRoute to preserve original function properties
1 parent f82f59b commit e704778

5 files changed

Lines changed: 283 additions & 37 deletions

File tree

packages/hono/src/shared/patchAppRequest.ts

Lines changed: 30 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {
22
debug,
33
getActiveSpan,
44
getOriginalFunction,
5-
markFunctionWrapped,
65
SEMANTIC_ATTRIBUTE_SENTRY_OP,
76
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
87
startSpan,
@@ -14,9 +13,6 @@ import { DEBUG_BUILD } from '../debug-build';
1413
const INTERNAL_REQUEST_OP = 'hono.request';
1514
const INTERNAL_REQUEST_ORIGIN = 'auto.http.hono.internal_request';
1615

17-
// Widened type to allow forwarding opaque args (env bindings, execution context)
18-
type LooseRequestFn = (input: string | Request | URL, requestInit?: RequestInit, ...rest: unknown[]) => unknown;
19-
2016
function extractPathname(input: string | Request | URL): string {
2117
if (typeof input === 'string') {
2218
return /^https?:\/\//.test(input) ? new URL(input).pathname : input;
@@ -39,34 +35,41 @@ export function patchAppRequest<E extends Env>(app: Hono<E>): void {
3935
return;
4036
}
4137

42-
const originalRequest = app.request as LooseRequestFn;
38+
const originalRequest = app.request;
4339

44-
const patchedRequest = (input: string | Request | URL, requestInit?: RequestInit, ...rest: unknown[]) => {
45-
if (!getActiveSpan()) {
46-
return originalRequest(input, requestInit, ...rest);
47-
}
40+
app.request = new Proxy(originalRequest, {
41+
apply(_target, thisArg, args: [string | Request | URL, RequestInit?, ...unknown[]]) {
42+
const [input, requestInit, ...rest] = args;
4843

49-
let method = requestInit?.method ?? (input instanceof Request ? input.method : 'GET');
50-
method = method.toUpperCase();
44+
if (!getActiveSpan()) {
45+
return Reflect.apply(_target, thisArg, args);
46+
}
5147

52-
const path = extractPathname(input);
48+
let method = requestInit?.method ?? (input instanceof Request ? input.method : 'GET');
49+
method = method.toUpperCase();
5350

54-
return startSpan(
55-
{
56-
name: `${method} ${path}`,
57-
op: INTERNAL_REQUEST_OP,
58-
onlyIfParent: true,
59-
attributes: {
60-
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: INTERNAL_REQUEST_OP,
61-
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: INTERNAL_REQUEST_ORIGIN,
62-
},
63-
},
64-
() => originalRequest(input, requestInit, ...rest),
65-
);
66-
};
51+
const path = extractPathname(input);
6752

68-
markFunctionWrapped(patchedRequest as unknown as WrappedFunction, originalRequest as unknown as WrappedFunction);
69-
app.request = patchedRequest as typeof app.request;
53+
return startSpan(
54+
{
55+
name: `${method} ${path}`,
56+
op: INTERNAL_REQUEST_OP,
57+
onlyIfParent: true,
58+
attributes: {
59+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: INTERNAL_REQUEST_OP,
60+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: INTERNAL_REQUEST_ORIGIN,
61+
},
62+
},
63+
() => Reflect.apply(_target, thisArg, [input, requestInit, ...rest]),
64+
);
65+
},
66+
get(target, prop, receiver) {
67+
if (prop === '__sentry_original__') {
68+
return originalRequest;
69+
}
70+
return Reflect.get(target, prop, receiver);
71+
},
72+
});
7073

7174
DEBUG_BUILD && debug.log('[hono] Patched app.request for internal dispatch tracing.');
7275
}

packages/hono/src/shared/patchRoute.ts

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { debug, getOriginalFunction, markFunctionWrapped } from '@sentry/core';
1+
import { debug, getOriginalFunction } from '@sentry/core';
22
import type { WrappedFunction } from '@sentry/core';
33
import type { Hono, MiddlewareHandler } from 'hono';
44
import { Hono as HonoClass } from 'hono';
@@ -81,16 +81,22 @@ export function installRouteHookOnPrototype(): RouteHookHandle {
8181
const originalRoute = honoBaseProto.route;
8282
const { handle, onSubAppMounted } = createRouteHook();
8383

84-
const patchedRoute = function (this: HonoAny, path: string, subApp: HonoAny): HonoAny {
85-
if (subApp && Array.isArray(subApp.routes)) {
86-
onSubAppMounted(subApp);
87-
}
88-
89-
return originalRoute.call(this, path, subApp);
90-
};
84+
honoBaseProto.route = new Proxy(originalRoute, {
85+
apply(_target, thisArg, args: [string, HonoAny]) {
86+
const [, subApp] = args;
87+
if (subApp && Array.isArray(subApp.routes)) {
88+
onSubAppMounted(subApp);
89+
}
9190

92-
markFunctionWrapped(patchedRoute as unknown as WrappedFunction, originalRoute as unknown as WrappedFunction);
93-
honoBaseProto.route = patchedRoute;
91+
return Reflect.apply(_target, thisArg, args);
92+
},
93+
get(target, prop, receiver) {
94+
if (prop === '__sentry_original__') {
95+
return originalRoute;
96+
}
97+
return Reflect.get(target, prop, receiver);
98+
},
99+
});
94100
honoBaseProto.__sentryRouteHook__ = handle;
95101

96102
DEBUG_BUILD && debug.log('[hono] Installed route hook on HonoBase.prototype.');

packages/hono/test/shared/applyPatches.test.ts

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,105 @@ describe('applyPatches', () => {
395395
});
396396
});
397397

398+
describe('wrapSubAppMiddleware non-invasive patching', () => {
399+
it('preserves symbol-keyed properties on sub-app middleware handlers', async () => {
400+
const app = new Hono();
401+
applyPatches(app);
402+
403+
const OPENAPI = Symbol('openapi');
404+
const META = Symbol('meta');
405+
const middleware = async function authMiddleware(_c: unknown, next: () => Promise<void>) {
406+
await next();
407+
};
408+
(middleware as any)[OPENAPI] = { security: [{ bearer: [] }] };
409+
(middleware as any)[META] = { rateLimit: 100 };
410+
(middleware as any).customProp = 'preserved';
411+
412+
const subApp = new Hono();
413+
subApp.use(middleware);
414+
subApp.get('/', () => new Response('ok'));
415+
416+
app.route('/api', subApp);
417+
418+
const route = (subApp.routes as Array<{ handler: Function }>).find(
419+
r => (r.handler as any).__sentry_original__ === middleware || r.handler === middleware,
420+
);
421+
expect(route).toBeDefined();
422+
423+
const wrappedHandler = route!.handler;
424+
const symbols = Object.getOwnPropertySymbols(wrappedHandler);
425+
expect(symbols).toContain(OPENAPI);
426+
expect(symbols).toContain(META);
427+
expect((wrappedHandler as any)[OPENAPI]).toEqual({ security: [{ bearer: [] }] });
428+
expect((wrappedHandler as any)[META]).toEqual({ rateLimit: 100 });
429+
expect((wrappedHandler as any).customProp).toBe('preserved');
430+
});
431+
432+
it('preserves function.name on sub-app middleware after wrapping', async () => {
433+
const app = new Hono();
434+
applyPatches(app);
435+
436+
const subApp = new Hono();
437+
subApp.use(async function corsMiddleware(_c: unknown, next: () => Promise<void>) {
438+
await next();
439+
});
440+
subApp.get('/', () => new Response('ok'));
441+
442+
app.route('/api', subApp);
443+
444+
const route = (subApp.routes as Array<{ handler: Function; method: string }>).find(
445+
r => r.method === 'ALL' && r.handler.name === 'corsMiddleware',
446+
);
447+
expect(route).toBeDefined();
448+
expect(route!.handler.name).toBe('corsMiddleware');
449+
});
450+
451+
it('preserves function.length on sub-app middleware after wrapping', async () => {
452+
const app = new Hono();
453+
applyPatches(app);
454+
455+
const subApp = new Hono();
456+
const mw = async function twoArgMw(_c: unknown, next: () => Promise<void>) {
457+
await next();
458+
};
459+
const originalLength = mw.length;
460+
subApp.use(mw);
461+
subApp.get('/', () => new Response('ok'));
462+
463+
app.route('/api', subApp);
464+
465+
const route = (subApp.routes as Array<{ handler: Function; method: string }>).find(
466+
r => r.method === 'ALL' && r.handler.length === originalLength,
467+
);
468+
expect(route).toBeDefined();
469+
expect(route!.handler.length).toBe(originalLength);
470+
});
471+
472+
it('does not alter the behavior of sub-app route handlers (non-middleware)', async () => {
473+
const app = new Hono();
474+
applyPatches(app);
475+
476+
const HANDLER_META = Symbol('handler-meta');
477+
const handler = async function getItems() {
478+
return new Response('items');
479+
};
480+
(handler as any)[HANDLER_META] = { cached: true };
481+
482+
const subApp = new Hono();
483+
subApp.get('/items', handler);
484+
485+
app.route('/api', subApp);
486+
487+
const route = (subApp.routes as Array<{ handler: Function; path: string }>).find(r => r.path === '/items');
488+
expect(route).toBeDefined();
489+
expect((route!.handler as any)[HANDLER_META]).toEqual({ cached: true });
490+
491+
const res = await app.fetch(new Request('http://localhost/api/items'));
492+
expect(res.status).toBe(200);
493+
expect(await res.text()).toBe('items');
494+
});
495+
});
496+
398497
describe('patchAppRequest integration', () => {
399498
it('patches .request() on sub-apps when they are mounted via route()', async () => {
400499
const app = new Hono();

packages/hono/test/shared/earlyPatchRoute.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,73 @@ describe('installRouteHookOnPrototype idempotency', () => {
140140
expect(startSpanMock).toHaveBeenCalledTimes(1);
141141
});
142142
});
143+
144+
describe('installRouteHookOnPrototype non-invasive patching', () => {
145+
afterAll(() => {
146+
honoBaseProto.route = originalRoute;
147+
});
148+
149+
it('preserves function.name of the original route method', () => {
150+
honoBaseProto.route = originalRoute;
151+
const originalName = originalRoute.name;
152+
153+
installRouteHookOnPrototype();
154+
155+
expect(honoBaseProto.route!.name).toBe(originalName);
156+
});
157+
158+
it('preserves function.length of the original route method', () => {
159+
honoBaseProto.route = originalRoute;
160+
const originalLength = originalRoute.length;
161+
162+
installRouteHookOnPrototype();
163+
164+
expect(honoBaseProto.route!.length).toBe(originalLength);
165+
});
166+
167+
it('preserves symbol-keyed properties on the route method', () => {
168+
honoBaseProto.route = originalRoute;
169+
const ROUTER_META = Symbol('router-meta');
170+
(originalRoute as any)[ROUTER_META] = { version: 3 };
171+
172+
installRouteHookOnPrototype();
173+
174+
const symbols = Object.getOwnPropertySymbols(honoBaseProto.route!);
175+
expect(symbols).toContain(ROUTER_META);
176+
expect((honoBaseProto.route as any)[ROUTER_META]).toEqual({ version: 3 });
177+
});
178+
179+
it('preserves string-keyed custom properties on the route method', () => {
180+
honoBaseProto.route = originalRoute;
181+
(originalRoute as any).pluginId = 'openapi-router';
182+
(originalRoute as any).__patched_by_other_lib__ = true;
183+
184+
installRouteHookOnPrototype();
185+
186+
expect((honoBaseProto.route as any).pluginId).toBe('openapi-router');
187+
expect((honoBaseProto.route as any).__patched_by_other_lib__).toBe(true);
188+
});
189+
190+
it('preserves prototype chain of the original function', () => {
191+
honoBaseProto.route = originalRoute;
192+
const originalProto = Object.getPrototypeOf(originalRoute);
193+
194+
installRouteHookOnPrototype();
195+
196+
expect(Object.getPrototypeOf(honoBaseProto.route!)).toBe(originalProto);
197+
});
198+
199+
it('correctly calls the original route and preserves return value', () => {
200+
honoBaseProto.route = originalRoute;
201+
installRouteHookOnPrototype();
202+
203+
const app = new Hono();
204+
applyPatches(app);
205+
206+
const subApp = new Hono();
207+
subApp.get('/test', c => c.text('ok'));
208+
209+
const result = app.route('/api', subApp);
210+
expect(result).toBe(app);
211+
});
212+
});

packages/hono/test/shared/patchAppRequest.test.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,4 +154,72 @@ describe('patchAppRequest', () => {
154154
expect.any(Function),
155155
);
156156
});
157+
158+
describe('non-invasive patching (preserves existing properties)', () => {
159+
it('preserves symbol-keyed properties on app.request', () => {
160+
const app = new Hono();
161+
const CUSTOM_SYMBOL = Symbol('custom-meta');
162+
(app.request as any)[CUSTOM_SYMBOL] = { version: 2 };
163+
164+
patchAppRequest(app);
165+
166+
const symbols = Object.getOwnPropertySymbols(app.request);
167+
expect(symbols).toContain(CUSTOM_SYMBOL);
168+
expect((app.request as any)[CUSTOM_SYMBOL]).toEqual({ version: 2 });
169+
});
170+
171+
it('preserves string-keyed custom properties on app.request', () => {
172+
const app = new Hono();
173+
(app.request as any).customFlag = true;
174+
(app.request as any).metadata = { wrapped: false };
175+
176+
patchAppRequest(app);
177+
178+
expect((app.request as any).customFlag).toBe(true);
179+
expect((app.request as any).metadata).toEqual({ wrapped: false });
180+
});
181+
182+
it('preserves function.name of the original request method', () => {
183+
const app = new Hono();
184+
const originalName = app.request.name;
185+
patchAppRequest(app);
186+
187+
expect(app.request.name).toBe(originalName);
188+
});
189+
190+
it('preserves function.length of the original request method', () => {
191+
const app = new Hono();
192+
const originalLength = app.request.length;
193+
patchAppRequest(app);
194+
195+
expect(app.request.length).toBe(originalLength);
196+
});
197+
198+
it('does not interfere with instanceof or typeof checks', () => {
199+
const app = new Hono();
200+
patchAppRequest(app);
201+
202+
expect(typeof app.request).toBe('function');
203+
});
204+
205+
it('preserves prototype chain of the original function', () => {
206+
const app = new Hono();
207+
const originalProto = Object.getPrototypeOf(app.request);
208+
patchAppRequest(app);
209+
210+
expect(Object.getPrototypeOf(app.request)).toBe(originalProto);
211+
});
212+
213+
it('preserves properties added by third-party libraries (e.g. OpenAPI metadata)', () => {
214+
const app = new Hono();
215+
const OPENAPI = Symbol('openapi');
216+
(app.request as any)[OPENAPI] = { paths: { '/hello': { get: {} } } };
217+
(app.request as any).__middleware_chain__ = ['auth', 'cors'];
218+
219+
patchAppRequest(app);
220+
221+
expect((app.request as any)[OPENAPI]).toEqual({ paths: { '/hello': { get: {} } } });
222+
expect((app.request as any).__middleware_chain__).toEqual(['auth', 'cors']);
223+
});
224+
});
157225
});

0 commit comments

Comments
 (0)