fix(cloudflare): Add hono transaction name when error is thrown#18529
fix(cloudflare): Add hono transaction name when error is thrown#18529
Conversation
| activeSpan.updateName(`${context.req.method} ${routePath(context)}`); | ||
| updateSpanName(getRootSpan(activeSpan), `${context.req.method} ${routePath(context)}`); | ||
| } | ||
|
|
||
| getIsolationScope().setTransactionName(`${context.req.method} ${routePath(context)}`); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| if (shouldHandleError(err)) { | ||
| const activeSpan = getActiveSpan(); | ||
| if (activeSpan) { | ||
| activeSpan.updateName(`${context.req.method} ${routePath(context)}`); |
There was a problem hiding this comment.
l: "${context.req.method} ${routePath(context)}" is used 3 times. We could use a const at the beginning
| if (activeSpan) { | ||
| activeSpan.updateName(spanName); | ||
| updateSpanName(getRootSpan(activeSpan), spanName); | ||
| } |
There was a problem hiding this comment.
Bug: Renaming active span can corrupt traces
The error handler updates activeSpan.updateName(spanName) and also updates the root span name. If the currently active span at error time is not the root http.server span (e.g., a nested span is still active), this change can rename that child span to the transaction name and distort span descriptions in the trace.
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
| if (shouldHandleError(err)) { | ||
| if (context) { | ||
| const activeSpan = getActiveSpan(); | ||
| const spanName = `${context.req.method} ${routePath(context)}`; |
There was a problem hiding this comment.
Bug: Inconsistent null-safety between routePath and method access
The routePath function (vendored from Hono) uses optional chaining c.req?.path to defensively handle cases where req might be undefined, but the span name construction directly accesses context.req.method without the same protection. If context is truthy but context.req is unexpectedly undefined, accessing context.req.method would throw a TypeError. For consistency with the vendored defensive pattern, consider using context.req?.method with an appropriate fallback.
If the error is caught with the hono error handler, no
tracedata was sent alongside the event. This PR fixed this.Before:
After:
