Skip to content

fix: observe rejection of a payload dropped after the reply was sent - #6936

Open
skdas20 wants to merge 2 commits into
fastify:mainfrom
skdas20:fix/dropped-thenable-rejection
Open

skdas20 wants to merge 2 commits into
fastify:mainfrom
skdas20:fix/dropped-thenable-rejection

Conversation

@skdas20

@skdas20 skdas20 commented Aug 10, 2026

Copy link
Copy Markdown

Split out of #6526, as offered there — this half stands on its own and does not depend on whether send(promise) becomes a supported API.

The bug

send() drops the payload when the reply has already been sent:

if (this.sent === true) {
  this.log.warn({ err: new FST_ERR_REP_ALREADY_SENT(...) })
  return this
}

If that dropped payload is a thenable, nothing has attached a handler to it. A later rejection therefore escapes as an unhandled rejection, which terminates the process by default on modern Node.

This is reachable from plain JavaScript today regardless of the declared types — any second send() in a handler that has already replied:

fastify.get('/', function (req, reply) {
  reply.send('immediate')
  reply.send(somethingThatRejectsLater())   // dropped, rejection unobserved
})

The fix

Attach a handler to the dropped thenable that logs the rejection, so it is reported rather than fatal. The payload is still discarded and the existing FST_ERR_REP_ALREADY_SENT warning is unchanged.

Testing

Added a promise rejecting after the reply was sent is logged, not left unhandled, asserting the response is unaffected and the rejection reaches the logger.

  • Fails on main (the rejection goes unhandled).
  • Passes with the change: 83/83 in test/internals/reply.test.js, lint clean.

@mcollina — this is option (2) from my last comment on #6526. I've kept it deliberately narrow so it can be judged on its own; happy to close #6526 entirely if you'd prefer the feature not to land.

send() discards the payload when the reply has already been sent. If that
payload is a thenable, nothing ever attaches a handler to it, so a later
rejection escapes as an unhandled rejection, which terminates the process
by default on modern Node.

Attach a handler that logs it instead. Reachable today from plain JS
regardless of the declared types, e.g. a second send() in a handler that
already replied.

Signed-off-by: skdas20 <skdas5405@gmail.com>
Comment thread lib/reply.js
// observe its outcome, so an eventual rejection would surface as an
// unhandled rejection and terminate the process by default. Attach a
// handler that reports it instead.
if (payload !== null && typeof payload === 'object' && typeof payload.then === 'function') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit:

Suggested change
if (payload !== null && typeof payload === 'object' && typeof payload.then === 'function') {
if (payload != null && typeof payload.then === 'function') {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants