fix(fastify): bound error-channel recursion and stop swallowing async hook rejections#9118
Merged
Merged
Conversation
Contributor
Overall package sizeSelf size: 6.53 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: a7a0b06 | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9118 +/- ##
==========================================
- Coverage 93.69% 93.64% -0.05%
==========================================
Files 889 898 +9
Lines 50856 52370 +1514
Branches 11830 12325 +495
==========================================
+ Hits 47647 49042 +1395
- Misses 3209 3328 +119 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
BenchmarksBenchmark execution time: 2026-07-02 16:48:28 Comparing candidate commit a7a0b06 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2255 metrics, 31 unstable metrics.
|
… hook rejections Fastify boots through avvio, whose `_encapsulateThreeParam` re-invokes an encapsulated hook after it throws. The same error object rides every re-drive, and the wrapped hook republishes it on `apm:fastify:middleware:error` each time. The in-flight boolean added in #8788 only blocks a publish nested inside another publish; a sequential re-drive runs after the publish returned and its `finally` cleared the flag, so every re-drive publishes again, the channel subscriber recurses, and boot dies with `RangeError: Maximum call stack size exceeded`. A persistent `DatadogRaspAbortError` (appsec RASP) or the boot deprecation warning is the error that rides the loop. 1. `createErrorPublisher` keeps the boolean for nested re-entry and remembers the last published error. The re-drive re-throws the one caught error on every hop, so a single reference compare against the previous publish terminates the loop where the boolean cannot. This costs one comparison per publish rather than a per-publish set lookup, and it does not mutate the error. A distinct error still reaches subscribers; only a re-drive alternating two distinct persistent errors stays uncollapsed, which is not a shape fastify produces. 2. `invokeHookWithContext`'s async branch returned `publishError(ctx)`, i.e. `ctx.error`, from the `.catch` handler, which resolves the promise with the error and silently swallows the rejection. It now publishes and re-throws so the rejection keeps propagating. The synchronous branch is decoupled the same way: publish, then throw the original error rather than throwing the publisher's return value. Fixes: #9099 Refs: #8783
BridgeAR
force-pushed
the
BridgeAR/2026-06-27-fastify-error-recursion
branch
from
June 27, 2026 22:38
25aa397 to
aba83d9
Compare
… shared publisher The error-keyed dedupe that bounds avvio's boot re-drive lived in the shared `createErrorPublisher`, but koa, router, connect and restify republish the one thrown error once per unwound middleware layer so each layer's span gets tagged on its own publish. Keying the shared guard on the error object dropped every publish after the first, so an outer middleware span never received its `error.type` tag and the koa request-error spec went red on every version. Move the `WeakSet` to fastify's own `publishError`, where the sequential re-drive of the same error object actually happens, and revert the shared publisher to the synchronous re-entry flag the other frameworks need. Refs: #9099
…plit the rejection publish The avvio re-drive re-throws the one caught error on every hop, so a reference compare against the previously published error terminates the loop without the allocation and per-publish lookup of a WeakSet; fastify's lifecycle never feeds the publisher an A-B-A sequence the compare would miss. Split the promise branch so the rejection handler only publishes: the wrapper hands back the original promise, which keeps rejecting untouched, instead of re-throwing from a `.catch` that returns a new promise. The publish is a side effect, the propagation is the return value, and the two no longer share a line. Refs: #9099
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d544ad623
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
crysmags
requested changes
Jul 2, 2026
crysmags
left a comment
crysmags
left a comment
Collaborator
There was a problem hiding this comment.
one blocker, otherwise this looks good.
lastPublishedError is still keyed only on the Error object. That fixes the avvio re-drive, but it also drops a later request if the app reuses the same Error instance.
The last-error guard that bounds avvio's boot re-drive (#9099) was keyed on the error object alone, so two distinct requests reusing one cached `Error` instance collapsed to a single publish. The error channel's subscribers tag `web.addError(req, error)` per request, so request two never reached them and its span went untagged. Key the guard on the error object plus its request. A re-drive of the same error against the same request still collapses - boot hooks carry no request, so their re-drives share the same absent request and bound the recursion - while a distinct request reusing a cached error publishes again. Refs: #9099
The re-drive re-throws the one caught error on the trailing hop and a request publishes its error once, so the guard only ever compares against the single previous publish. A pair of module-level scalars (last error, last request) answers that with two reference compares and no allocation, dropping the per-error WeakMap lookup and side table the previous shape carried. Refs: #9099
dd-octo-sts Bot
pushed a commit
that referenced
this pull request
Jul 7, 2026
…ook rejections (#9118) Fastify boots through avvio, whose `_encapsulateThreeParam` re-invokes an encapsulated hook after it throws, re-throwing the same error object on every sequential re-drive. Each re-drive runs after the previous publish returned, so the shared publisher's synchronous re-entry boolean had already reset; every re-drive republished on `apm:fastify:middleware:error`, the channel subscriber recursed, and boot died with `RangeError: Maximum call stack size exceeded`. 1. `publishError` now guards on two module-level scalars - the last published error and its request, compared by reference - so a re-drive of the same error against the same request collapses to one publish. Boot hooks carry no request, so their re-drives share the same absent request and still collapse; a distinct request reusing a cached `Error` instance still publishes, since the subscribers tag `web.addError(req, error)` per request. The shared `createErrorPublisher` keeps the plain re-entry boolean instead, since koa, connect, restify and router each republish the one thrown error once per unwound middleware layer to tag a distinct span. 2. `invokeHookWithContext`'s async branch returned the `.catch` handler's promise, which resolved with the error and silently swallowed the rejection. It now observes the rejection to publish and returns the original promise unchanged, so the rejection keeps propagating; the synchronous branch is decoupled the same way, throwing the caught error instead of the publisher's return value. Fixes: #9099 Refs: #8783
Merged
dd-octo-sts Bot
pushed a commit
that referenced
this pull request
Jul 7, 2026
…ook rejections (#9118) Fastify boots through avvio, whose `_encapsulateThreeParam` re-invokes an encapsulated hook after it throws, re-throwing the same error object on every sequential re-drive. Each re-drive runs after the previous publish returned, so the shared publisher's synchronous re-entry boolean had already reset; every re-drive republished on `apm:fastify:middleware:error`, the channel subscriber recursed, and boot died with `RangeError: Maximum call stack size exceeded`. 1. `publishError` now guards on two module-level scalars - the last published error and its request, compared by reference - so a re-drive of the same error against the same request collapses to one publish. Boot hooks carry no request, so their re-drives share the same absent request and still collapse; a distinct request reusing a cached `Error` instance still publishes, since the subscribers tag `web.addError(req, error)` per request. The shared `createErrorPublisher` keeps the plain re-entry boolean instead, since koa, connect, restify and router each republish the one thrown error once per unwound middleware layer to tag a distinct span. 2. `invokeHookWithContext`'s async branch returned the `.catch` handler's promise, which resolved with the error and silently swallowed the rejection. It now observes the rejection to publish and returns the original promise unchanged, so the rejection keeps propagating; the synchronous branch is decoupled the same way, throwing the caught error instead of the publisher's return value. Fixes: #9099 Refs: #8783
Merged
juan-fernandez
pushed a commit
that referenced
this pull request
Jul 8, 2026
…ook rejections (#9118) Fastify boots through avvio, whose `_encapsulateThreeParam` re-invokes an encapsulated hook after it throws, re-throwing the same error object on every sequential re-drive. Each re-drive runs after the previous publish returned, so the shared publisher's synchronous re-entry boolean had already reset; every re-drive republished on `apm:fastify:middleware:error`, the channel subscriber recursed, and boot died with `RangeError: Maximum call stack size exceeded`. 1. `publishError` now guards on two module-level scalars - the last published error and its request, compared by reference - so a re-drive of the same error against the same request collapses to one publish. Boot hooks carry no request, so their re-drives share the same absent request and still collapse; a distinct request reusing a cached `Error` instance still publishes, since the subscribers tag `web.addError(req, error)` per request. The shared `createErrorPublisher` keeps the plain re-entry boolean instead, since koa, connect, restify and router each republish the one thrown error once per unwound middleware layer to tag a distinct span. 2. `invokeHookWithContext`'s async branch returned the `.catch` handler's promise, which resolved with the error and silently swallowed the rejection. It now observes the rejection to publish and returns the original promise unchanged, so the rejection keeps propagating; the synchronous branch is decoupled the same way, throwing the caught error instead of the publisher's return value. Fixes: #9099 Refs: #8783
juan-fernandez
pushed a commit
that referenced
this pull request
Jul 8, 2026
…ook rejections (#9118) Fastify boots through avvio, whose `_encapsulateThreeParam` re-invokes an encapsulated hook after it throws, re-throwing the same error object on every sequential re-drive. Each re-drive runs after the previous publish returned, so the shared publisher's synchronous re-entry boolean had already reset; every re-drive republished on `apm:fastify:middleware:error`, the channel subscriber recursed, and boot died with `RangeError: Maximum call stack size exceeded`. 1. `publishError` now guards on two module-level scalars - the last published error and its request, compared by reference - so a re-drive of the same error against the same request collapses to one publish. Boot hooks carry no request, so their re-drives share the same absent request and still collapse; a distinct request reusing a cached `Error` instance still publishes, since the subscribers tag `web.addError(req, error)` per request. The shared `createErrorPublisher` keeps the plain re-entry boolean instead, since koa, connect, restify and router each republish the one thrown error once per unwound middleware layer to tag a distinct span. 2. `invokeHookWithContext`'s async branch returned the `.catch` handler's promise, which resolved with the error and silently swallowed the rejection. It now observes the rejection to publish and returns the original promise unchanged, so the rejection keeps propagating; the synchronous branch is decoupled the same way, throwing the caught error instead of the publisher's return value. Fixes: #9099 Refs: #8783
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
2 participants
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
Fastify boots through avvio, whose
_encapsulateThreeParamre-invokes an encapsulated hook after it throws. The same error object rides every re-drive, and the wrapped hook republishes it onapm:fastify:middleware:erroreach time. The in-flight boolean added in #8788 only blocks a publish nested inside another publish; a sequential re-drive runs after the publish returned and itsfinallycleared the flag, so every re-drive publishes again, the channel subscriber recurses, and boot dies withRangeError: Maximum call stack size exceeded. A persistentDatadogRaspAbortError(appsec RASP) or the boot deprecation warning is the error that rides the loop.createErrorPublisherkeeps the boolean for nested re-entry and adds aWeakSetkeyed on the error object, so the same error publishes at most once per channel — bounding the sequential re-drive a boolean cannot. A genuinely distinct error still reaches subscribers; the entry frees once the error object is unreachable.invokeHookWithContext's async branch returnedpublishError(ctx)(i.e.ctx.error) from the.catchhandler, which resolves the promise with the error and silently swallows the rejection. It now publishes and re-throws so the rejection keeps propagating. The synchronous branch is decoupled the same way: publish, then throw the original error rather than the publisher's return value.Why
The error channel's two subscribers — the router plugin's
web.addError(tag the request span once) and appsec RASP'sblock(block the response once) — both want once-per-error cardinality, so keying the guard on the error object matches what they need. The residual trade-off is that the same error object reaching the channel through two genuinely distinct contexts publishes once; that is exactly the recursion being bounded, and once-per-error is the correct cardinality for both subscribers.The real avvio re-drive needs a specific plugin/encapsulation arrangement from the reporter's app that I could not reconstruct, so the regression pins the precise invariant at the real seam instead: the real wrapped hook, driven the way avvio drives a re-thrown hook, publishes a persistent error exactly once across thousands of re-drives (fails on
masterwith one publish per re-drive), a distinct error still publishes each time, and a rejecting async hook re-rejects instead of resolving-with-error.Test plan
packages/datadog-instrumentations/test/fastify.spec.js(exercised by theinstrumentation-fastifyCI job): the three added/changed cases fail onmasterand pass here.createErrorPublisher(koa,connect,restify,router) — instrumentation specs stay green.Fixes: #9099
Refs: #8783