Skip to content

fix(node,fastify): handle synchronous throws on the response-send path - #51

Merged
dinwwwh merged 2 commits into
mainfrom
claude/response-send-throw-leak-56af52
Aug 3, 2026
Merged

fix(node,fastify): handle synchronous throws on the response-send path#51
dinwwwh merged 2 commits into
mainfrom
claude/response-send-throw-leak-56af52

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Aug 3, 2026

Copy link
Copy Markdown
Member

A synchronous throw while sending a response (an invalid header value in the node adapter, FST_ERR_BAD_STATUS_CODE in fastify) used to reject the promise while leaving the connection neither ended nor destroyed and the already-built body stream leaked. In the node adapter the client would hang until it gave up, since Node has no default response-send timeout.

Fixes

  • Both adapters wrap the status/header/send sequence in try/catch: on failure the body stream is destroyed (a caller-supplied ReadableStream gets its cancel() called instead of staying locked), and the promise rejects with the original error.
  • The node adapter also destroys the response socket, so the connection is no longer left half-open with partially applied headers.
  • The fastify adapter deliberately leaves reply.raw intact — its error handler still delivers the 500.
  • canWriteToNodeResponse now treats headersSent as non-writable, so a response whose headers were flushed upstream takes the graceful not-writable path instead of throwing ERR_HTTP_HEADERS_SENT with headers half-applied.

Testing

  • New regression tests: invalid header value rejects and destroys both response and body; already-flushed headers resolve cleanly; fastify status outside [100, 599] rejects, cancels the body stream, and still returns fastify's 500.
  • Full workspace suite passes (912 tests, 59 files) and all packages typecheck.

A synchronous throw while applying status/headers or sending (e.g. an
invalid header value, or FST_ERR_BAD_STATUS_CODE in fastify) previously
rejected the promise while leaving the connection neither ended nor
destroyed and the built body stream leaked.

Both adapters now wrap the send sequence in try/catch: the body stream
is destroyed, the node response socket is destroyed so the client isn't
left hanging, and the promise rejects with the original error. Fastify
keeps reply.raw intact so its error handler can still respond.

canWriteToNodeResponse also treats headersSent as non-writable, so
responses whose headers were flushed upstream take the graceful
not-writable path instead of throwing ERR_HTTP_HEADERS_SENT.
@pkg-pr-new

pkg-pr-new Bot commented Aug 3, 2026

Copy link
Copy Markdown
@standardserver/aws-lambda

npm i https://pkg.pr.new/@standardserver/aws-lambda@51

@standardserver/bun

npm i https://pkg.pr.new/@standardserver/bun@51

@standardserver/core

npm i https://pkg.pr.new/@standardserver/core@51

@standardserver/deno

npm i https://pkg.pr.new/@standardserver/deno@51

@standardserver/fastify

npm i https://pkg.pr.new/@standardserver/fastify@51

@standardserver/fetch

npm i https://pkg.pr.new/@standardserver/fetch@51

@standardserver/node

npm i https://pkg.pr.new/@standardserver/node@51

@standardserver/peer

npm i https://pkg.pr.new/@standardserver/peer@51

@standardserver/shared

npm i https://pkg.pr.new/@standardserver/shared@51

commit: 658f629

@codecov

codecov Bot commented Aug 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed-hq

codspeed-hq Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 26 untouched benchmarks
⏩ 108 skipped benchmarks1


Comparing claude/response-send-throw-leak-56af52 (658f629) with main (d32b063)

Open in CodSpeed

Footnotes

  1. 108 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@dinwwwh
dinwwwh merged commit 4ef7d9f into main Aug 3, 2026
10 checks passed
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.

1 participant