Repository navigation
fix: trace parameter callbacks and preserve error attribution - #215
Merged
Merged
Conversation
Parameter callbacks bypassed layer instrumentation, so their errors were either missing from the error channel or attributed to a downstream error handler that merely forwarded them. Trace each executed parameter callback and share error publication and deduplication with layer handlers. Expose the actual callback as handler and its parameter name as param while retaining the associated Layer and request-scoped matched route pattern. This identifies the origin without modifying shared Layer metadata or Route.dispatch. Preserve parameter caching, callback arguments, routing signals and promise semantics. Skip tracing context allocation when there are no subscribers. Document the context fields and cover synchronous and asynchronous errors, concurrent requests, nested router copies and bound tracing stores with 24 regression tests. Refs #213. Validation: npm test (792 passing), npm run lint, npm run test-types.
Publishing asyncStart directly skipped its bound AsyncLocalStorage stores, leaving asyncEnd subscribers without the expected context. Errors raised before tracing was enabled or during route matching could also be blamed on handlers and mounted routers that merely forwarded them. Run async completion publication through asyncStart.runStores so bound stores remain active through asyncEnd without scheduling additional work. Keep the existing order of error propagation and lifecycle events. Record error identities at router boundaries, matching failures and error handler entry. Track them per request even when their origin was untraced, so a late subscriber cannot misattribute a forwarded error. Newly created replacement errors are still published at their origin, and the existing registered symbol preserves deduplication across router copies. Document these semantics and add 33 tests for bound stores, late subscriptions, incoming errors and malformed URLs in mounted routers. Validation: npm test (825 passing), npm run lint, npm run test-types.
Use one callParam helper for traced and untraced callbacks so exception, promise rejection and deprecation handling cannot drift between them. Keep completion notification optional and preserve tracing's end event in finally. Resolved promises still require an explicit next() call. Cover resolved promises without tracing and correct the rejection test to exercise the parameter callback itself. Validation: 826 tests passing, npm run lint, npm run test-types.
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.
Errors from
router.param()callbacks bypassed tracing, so they could go unreported or be attributed to an error handler that only forwarded them. Trace each executed parameter callback and publish its errors once, at their origin. In the example from #213,ctx.handler.namenow identifiesvalidateIdandctx.paramidentifiesid.The changes are split into two commits:
handlerandparamwhile retaining the associatedlayerand request-scoped matched route. Preserve parameter caching, callback arguments, routing signals and promise behavior. Skip tracing context allocation when there are no subscribers.asyncStart.runStoresso its bound stores remain active duringasyncEnd. Track error identities per request at router boundaries, matching failures and error-handler entry so errors originating before tracing was enabled, or during URL decoding, are not blamed on forwarding handlers. Replacement errors are still reported at their origin.Update the diagnostics documentation and add 57 regression tests covering parameter callbacks, synchronous and asynchronous errors, concurrent requests, nested router copies, bound stores, late subscriptions and malformed URLs.
Validation on Node 26.10.0:
npm test -- --reporter dot— 825 passing.npm run lint— passed.npm run test-types— passed.Fixes #213.