feat: wrap route handlers with tracing at build time#4240
Merged
Conversation
Nitro resolves file-based routes at request time via h3["~findRoute"], bypassing h3["~routes"]. The h3 tracing plugin only wraps routes registered through ~routes / .on() / .use(), so file-based route handlers were never traced — only middleware spans were emitted. Use the new wrapFindRouteWithTracing helper exported from h3/tracing to wrap ~findRoute so route handler spans are emitted too. Ref: h3js/h3#1369 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@logaretm is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
📝 WalkthroughWalkthroughAdds conditional H3 tracing to the generated routing module: compute ChangesH3 Tracing Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Wrap Nitro’s file-based route resolution (h3["~findRoute"]) with the h3 tracing helper so route handlers discovered at request time emit h3.request tracing events with type: "route".
Changes:
- Import
wrapFindRouteWithTracingfromh3/tracingwhentracingChannel.h3is enabled - Emit generated setup code that wraps
nitroApp.h3["~findRoute"]after installing the h3 tracing plugin
commit: |
3 tasks
Use h3's new `wrapHandlerWithTracing` export to wrap compiled route handlers during codegen instead of patching ~findRoute at runtime. This ensures route traces are emitted with zero per-request overhead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
logaretm
changed the title
import wrapHandlerWithTracing from h3/tracing export
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/build/virtual/routing.ts (1)
95-112: ⚡ Quick win
tracedSerializeHandlerduplicatesserializeHandler's body — extract a factory.The two functions are structurally identical; they diverge only in whether the handler expression is wrapped with
__wrapHandler__(...). Any future addition to the serialized shape (e.g. a new metadata field) must be kept in sync manually across both.A closure factory eliminates the duplication while remaining compatible with
compileToString's(h) => stringcallback contract:♻️ Proposed refactor — factory-based serializer
-function serializeHandler(h: MaybeArray<NitroEventHandler & { _importHash: string }>): string { - const meta = Array.isArray(h) ? h[0] : h; - - return `{${[ - `route:${JSON.stringify(meta.route)}`, - meta.method && `method:${JSON.stringify(meta.method)}`, - meta.meta && `meta:${JSON.stringify(meta.meta)}`, - `handler:${ - Array.isArray(h) - ? `multiHandler(${h.map((handler) => serializeHandlerFn(handler)).join(",")})` - : serializeHandlerFn(h) - }`, - ] - .filter(Boolean) - .join(",")}}`; -} - -function tracedSerializeHandler( - h: MaybeArray<NitroEventHandler & { _importHash: string }> -): string { - const meta = Array.isArray(h) ? h[0] : h; - - return `{${[ - `route:${JSON.stringify(meta.route)}`, - meta.method && `method:${JSON.stringify(meta.method)}`, - meta.meta && `meta:${JSON.stringify(meta.meta)}`, - `handler:__wrapHandler__(${ - Array.isArray(h) - ? `multiHandler(${h.map((handler) => serializeHandlerFn(handler)).join(",")})` - : serializeHandlerFn(h) - })`, - ] - .filter(Boolean) - .join(",")}}`; -} +function makeSerializeHandler(wrapWith?: string) { + return (h: MaybeArray<NitroEventHandler & { _importHash: string }>): string => { + const meta = Array.isArray(h) ? h[0] : h; + const handlerCode = Array.isArray(h) + ? `multiHandler(${h.map((handler) => serializeHandlerFn(handler)).join(",")})` + : serializeHandlerFn(h); + + return `{${[ + `route:${JSON.stringify(meta.route)}`, + meta.method && `method:${JSON.stringify(meta.method)}`, + meta.meta && `meta:${JSON.stringify(meta.meta)}`, + `handler:${wrapWith ? `${wrapWith}(${handlerCode})` : handlerCode}`, + ] + .filter(Boolean) + .join(",")}}`; + }; +} + +const serializeHandler = makeSerializeHandler(); +const tracedSerializeHandler = makeSerializeHandler("__wrapHandler__");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/build/virtual/routing.ts` around lines 95 - 112, Extract the common serialization logic from serializeHandler and tracedSerializeHandler into a factory function (e.g., makeSerializeHandler) that returns a (h) => string serializer; have makeSerializeHandler accept a single option/flag or wrapper function to apply to the handler expression (so tracedSerializeHandler calls makeSerializeHandler with a wrapper that wraps the handler expression with "__wrapHandler__(...)" and serializeHandler calls it with an identity/no-op wrapper), reuse serializeHandlerFn and multiHandler inside the factory to build the handler expression, and update both serializeHandler and tracedSerializeHandler to delegate to the new factory so future metadata fields are added once in the shared factory rather than duplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/build/virtual/routing.ts`:
- Around line 95-112: Extract the common serialization logic from
serializeHandler and tracedSerializeHandler into a factory function (e.g.,
makeSerializeHandler) that returns a (h) => string serializer; have
makeSerializeHandler accept a single option/flag or wrapper function to apply to
the handler expression (so tracedSerializeHandler calls makeSerializeHandler
with a wrapper that wraps the handler expression with "__wrapHandler__(...)" and
serializeHandler calls it with an identity/no-op wrapper), reuse
serializeHandlerFn and multiHandler inside the factory to build the handler
expression, and update both serializeHandler and tracedSerializeHandler to
delegate to the new factory so future metadata fields are added once in the
shared factory rather than duplicated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d43f53de-ace4-4fbf-b9b9-5414dd08b122
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.jsonsrc/build/virtual/routing.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/unit/virtual-routing.test.ts (1)
53-54: ⚡ Quick winMake the positive-case assertion less brittle to codegen formatting changes.
The exact
toContain(...)for the full wrapper call can fail on benign serialization differences. Prefer a regex that checks semantic wrapping while allowing minor formatting/identifier variance.Proposed test assertion tweak
- expect(template).toContain("wrapHandlerWithTracing(h3.toEventHandler(_abc123))"); + expect(template).toMatch( + /wrapHandlerWithTracing\(h3\.toEventHandler\(_[A-Za-z0-9]+\)\)/ + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/unit/virtual-routing.test.ts` around lines 53 - 54, The test is brittle because it asserts the exact string for the wrapper call; change the assertion to use a regex that checks for semantic wrapping of wrapHandlerWithTracing around h3.toEventHandler while allowing spacing and identifier differences. Replace the expect(template).toContain("wrapHandlerWithTracing(h3.toEventHandler(_abc123))") check with an assertion that template matches a regex like /wrapHandlerWithTracing\(\s*h3\.toEventHandler\(/ (referencing the template variable and the wrapHandlerWithTracing and h3.toEventHandler symbols) so minor formatting or identifier changes won’t break the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@test/unit/virtual-routing.test.ts`:
- Around line 53-54: The test is brittle because it asserts the exact string for
the wrapper call; change the assertion to use a regex that checks for semantic
wrapping of wrapHandlerWithTracing around h3.toEventHandler while allowing
spacing and identifier differences. Replace the
expect(template).toContain("wrapHandlerWithTracing(h3.toEventHandler(_abc123))")
check with an assertion that template matches a regex like
/wrapHandlerWithTracing\(\s*h3\.toEventHandler\(/ (referencing the template
variable and the wrapHandlerWithTracing and h3.toEventHandler symbols) so minor
formatting or identifier changes won’t break the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 449e8ab8-c1d4-4f4d-9716-353620af5bcf
📒 Files selected for processing (1)
test/unit/virtual-routing.test.ts
RihanArfan
pushed a commit
that referenced
this pull request
May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
wrapHandlerWithTracingfromh3/tracingduring codegen, ensuring route traces are emitted for file-based routing with zero per-request overheadwrapFindRouteWithTracingcall from the tracing virtual module — tracing is now applied at build time in the routing virtual moduletracingChannel.h3is enabled; otherwise codegen is unchangedCompanion PR
wrapHandlerWithTracingfor manual handler wrapping h3js/h3#1369Test plan
tracingChannel.h3enabled and verify route handlers emith3.requestdiagnostics_channel tracesh3/tracingimport in generated routing module🤖 Generated with Claude Code