node:http: defer ServerResponse.destroy() 'close' and stop rewriting completed requests as aborted by robobun · Pull Request #33818 · oven-sh/bun

robobun · 2026-07-09T06:51:25Z

What

res.destroy() on a node:http server response tore the exchange down synchronously and re-entrantly, breaking two independent Node.js contracts in one call:

res 'close' was dispatched from inside destroy(), so res.closed was already true when destroy() returned. The streams contract says closed becomes true only after 'close' has been emitted on a later tick.
An already fully-received request was rewritten as a client abort: 'aborted' fired, 'end' was dropped, req.complete flipped to false. on-finished-style middleware and request accounting misclassify a completed request as client-aborted.

Repro

import { createServer } from "node:http";
import * as net from "node:net";

const srv = createServer((req, res) => {
  req.on("data", () => {});
  req.on("aborted", () => log.push("req.aborted"));
  req.on("end", () => log.push("req.end"));
  req.on("close", () => log.push(`req.close(aborted=${+req.aborted},complete=${+req.complete})`));
  res.on("close", () => log.push("res.close"));
  log.push("call-destroy");
  res.destroy();
  log.push(`destroy-returned(closed=${+res.closed})`);
});

node v26.3.0:
  call-destroy > destroy-returned(closed=0) > req.end > req.close(aborted=0,complete=1) > res.close
bun (before):
  call-destroy > req.aborted > res.close > destroy-returned(closed=1) > req.close(aborted=1,complete=0)

Cause

ServerResponse.prototype.destroy emitted 'close' synchronously (this._closed = true; this.emit("close")) before returning.
NodeHTTPServerSocket#closeHandle (reached synchronously via socket.destroy() → _destroy) called req.destroy() before the queued push(null)/complete = true nextTick for the body-less request could run, so IncomingMessage#_destroy saw !readableEnded || !complete and emitted 'aborted'.

Fix

Route the teardown through the same path Node.js uses (OutgoingMessage#destroy → socket.destroy → socket close callback):

Drop the synchronous 'close' emission from ServerResponse.prototype.destroy. In the native server path the socket's #onClose (scheduled as a task from the native close) schedules emitCloseNT(res); for a standalone response with no native handle, schedule it via process.nextTick. The socket is read from this[fakeSocketSymbol] directly (the socket getter auto-creates a FakeSocket, which would make the no-handle fallback unreachable).
Drop the synchronous req.destroy() from #closeHandle; #onClose already destroys the request after pending nextTicks have drained, so a fully-received request has readableEnded/complete set by then.
After a synchronous in-handler destroy, keep the socket attached (skip detachSocket at the handle.finished fast path) so #onClose can still find _httpMessage to close the response and destroy the request.

Verification

New test/js/node/http/node-http-server-response-destroy.test.ts covers res.destroy() before any write, after writeHead+partial body, with an error argument, and on a standalone new ServerResponse(new IncomingMessage()). Each variant asserts res.closed === false when destroy() returns and that 'close' is emitted on a later tick; the server variants also assert the exact event order call-destroy > destroy-returned > req.end > req.close > res.close with req.aborted === false and req.complete === true.

All four fail on main and pass with this change; all assertions verified against Node v26.3.0 (identical event order). Swept the ~360 vendored test/js/node/test/parallel/test-http-* scripts and test/js/third_party/express/res.sendFile.test.ts: no new failures versus main.

[review] gate passed · iteration 3 · 2 files touched

fails on main (without fix)

ASAN without fix: 4 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/node/http/node-http-server-response-destroy.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (bb231be76)

test/js/node/http/node-http-server-response-destroy.test.ts:
70 |     client.on("error", () => {});
71 |     client.on("close", () => resolveClientClosed());
72 |     await Promise.all([clientClosed, resClosed, reqClosed]);
73 | 
74 |     // Writable.destroy semantics: 'close' is emitted on a later tick.
75 |     expect(closedAtReturn).toBe(false);
                                ^
error: expect(received).toBe(expected)

Expected: false
Received: true

      at <anonymous> (/workspace/bun/test/js/node/http/node-http-server-response-destroy.test.ts:75:28)
(fail) ServerResponse.destroy() before any write > defers 'close' and leaves a fully-received request complete (not aborted) [568.58ms]
70 |   
... (truncated)

release without fix: all passed
bun test v1.4.0-canary.1 (e1597e76e)

test/js/node/http/node-http-server-response-destroy.test.ts:
(pass) ServerResponse.destroy() before any write > defers 'close' and leaves a fully-received request complete (not aborted) [11.01ms]
(pass) ServerResponse.destroy() after writeHead + partial body > defers 'close' and leaves a fully-received request complete (not aborted) [4.68ms]
(pass) ServerResponse.destroy() with an error argument > defers 'close' and leaves a fully-received request complete (not aborted) [2.75ms]
(pass) standalone ServerResponse.destroy() defers 'close' to a later tick [0.23ms]

 4 pass
 0 fail
 14 expect() calls
Ran 4 tests across 1 file. [162.00ms]
__F:0:S:0

passes on PR (with fix)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/node/http/node-http-server-response-destroy.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (bb231be76)

test/js/node/http/node-http-server-response-destroy.test.ts:
(pass) ServerResponse.destroy() before any write > defers 'close' and leaves a fully-received request complete (not aborted) [565.79ms]
(pass) ServerResponse.destroy() after writeHead + partial body > defers 'close' and leaves a fully-received request complete (not aborted) [139.06ms]
(pass) ServerResponse.destroy() with an error argument > defers 'close' and leaves a fully-received request complete (not aborted) [113.40ms]
(pass) standalone ServerResponse.destroy() defers 'close' to a later tick [12.78ms]

 4 pass
 0 fail
 14 expect() calls
Ran 4 tests across 1 file. [3.92s]
__F:0:S:0

release with fix: all passed
$ bun scripts/build.ts --profile=release
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
[configured] bun-profile → bun (stripped) in 659ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/48] gen cpp.rs (cppbind)
[2/48] gen JS modules (bundle-modules)
Preprocess modules (6182ms)
Bundle modules (25ms)
Postprocesss modules (20ms)
Bundle Functions (631ms)
Generate Code (71ms)

[6.94s] Bundled "src/js" for production
  1888 kb
  161 internal modules
  12 native modules
  90 internal functions across 19 files
[2/37] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu)
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: component rust-std is up to date

  nightly-2026-05-06-x86_64-unknown-linux-gnu unchanged - rustc 1.97.0-nightly (e95e73209 2026-05-05)

info: checking for 
... (truncated)

diff hotspot

src/js/node/_http_server.ts                        | 36 +++++----
 .../http/node-http-server-response-destroy.test.ts | 92 ++++++++++++++++++++++
 2 files changed, 114 insertions(+), 14 deletions(-)

gate history · 2 passed · 1 rejected · iteration 3

evidence per changed file

file                                                      reads  edits  tests
src/js/node/_http_server.ts                                  14      9      0
…/js/node/http/node-http-server-response-destroy.test.ts      2      5      0

…completed requests as aborted ServerResponse.prototype.destroy() emitted 'close' synchronously from inside the call (so res.closed was already true when destroy() returned), and the socket _destroy path synchronously destroyed the request before its queued EOF could flip readableEnded/complete, firing 'aborted' on a fully-received request and dropping its pending 'end'. In Node.js res.destroy() destroys the socket and the socket's close callback (onServerResponseClose/abortIncoming) tears the request and response down on a later tick. Route the teardown through the same path: drop the synchronous 'close' emission from ServerResponse.destroy (the socket's #onClose schedules it, with a nextTick fallback when no socket is attached), drop the synchronous req.destroy() from #closeHandle (the same #onClose runs it after pending nextTicks), and keep the socket attached after a synchronous in-handler destroy so #onClose can still see it. With the fix, for a fully-received request res.destroy() matches Node.js exactly: destroy() returns with res.closed === false, 'end' is delivered, 'aborted' is not, req.complete stays true, and res 'close' follows req 'close' on a later tick.

coderabbitai · 2026-07-09T06:51:32Z

Walkthrough

This PR changes ServerResponse.destroy() and related socket/request teardown logic in Bun's Node.js HTTP server implementation so that 'close' emission is deferred to a later tick rather than immediate, preventing fully-received requests from being incorrectly marked as aborted. A new test file validates this deferred close behavior.

Changes

Deferred ServerResponse close behavior

Layer / File(s)	Summary
Core destroy/close flow changes `src/js/node/_http_server.ts`	Socket detachment in the request-completion path now conditionally clears the close callback based on response closed/destroyed state; `Socket.#closeHandle` no longer synchronously calls `req.destroy()`, deferring to the `#onClose` task; `ServerResponse.destroy` now destroys via `fakeSocket` and schedules `'close'` emission via `process.nextTick(emitCloseNT, this)` instead of emitting immediately.
Deferred close ordering tests `test/js/node/http/node-http-server-response-destroy.test.ts`	New test suite with a parameterized `describe.each` matrix of `destroy()` invocation variants, an async server test asserting `res.closed` remains `false` at return time and that requests are not aborted, and a standalone test confirming `destroyed` is set immediately while `closed` is deferred.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main behavior change in ServerResponse.destroy() and the completed-request fix.
Description check	✅ Passed	The description covers the required purpose and verification details, with clear sections for cause, fix, and tests despite different headings.

_{Comment @coderabbitai help to get the list of available commands.}

robobun · 2026-07-09T06:51:35Z

^{Updated 4:53 AM PT - Jul 9th, 2026}

✅ @robobun, your commit bb231be76fc706e3054afbed542b083561e867ac passed in Build #70950! 🎉

🧪 To try this PR locally:

bunx bun-pr 33818

That installs a local version of the PR into your bun-33818 executable, so you can run:

bun-33818 --bun

github-actions · 2026-07-09T06:58:46Z

Found 3 issues this PR may fix:

node:http ServerResponse doesn't emit close event #14697 - ServerResponse doesn't emit 'close' event; this PR defers 'close' emission through process.nextTick(emitCloseNT) matching Node.js behavior
http.ServerResponse emits neither "finish" nor "close" after aborted request #30571 - ServerResponse emits neither 'finish' nor 'close' after aborted request; this PR fixes the teardown path so 'close' is properly emitted via the socket close callback
Express: req/res/socket 'close' events never fire on client abort for POST requests with a body #28976 - Express req/res/socket 'close' events never fire on client abort for POST requests with a body; this PR stops synchronously destroying IncomingMessage in #closeHandle, letting #onClose handle it after pending nextTicks drain

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #14697
Fixes #30571
Fixes #28976

🤖 Generated with Claude Code

…file The ServerResponse socket getter auto-creates a FakeSocket, so the no-socket fallback was unreachable and a standalone new ServerResponse() never emitted 'close' after destroy(). Read this[fakeSocketSymbol] directly (same pattern as end()/setTimeout()) and schedule emitCloseNT whenever there is no native handle, which covers the FakeSocket case. Also move the tests to test/js/node/http/node-http-server-response-destroy.test.ts so the gate runs them in isolation, and add coverage for the standalone response path.

claude

Thanks for the fix in 7c32ef5 — the standalone-response path now looks right and is covered by a test. I didn't find further issues, but the removal of the synchronous req.destroy() from #closeHandle and the new conditional detachSocket skip touch subtle request/response lifecycle ordering on a hot path, so this is worth a human look.

Extended reasoning...

Overview

This PR changes three coupled sites in src/js/node/_http_server.ts to make ServerResponse.destroy() follow Node's Writable-destroy contract (async 'close') and to stop a server-initiated destroy from retroactively marking a fully-received request as aborted:

ServerResponse.prototype.destroy: drops the synchronous 'close' emit; reads this[fakeSocketSymbol] directly and schedules process.nextTick(emitCloseNT, this) when there is no native handle.
NodeHTTPServerSocket#closeHandle: removes the synchronous req.destroy(); relies on #onClose (reached via the native close callback) to destroy the request after pending nextTicks drain.
onNodeHTTPRequest fast path: skips detachSocket when the response is destroyed-but-not-closed so #onClose can still find _httpMessage.

A new test file exercises destroy before-write / after-partial-write / with-error, plus a standalone new ServerResponse(new IncomingMessage()), asserting exact event order against Node v26.

Security risks

None identified — this is event-ordering / lifecycle bookkeeping in the node:http compat layer; no auth, crypto, parsing, or untrusted-input handling is touched.

Level of scrutiny

Medium-high. _http_server.ts is a hot path underneath every node:http-based framework, and the three changes interact: correctness of the new 'close' scheduling depends on #onClose reliably reaching _httpMessage, which in turn depends on the new conditional detach. Removing the synchronous req.destroy() from #closeHandle shifts request teardown to an async native-close callback; a maintainer familiar with why that sync destroy was originally added (and with keep-alive / pipelining edge cases) should confirm no path is left where the request is never destroyed or where a genuine mid-body client abort now fails to emit 'aborted'.

Other factors

My earlier inline review flagged an unreachable fallback branch; that was fixed in 7c32ef5 (reads fakeSocketSymbol storage directly, gates the fallback on !handle) and a regression test for the standalone case was added. The current bug-hunt pass found nothing further. The author reports sweeping ~360 vendored test-http-* parallel tests and the express res.sendFile suite with no new failures, which is reassuring but doesn't substitute for a maintainer sanity-check on the lifecycle reordering.

events.once() installs its own error listener that rejects the awaited promise; a separate no-op error listener does not suppress it. Use an explicit close resolver so an expected ECONNRESET from the force-closed connection cannot fail the test.

robobun · 2026-07-09T11:06:02Z

CI on this diff is green; the remaining red is unrelated infra across three runs:

#70950 (bb231be): darwin-14-aarch64-test-bun job Expired (no agent picked it up). 284 jobs passed; two flaky warnings (bake/dev-and-prod, node-tls-connect on Windows) passed on retry.
#70911 (7c32ef5): postgres-binary-array-bounds on Windows (ERR_POSTGRES_CONNECTION_REFUSED), 26030.test.ts on alpine (MySQL container healthcheck timeout), update_interactive_install on Windows.
#70886 (142c439): same Windows postgres lane.

The new test file test/js/node/http/node-http-server-response-destroy.test.ts passed on every lane that ran it, and a local diff of all ~360 vendored test/js/node/test/parallel/test-http-* scripts against main shows no new failures. Ready for review.

robobun · 2026-07-24T09:39:53Z

Superseded by #35025 (on top of #32488). #28976 is now closed: req/res/socket 'close' events fire on client abort mid-body on main.

github-actions Bot added the claude label Jul 9, 2026

claude Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread src/js/node/_http_server.ts Outdated

claude Bot reviewed Jul 9, 2026

View reviewed changes

ci: retrigger

e1597e7

claude Bot reviewed Jul 9, 2026

View reviewed changes

Comment thread test/js/node/http/node-http-server-response-destroy.test.ts Outdated

robobun closed this Jul 24, 2026

Navigation Menu

Uh oh!

node:http: defer ServerResponse.destroy() 'close' and stop rewriting completed requests as aborted - #33818

node:http: defer ServerResponse.destroy() 'close' and stop rewriting completed requests as aborted#33818
robobun wants to merge 4 commits into
mainfrom
farm/3167c192/http-server-response-destroy-async-close

robobun commented Jul 9, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

Uh oh!

robobun commented Jul 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

robobun commented Jul 9, 2026

Uh oh!

robobun commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

robobun commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Repro

Cause

Fix

Verification

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

robobun commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 9, 2026

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Uh oh!

robobun commented Jul 9, 2026

Uh oh!

robobun commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented Jul 9, 2026 •

edited

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

robobun commented Jul 9, 2026 •

edited

Loading