fix(fastify): guard error publishing against re-entrant recursion#8788
Conversation
A subscriber on the public apm:fastify:middleware:error channel that re-enters a wrapped hook while handling the error republishes through publishError, which republishes again, until the stack overflows (RangeError: Maximum call stack size exceeded during plugin registration). Two coordinated pieces close it: 1. publishError tracks an in-flight flag and drops the re-entrant publish instead of recursing; try/finally clears it even when a subscriber throws. 2. reply.send(error) routes through publishError rather than a bare errorChannel.publish, so the response-error path is guarded too. Drive-by fix: * preValidation forwards done() when parsingContexts holds no entry, instead of dereferencing a missing ctx in processInContext (TypeError when a param channel has a subscriber but onRequest took the fast path).
Overall package sizeSelf size: 6.17 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.0.1 | 82.56 kB | 817.39 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: de99867 | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-06-03 23:47:29 Comparing candidate commit de99867 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 1443 metrics, 23 unstable metrics. scenario:spans-finish-later-20
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de998676a8
ℹ️ 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".
| // No stored context means the onRequest/preParsing fast path ran (no error / | ||
| // cookie / callback subscribers), so there is nothing to publish on; forward | ||
| // `done` instead of dereferencing a missing ctx in processInContext. | ||
| if (!ctx) return done() |
There was a problem hiding this comment.
Preserve parser channels without a stored context
In the missing-context case this now returns before processInContext, but AppSec and IAST subscribe directly to the Fastify body/query/path channels (packages/dd-trace/src/appsec/index.js and .../iast/taint-tracking/plugin.js). When Fastify reaches preValidation without an entry in parsingContexts and any of those subscribers are enabled, this skips publishing the request body/query/params entirely, so WAF checks, aborts, and tainting are silently bypassed for that request; the crash fix should synthesize a minimal { req, res } context and continue through the parser publishing path instead of dropping it.
Useful? React with 👍 / 👎.
) A subscriber on the public apm:fastify:middleware:error channel that re-enters a wrapped hook while handling the error republishes through publishError, which republishes again, until the stack overflows (RangeError: Maximum call stack size exceeded during plugin registration). Two coordinated pieces close it: 1. publishError tracks an in-flight flag and drops the re-entrant publish instead of recursing; try/finally clears it even when a subscriber throws. 2. reply.send(error) routes through publishError rather than a bare errorChannel.publish, so the response-error path is guarded too. Drive-by fix: * preValidation forwards done() when parsingContexts holds no entry, instead of dereferencing a missing ctx in processInContext (TypeError when a param channel has a subscriber but onRequest took the fast path).
) A subscriber on the public apm:fastify:middleware:error channel that re-enters a wrapped hook while handling the error republishes through publishError, which republishes again, until the stack overflows (RangeError: Maximum call stack size exceeded during plugin registration). Two coordinated pieces close it: 1. publishError tracks an in-flight flag and drops the re-entrant publish instead of recursing; try/finally clears it even when a subscriber throws. 2. reply.send(error) routes through publishError rather than a bare errorChannel.publish, so the response-error path is guarded too. Drive-by fix: * preValidation forwards done() when parsingContexts holds no entry, instead of dereferencing a missing ctx in processInContext (TypeError when a param channel has a subscriber but onRequest took the fast path).
) A subscriber on the public apm:fastify:middleware:error channel that re-enters a wrapped hook while handling the error republishes through publishError, which republishes again, until the stack overflows (RangeError: Maximum call stack size exceeded during plugin registration). Two coordinated pieces close it: 1. publishError tracks an in-flight flag and drops the re-entrant publish instead of recursing; try/finally clears it even when a subscriber throws. 2. reply.send(error) routes through publishError rather than a bare errorChannel.publish, so the response-error path is guarded too. Drive-by fix: * preValidation forwards done() when parsingContexts holds no entry, instead of dereferencing a missing ctx in processInContext (TypeError when a param channel has a subscriber but onRequest took the fast path).
… 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
Summary
A subscriber on the public
apm:fastify:middleware:errorchannel that re-enters a wrapped hook while handling the error republishes throughpublishError, which republishes again, until the stack overflows (RangeError: Maximum call stack size exceededduring plugin registration).Two coordinated pieces close it:
publishErrortracks an in-flight flag and drops the re-entrant publish instead of recursing;try/finallyclears it even when a subscriber throws.reply.send(error)routes throughpublishErrorrather than a bareerrorChannel.publish, so the response-error path is guarded too.Drive-by
preValidationforwardsdone()whenparsingContextsholds no entry, instead of dereferencing a missingctxinprocessInContext(TypeErrorwhen a param channel has a subscriber butonRequesttook the fast path).Test plan
packages/datadog-instrumentations/test/fastify.spec.jsre-entrancy regression test fails without the guard (depth 51) and passes with it.This PR addresses: #8783