Skip to content

Commit 6b86b99

Browse files
logaretmclaude
andcommitted
refactor: export wrapFindRouteWithTracing helper, remove plugin-side ~findRoute patching
Per maintainer feedback, move ~findRoute wrapping out of the tracing plugin and into a standalone exported helper. Frameworks like nitro can opt in by calling wrapFindRouteWithTracing() on their own ~findRoute function, keeping h3's plugin lean and the runtime cost explicit. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0abccd7 commit 6b86b99

2 files changed

Lines changed: 90 additions & 166 deletions

File tree

src/tracing.ts

Lines changed: 71 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import type { H3Event } from "./event.ts";
22
import type { H3, H3Core } from "./h3.ts";
3-
import { type H3Plugin, type H3Route, type MiddlewareOptions } from "./types/h3.ts";
3+
import {
4+
type H3Plugin,
5+
type H3Route,
6+
type MatchedRoute,
7+
type MiddlewareOptions,
8+
} from "./types/h3.ts";
49
import type { EventHandler, Middleware } from "./types/handler.ts";
510

611
/**
@@ -83,40 +88,6 @@ export function tracingPlugin(traceOpts?: TracingPluginOptions): H3Plugin {
8388
} satisfies H3Route;
8489
});
8590

86-
// Wrap route handlers returned by ~findRoute so frameworks that resolve
87-
// routes at request time (e.g. nitro file-based routes) without pushing
88-
// into ~routes still emit route traces. A fresh wrapper is rebuilt on
89-
// every assignment so previously-captured references stay stable — a
90-
// framework pattern like `const prev = app["~findRoute"]; app["~findRoute"]
91-
// = (e) => prev(e)` must not recurse into itself.
92-
const wrapFindRoute = (fn: H3Core["~findRoute"]): H3Core["~findRoute"] => {
93-
return (event: H3Event) => {
94-
const route = fn.call(h3, event);
95-
// Mutate route.data in place: findRoute often returns shared references
96-
// (e.g. entries from ~routes), and the wrappers are idempotent via
97-
// __traced__, so repeated calls on the same object are cheap. Do not
98-
// clone — a cloned route would re-wrap on every request.
99-
if (route?.data.handler) {
100-
route.data.handler = wrapEventHandler(route.data.handler);
101-
}
102-
if (route?.data.middleware) {
103-
for (let i = 0; i < route.data.middleware.length; i++) {
104-
route.data.middleware[i] = wrapMiddleware(route.data.middleware[i]);
105-
}
106-
}
107-
return route;
108-
};
109-
};
110-
let wrappedFindRoute = wrapFindRoute(h3["~findRoute"]);
111-
Object.defineProperty(h3, "~findRoute", {
112-
configurable: true,
113-
enumerable: false,
114-
get: () => wrappedFindRoute,
115-
set: (fn: H3Core["~findRoute"]) => {
116-
wrappedFindRoute = wrapFindRoute(fn);
117-
},
118-
});
119-
12091
if ("on" in h3 && typeof h3.on === "function") {
12192
const originalOn = h3.on;
12293

@@ -176,3 +147,68 @@ export function tracingPlugin(traceOpts?: TracingPluginOptions): H3Plugin {
176147
return h3;
177148
};
178149
}
150+
151+
type FindRouteFunction = (event: H3Event) => MatchedRoute<H3Route> | void;
152+
153+
/**
154+
* Wraps a `~findRoute` function so that returned route handlers and middleware
155+
* are traced via the `h3.request` diagnostics channel. Intended for frameworks
156+
* (e.g. nitro) that resolve routes at request time without pushing them into
157+
* `h3["~routes"]`.
158+
*
159+
* Returns the original function unchanged when `diagnostics_channel` is not
160+
* available.
161+
*/
162+
export function wrapFindRouteWithTracing(
163+
findRoute: FindRouteFunction,
164+
traceOpts?: TracingPluginOptions,
165+
): FindRouteFunction {
166+
const { tracingChannel } = globalThis.process?.getBuiltinModule?.("diagnostics_channel") ?? {};
167+
168+
if (!tracingChannel) {
169+
return findRoute;
170+
}
171+
172+
const channel = tracingChannel("h3.request");
173+
174+
function wrapHandler(handler: MaybeTracedEventHandler): EventHandler {
175+
if (handler.__traced__ || traceOpts?.traceRoutes === false) {
176+
return handler;
177+
}
178+
const wrapped: MaybeTracedEventHandler = (...args) => {
179+
return channel.tracePromise(async () => handler(...args), {
180+
event: args[0],
181+
type: "route",
182+
} satisfies TracingRequestEvent);
183+
};
184+
wrapped.__traced__ = true;
185+
return wrapped;
186+
}
187+
188+
function wrapMiddleware(middleware: MaybeTracedMiddleware): Middleware {
189+
if (middleware.__traced__ || traceOpts?.traceMiddleware === false) {
190+
return middleware;
191+
}
192+
const wrapped: MaybeTracedMiddleware = (...args) => {
193+
return channel.tracePromise(async () => middleware(...args), {
194+
event: args[0],
195+
type: "middleware",
196+
} satisfies TracingRequestEvent);
197+
};
198+
wrapped.__traced__ = true;
199+
return wrapped;
200+
}
201+
202+
return (event: H3Event) => {
203+
const route = findRoute(event);
204+
if (route?.data.handler) {
205+
route.data.handler = wrapHandler(route.data.handler);
206+
}
207+
if (route?.data.middleware) {
208+
for (let i = 0; i < route.data.middleware.length; i++) {
209+
route.data.middleware[i] = wrapMiddleware(route.data.middleware[i]);
210+
}
211+
}
212+
return route;
213+
};
214+
}

test/tracing.test.ts

Lines changed: 19 additions & 131 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,47 +1156,38 @@ describe("tracing channels for H3Core instances", () => {
11561156
listener.cleanup();
11571157
}
11581158
});
1159+
});
11591160

1160-
it("traces handlers returned from a custom ~findRoute (nitro-style file routes)", async () => {
1161+
describe("wrapFindRouteWithTracing", () => {
1162+
it("wraps route handlers returned by findRoute", async () => {
11611163
const listener = createTracingListener();
11621164
const { H3Core } = await import("../src/h3.ts");
1163-
const { tracingPlugin } = await import("../src/tracing.ts");
1165+
const { wrapFindRouteWithTracing } = await import("../src/tracing.ts");
11641166
const { H3Event } = await import("../src/event.ts");
11651167

11661168
try {
11671169
const app = new H3Core();
11681170
const routeHandler = () => "file-based response";
11691171

1170-
// Simulate a framework that resolves routes at request time and never
1171-
// pushes them into app["~routes"]. The handler only exists inside the
1172-
// custom ~findRoute implementation.
1173-
app["~findRoute"] = (event: any) => {
1172+
const baseFindRoute = (event: any) => {
11741173
if (event.url.pathname === "/file-route" && event.req.method === "GET") {
11751174
return {
1176-
data: {
1177-
method: "GET",
1178-
route: "/file-route",
1179-
handler: routeHandler,
1180-
},
1175+
data: { method: "GET", route: "/file-route", handler: routeHandler },
11811176
params: {},
11821177
};
11831178
}
11841179
return undefined;
11851180
};
11861181

1187-
// Apply tracing plugin after ~findRoute is set to verify the wrapper
1188-
// picks up the already-installed resolver.
1189-
tracingPlugin()(app as any);
1182+
app["~findRoute"] = wrapFindRouteWithTracing(baseFindRoute as any);
11901183

11911184
const request = new Request("http://localhost/file-route", { method: "GET" });
11921185
const event = new H3Event(request, undefined, app as any);
11931186

11941187
await app.handler(event);
1195-
11961188
await new Promise((resolve) => setTimeout(resolve, 10));
11971189

11981190
const routeEvents = listener.events.filter((e) => e.asyncStart?.data.type === "route");
1199-
12001191
expect(routeEvents.some((e) => e.asyncStart?.data.event.url.pathname === "/file-route")).toBe(
12011192
true,
12021193
);
@@ -1205,124 +1196,22 @@ describe("tracing channels for H3Core instances", () => {
12051196
}
12061197
});
12071198

1208-
it("traces handlers when ~findRoute is reassigned after tracingPlugin is applied", async () => {
1209-
const listener = createTracingListener();
1210-
const { H3Core } = await import("../src/h3.ts");
1211-
const { tracingPlugin } = await import("../src/tracing.ts");
1212-
const { H3Event } = await import("../src/event.ts");
1213-
1214-
try {
1215-
const app = new H3Core();
1216-
const routeHandler = () => "late-bound response";
1217-
1218-
// Apply plugin BEFORE the framework wires up routing (typical nitro
1219-
// order: plugins register, then file-based routing installs ~findRoute).
1220-
tracingPlugin()(app as any);
1221-
1222-
app["~findRoute"] = (event: any) => {
1223-
if (event.url.pathname === "/late-route" && event.req.method === "GET") {
1224-
return {
1225-
data: {
1226-
method: "GET",
1227-
route: "/late-route",
1228-
handler: routeHandler,
1229-
},
1230-
params: {},
1231-
};
1232-
}
1233-
return undefined;
1234-
};
1235-
1236-
const request = new Request("http://localhost/late-route", { method: "GET" });
1237-
const event = new H3Event(request, undefined, app as any);
1238-
1239-
await app.handler(event);
1240-
await new Promise((resolve) => setTimeout(resolve, 10));
1241-
1242-
const routeEvents = listener.events.filter((e) => e.asyncStart?.data.type === "route");
1243-
expect(routeEvents.some((e) => e.asyncStart?.data.event.url.pathname === "/late-route")).toBe(
1244-
true,
1245-
);
1246-
} finally {
1247-
listener.cleanup();
1248-
}
1249-
});
1250-
1251-
it("keeps ~findRoute non-enumerable after tracingPlugin is applied", async () => {
1252-
const { H3Core } = await import("../src/h3.ts");
1253-
const { tracingPlugin } = await import("../src/tracing.ts");
1254-
1255-
const app = new H3Core();
1256-
tracingPlugin()(app as any);
1257-
1258-
expect(Object.keys(app)).not.toContain("~findRoute");
1259-
const descriptor = Object.getOwnPropertyDescriptor(app, "~findRoute");
1260-
expect(descriptor?.enumerable).toBe(false);
1261-
});
1262-
1263-
it("does not recurse when a framework wraps ~findRoute via captured reference", async () => {
1264-
const listener = createTracingListener();
1265-
const { H3Core } = await import("../src/h3.ts");
1266-
const { tracingPlugin } = await import("../src/tracing.ts");
1267-
const { H3Event } = await import("../src/event.ts");
1268-
1269-
try {
1270-
const app = new H3Core();
1271-
const routeHandler = () => "ok";
1272-
1273-
tracingPlugin()(app as any);
1274-
1275-
// Common framework pattern: capture current ~findRoute then replace with
1276-
// a wrapper that delegates to the captured reference. Must not recurse.
1277-
const prev = app["~findRoute"];
1278-
app["~findRoute"] = (event: any) => {
1279-
const matched = prev.call(app, event);
1280-
if (matched) return matched;
1281-
if (event.url.pathname === "/wrapped" && event.req.method === "GET") {
1282-
return {
1283-
data: { method: "GET", route: "/wrapped", handler: routeHandler },
1284-
params: {},
1285-
};
1286-
}
1287-
return undefined;
1288-
};
1289-
1290-
const request = new Request("http://localhost/wrapped", { method: "GET" });
1291-
const event = new H3Event(request, undefined, app as any);
1292-
1293-
await app.handler(event);
1294-
await new Promise((resolve) => setTimeout(resolve, 10));
1295-
1296-
const routeEvents = listener.events.filter((e) => e.asyncStart?.data.type === "route");
1297-
expect(routeEvents.length).toBe(1);
1298-
} finally {
1299-
listener.cleanup();
1300-
}
1301-
});
1302-
1303-
it("does not double-wrap handlers across repeated ~findRoute calls", async () => {
1199+
it("does not double-wrap handlers across repeated calls", async () => {
13041200
const listener = createTracingListener();
13051201
const { H3Core } = await import("../src/h3.ts");
1306-
const { tracingPlugin } = await import("../src/tracing.ts");
1202+
const { wrapFindRouteWithTracing } = await import("../src/tracing.ts");
13071203
const { H3Event } = await import("../src/event.ts");
13081204

13091205
try {
13101206
const app = new H3Core();
13111207
const routeHandler = () => "ok";
13121208

1313-
// Cached route object returned from every ~findRoute call — mirrors a
1314-
// framework that memoizes resolved routes.
13151209
const cachedRoute = {
1316-
data: {
1317-
method: "GET" as const,
1318-
route: "/cached",
1319-
handler: routeHandler,
1320-
},
1210+
data: { method: "GET" as const, route: "/cached", handler: routeHandler },
13211211
params: {},
13221212
};
1323-
app["~findRoute"] = () => cachedRoute;
13241213

1325-
tracingPlugin()(app as any);
1214+
app["~findRoute"] = wrapFindRouteWithTracing((() => cachedRoute) as any);
13261215

13271216
const run = async () => {
13281217
const request = new Request("http://localhost/cached", { method: "GET" });
@@ -1331,24 +1220,27 @@ describe("tracing channels for H3Core instances", () => {
13311220
};
13321221

13331222
await run();
1334-
// Handler wrapped in place on first call; reference should stay stable
1335-
// across subsequent calls (wrapEventHandler short-circuits on __traced__).
13361223
const wrappedHandler = cachedRoute.data.handler;
13371224
expect((wrappedHandler as any).__traced__).toBe(true);
13381225

13391226
await run();
13401227
await run();
13411228

13421229
expect(cachedRoute.data.handler).toBe(wrappedHandler);
1230+
1231+
await new Promise((resolve) => setTimeout(resolve, 10));
1232+
1233+
const routeAsyncStarts = listener.events.filter((e) => e.asyncStart?.data.type === "route");
1234+
expect(routeAsyncStarts.length).toBe(3);
13431235
} finally {
13441236
listener.cleanup();
13451237
}
13461238
});
13471239

1348-
it("mutates cached route.data.middleware in place without reallocating the array", async () => {
1240+
it("mutates cached route.data.middleware in place without reallocating", async () => {
13491241
const listener = createTracingListener();
13501242
const { H3Core } = await import("../src/h3.ts");
1351-
const { tracingPlugin } = await import("../src/tracing.ts");
1243+
const { wrapFindRouteWithTracing } = await import("../src/tracing.ts");
13521244
const { H3Event } = await import("../src/event.ts");
13531245

13541246
try {
@@ -1365,9 +1257,8 @@ describe("tracing channels for H3Core instances", () => {
13651257
},
13661258
params: {},
13671259
};
1368-
app["~findRoute"] = () => cachedRoute;
13691260

1370-
tracingPlugin()(app as any);
1261+
app["~findRoute"] = wrapFindRouteWithTracing((() => cachedRoute) as any);
13711262

13721263
const run = async () => {
13731264
const request = new Request("http://localhost/cached", { method: "GET" });
@@ -1383,15 +1274,12 @@ describe("tracing channels for H3Core instances", () => {
13831274
await run();
13841275
await run();
13851276

1386-
// Array identity and entry identity must remain stable — no per-request
1387-
// allocations once the middleware is already traced.
13881277
expect(cachedRoute.data.middleware).toBe(middlewareArray);
13891278
expect(cachedRoute.data.middleware[0]).toBe(wrappedMw);
13901279

13911280
await new Promise((resolve) => setTimeout(resolve, 10));
13921281

13931282
const routeAsyncStarts = listener.events.filter((e) => e.asyncStart?.data.type === "route");
1394-
// Exactly one route trace per request — no double-wrapping.
13951283
expect(routeAsyncStarts.length).toBe(3);
13961284
} finally {
13971285
listener.cleanup();

0 commit comments

Comments
 (0)