Skip to content

Commit ad47c3c

Browse files
authored
ref(hono): Consolidate route patching and add clarification comments (#20829)
A little bit of refactoring to combine all tests into `applyPatches.test.ts` and make a better distinction between prototype patching and patching of class fields (also described better in the comments). This will be important for the upcoming changes for instrumenting `.request()` calls. Part of #20807
1 parent 28d6fe5 commit ad47c3c

7 files changed

Lines changed: 432 additions & 466 deletions

File tree

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
import type { Env, Hono } from 'hono';
2-
import { patchAppUse } from '../shared/patchAppUse';
3-
import { patchRoute } from '../shared/patchRoute';
2+
import { patchAppUse } from './patchAppUse';
3+
import { installRouteHookOnPrototype } from './patchRoute';
44

55
/**
6-
* Applies necessary patches to the Hono app to ensure that Sentry can properly trace middleware and route handlers.
6+
* Instruments a Hono app instance for Sentry tracing in middleware and route handlers.
7+
*
8+
* Two strategies are needed because Hono mixes instance fields and prototype methods:
9+
* - `use` is a per-instance class field (instance own property) → must be patched on the instance
10+
* - `route` is a prototype method → patched once globally, covers all instances
711
*/
812
export function applyPatches<E extends Env>(app: Hono<E>): void {
913
// `app.use` (instance own property) — wraps middleware at registration time on this instance.
1014
patchAppUse(app);
1115

12-
//`HonoBase.prototype.route` — wraps sub-app middleware at mount time so that route groups (`app.route('/prefix', subApp)`) are also instrumented.
13-
patchRoute(app);
16+
// `route()` lives on the shared prototype and is patched once globally.
17+
installRouteHookOnPrototype();
1418
}

packages/hono/src/shared/middlewareHandlers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ export function responseHandler(context: Context): void {
4343

4444
function updateSpanRouteName(isolationScope: Scope, context: Context): void {
4545
const activeSpan = getActiveSpan();
46+
47+
// Final matched route: https://hono.dev/docs/helpers/route#using-with-index-parameter
4648
const lastMatchedRoute = routePath(context, -1);
4749

4850
if (activeSpan) {

packages/hono/src/shared/patchAppUse.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@ import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan';
22
import type { Env, Hono, MiddlewareHandler } from 'hono';
33

44
/**
5-
* Patches the Hono app so that middleware is automatically traced as Sentry spans.
5+
* Patches `app.use` (instance own property) on a Hono instance to instrument middleware at registration time.
6+
*
7+
* Must be per-instance because `use` is a class field, not a prototype method.
68
*/
79
export function patchAppUse<E extends Env>(app: Hono<E>): void {
810
app.use = new Proxy(app.use, {

packages/hono/src/shared/patchRoute.ts

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { getOriginalFunction, markFunctionWrapped } from '@sentry/core';
22
import type { WrappedFunction } from '@sentry/core';
3-
import type { Env, Hono, MiddlewareHandler } from 'hono';
3+
import type { Hono, MiddlewareHandler } from 'hono';
4+
import { Hono as HonoClass } from 'hono';
45
import { wrapMiddlewareWithSpan } from './wrapMiddlewareSpan';
56

67
interface HonoRoute {
@@ -15,18 +16,20 @@ interface HonoBaseProto {
1516
}
1617

1718
/**
18-
* Patches `HonoBase.prototype.route` so that when a sub-app is mounted via `app.route('/prefix', subApp)`, its middleware handlers
19-
* are retroactively wrapped in Sentry spans before the parent copies them.
19+
* Patches `route()` on the Hono base prototype once, globally.
2020
*
21-
* `route` lives on the prototype (unlike `use` which is a class field)
21+
* Wraps sub-app middleware at mount time so that `app.route('/prefix', subApp)` is traced.
22+
* Idempotent: safe to call multiple times.
2223
*/
23-
export function patchRoute<E extends Env>(app: Hono<E>): void {
24-
const honoBaseProto = Object.getPrototypeOf(Object.getPrototypeOf(app)) as HonoBaseProto;
24+
export function installRouteHookOnPrototype(): void {
25+
// `route` is on the base prototype, not the concrete subclass, walk up one level
26+
const honoBaseProto = Object.getPrototypeOf(HonoClass.prototype) as HonoBaseProto;
2527
if (!honoBaseProto || typeof honoBaseProto?.route !== 'function') {
2628
return;
2729
}
2830

29-
if (getOriginalFunction(honoBaseProto.route as WrappedFunction)) {
31+
// Already patched: return
32+
if (getOriginalFunction(honoBaseProto.route as unknown as WrappedFunction)) {
3033
return;
3134
}
3235

@@ -45,18 +48,13 @@ export function patchRoute<E extends Env>(app: Hono<E>): void {
4548
}
4649

4750
/**
48-
* Figures out which handlers in a sub-app's flat routes array are middleware (and should get a span), then wraps them.
51+
* Identifies middleware handlers in a sub-app's flat routes array and wraps them in spans.
4952
*
50-
* The challenge: Hono stores every handler as a plain { method, path, handler } entry. There is no "isMiddleware" flag.
51-
* Two heuristics identify middleware:
52-
*
53-
* 1. Position within a group. `app.get('/path', mw, handler)` produces two entries with the same method+path.
54-
* All but the last one must be middleware, because only middleware calls `next()` to pass control to the next handler.
55-
*
56-
* 2. Function arity (# of params) for method 'ALL'. Both `.use()` and `.all()` store their handlers under method 'ALL',
57-
* so we can't use position alone to tell them apart when one is the last (or only) entry in its group.
58-
* The deciding factor: Hono's `.use()` only accepts `(context, next)` (handlers with 2+ params). While `.all()` route
59-
* handlers typically only accept `(context)`.
53+
* Heuristics (since Hono has no "isMiddleware" flag):
54+
* 1. Position: `app.get('/path', mw, handler)` produces entries with the same method+path.
55+
* All but the LAST are middleware (they call `next()`).
56+
* 2. Arity (# of params) for method 'ALL': `.use()` handlers always have 2+ params (context, next),
57+
* while `.all()` route handlers typically have 1 (`context` only).
6058
* See: https://github.com/honojs/hono/blob/18fe604c8cefc2628240651b1af219692e1918c1/src/hono-base.ts#L156-L168
6159
*/
6260
export function wrapSubAppMiddleware(routes: HonoRoute[]): void {

0 commit comments

Comments
 (0)