node:http2: emit no header events on a client stream that user code closed

[robobun]

Problem

• A node:http2 client stream that user code closed or destroyed still emits 'response', 'push', 'trailers', 'headers' and 'continue'. This happens when the header block arrives in the same socket read as the frame whose handler called close() or destroy(). node v26.3.0 emits nothing.
• The cause is the guard of ClientHttp2Session.#Handlers.streamHeaders (src/js/node/http2.ts:5106). It checks only stream.rstCode, which is 0 after a plain close() or destroy(). Both calls send their RST_STREAM from a later setImmediate, so the native parser still dispatches the rest of the read for that stream.

Fix

• The handler also returns when stream.closed is true.
• The server handler already has this check, and pushToStream already drops DATA for a closed stream. In node, nghttp2 ignores the frames of a stream once its RST_STREAM is submitted.
• Verified: test/js/node/http2/node-http2-client-close.test.ts. The 5 new tests fail without the fix and pass under node too. Also test/js/node/http2/, the 261 node test-http2-* tests, grpc-js.
• Self-reviewed: 11 concerns raised, 5 addressed. The other 6 are defects that predate this change (see Notes).

Background

• streamHeaders is the JS callback that the native HTTP/2 frame parser calls for each complete header block. It picks the event: 'headers' for a 1xx status, 'response', 'push' on a pushed stream, or 'trailers'.
• stream.closed reads the StreamState.Closed bit. close(), _destroy() and the native "stream is fully closed" callback set it.
• The parser handles all frames of one socket read in one loop. No setImmediate callback runs inside that loop.

Notes

Repro. A raw TCP HTTP/2 server answers the request with several frames in one socket.write(). The client acts in the first event handler. Events recorded on the stream, node v26.3.0 against bun 1.4.3-canary.1+367d939d9 and main at 26e7a4b:

frames in one write	handler	node	bun before
HEADERS 103, HEADERS 100, HEADERS 200, DATA	'headers': destroy()	headers 103, close	+ continue, headers 100, response 200
same	'headers': close()	headers 103, close	+ continue, headers 100, response 200
HEADERS 200, DATA, trailers	'response': destroy()	response 200, close	+ trailers
same	'response': close()	response 200, close	+ trailers
PUSH_PROMISE, HEADERS 200 and DATA on the pushed stream	session 'stream': pushed.destroy()	close	+ push 200

With the fix every row matches node. destroy(err) and close(code) with a code other than 0 already matched, because rstCode is not 0 there. For a pushed stream destroyed with code 0 the late 'push' did not need the same read: Bun sends no RST_STREAM for it, so a pushed response that arrives much later also emitted 'push'.

Differences from node that stay.

• close(NGHTTP2_NO_ERROR) while the request body is still blocked on flow control. node submits the RST_STREAM only on 'finish', so a response that arrives before 'finish' still emits 'response' and 'data' on node. Bun ends the readable inside close() and drops that DATA. Before this change Bun emitted a 'response' with no body here. Now it emits nothing.
• close(NGHTTP2_CANCEL) inside a frame handler. node defers that RST_STREAM until the read is done and still emits 'response' and 'data'. Bun emits nothing, before and after this change.
• A session over a JS stream (createConnection returns a plain Duplex). node queues each event with process.nextTick and parses the whole chunk before user code runs, so close() in 'headers' comes after 'response' and 'trailers' are already queued, and node emits them. Bun now emits nothing after close() on every transport. The parity this PR claims is with node over net and TLS sockets.

Same family, not in this PR. These handlers also act on a stream that user code closed in the same read. They predate this change. Their fix is not an early return, so each needs its own change and tests:

• wantTrailers (http2.ts:5246). After destroy() in 'response', a WINDOW_UPDATE in the same read releases the rest of a waitForTrailers body, and Bun emits 'wantTrailers' on the destroyed stream. The usual listener calls sendTrailers(), which throws ERR_HTTP2_INVALID_STREAM (uncaught). node returns early for a closed or destroyed stream. A stream that was only close()d still needs noTrailers() to finish its writable side, so a plain return can hang it.
• streamError and aborted (http2.ts:5022-5038). A peer RST_STREAM in the same read as the user's close() overwrites rstCode (node keeps 0) and, for an error code, emits ERR_HTTP2_STREAM_ERROR on a stream that closed cleanly. These handlers hold the only #connections-- for the stream, so they cannot return early.
• altsvc (http2.ts:5304). The session 'altsvc' event still fires for a stream that user code closed or destroyed.

Covered elsewhere: #42369 fixes late HEADERS on a stream that the client already reset (they leak #connections, and session.close() never completes). #43465 refuses a PUSH_PROMISE on a request that user code closed. pushed.close() on a client emits 'error' ("Invalid stream id") and never 'close', so the new push test covers destroy() only.

Tests. node-http2-client-close.test.ts holds the close(code) event contract and cross-checks itself under node. The new tests moved the frame helpers of its raw server to module scope and gave the server an optional scripted reply. The reply ends with a PING. The test waits for the PING ACK, so the client has dispatched every frame of the write before the assertion. A session 'error' or early 'close' rejects both awaited promises.

Server side: ServerHttp2Session.#Handlers.streamHeaders already returns for a closed stream, and 'trailers' after destroy() or close() in the 'stream' handler already match node.

Not related to this diff, seen locally on a debug build with and without the fix:

• h2-conformance.test.ts, "stream release after a queued END_STREAM" fails in some runs (test: deflake the h2 stream-release cases on debug builds #42357 is open for it).
• grpc-js/test-client.test.ts (100 ms connect deadline) and grpc-js/test-outlier-detection.test.ts (5 s timeout) fail. They pass on a release build.
• grpc-js/test-resolver.test.ts and test-tonic.test.ts need the network.

---

no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/http2/node-http2-client-close.test.ts

[coderabbitai]

Warning

Review paused — included plan limit reached

Keep your review moving with free on-demand reviews.

• Run this review for free

On-demand reviews are free for one more day.

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Promotion and pricing details

On-demand reviews are free for one more day. After that, they cost $0.25 per reviewed file.

Review limit details

Or wait 1 minute for your next included review.

Check out review usage here.

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: oven-sh/bun/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 7bbc8480-b30b-44cd-9ba9-3a02de4d56f7

📥 Commits

Reviewing files that changed from the base of the PR and between 26e7a4b and 330f7b4.

📒 Files selected for processing (2)

• src/js/node/http2.ts
• test/js/node/http2/node-http2-client-close.test.ts

---

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

[robobun]

^{Updated 1:50 PM PT - Sep 19th, 2026}

✅ @robobun, your commit 330f7b4ecfa3ea5c9c70a20c14391e2d6b69957e passed in Build #118443! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 43525

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

bun-43525 --bun

[robobun]

Status: the fix is pushed in this PR (#43525).

How I reproduced it: a raw TCP HTTP/2 server answers the request on stream 1 with several frames in one socket.write(), so one read delivers all of them. The client calls req.destroy() (or req.close()) in the first event handler and records the later events.

• HEADERS 103, HEADERS 100, HEADERS 200, DATA, with destroy() in 'headers': node v26.3.0 records headers 103, close. Bun 1.4.3-canary.1+367d939d9 and main at 26e7a4b also record continue, headers 100, response 200.
• HEADERS 200, DATA, trailers, with destroy() in 'response': Bun also records trailers.
• PUSH_PROMISE plus the pushed response, with pushed.destroy() in the session 'stream' handler: Bun also records push 200.

The same cases are in test/js/node/http2/node-http2-client-close.test.ts. They fail on main, pass with this change, and pass under node.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

• 🟣 src/js/node/http2.ts — A client that destroys a waitForTrailers request inside 'response' can crash with an uncaught ERR_HTTP2_INVALID_STREAM: a WINDOW_UPDATE in the same read still dispatches wantTrailers at src/js/node/http2.ts:5249-5259 on the destroyed stream, its listener calls sendTrailers(), and sendTrailers throws at http2.ts:2368-2370 because the stream is destroyed. Node's onStreamTrailers returns early when the stream is destroyed or closed. Fix: in the wantTrailers handler, return when stream.destroyed, and for a close()d-but-not-destroyed stream call native noTrailers(stream.id) instead of emitting, so the writable side still finishes without invoking user code.

  Extended reasoning...

  Trigger: client.request(headers, { waitForTrailers: true }), writes a body larger than the peer's window, and destroys the stream in its 'response' handler; the server's HEADERS 200 and a WINDOW_UPDATE share one socket read (the PR's own repro harness produces exactly this shape).
  The WINDOW_UPDATE releases the queued END_STREAM and native dispatches onWantTrailers (h2_frame_parser.rs:1649) for the stream.
  wantTrailers at 5249-5259 only checks the WantTrailer bit; stream.destroyed and stream.closed are ignored. It emits 'wantTrailers'.
  The idiomatic listener calls stream.sendTrailers({...}); sendTrailers at 2368 throws ERR_HTTP2_INVALID_STREAM because this.destroyed is true. The throw escapes the native dispatch as an uncaught exception.
  Node (lib/internal/http2/core.js onStreamTrailers) returns before emitting when destroyed or closed, so the same user code is safe there.
  The dismissal called this pre-existing and out of scope, but this PR introduces the contract 'a closed stream emits nothing' and tests it for four events while leaving the one that ends in a crash. Population:…

  Verification: pre-existing — acknowledged in diff: the PR description's Notes ("Same family, not in this PR ... wantTrailers (http2.ts:5246). After destroy() in 'response', a WINDOW_UPDATE in the same read releases the rest of a waitForTrailers body, and Bun emits 'wantTrailers' on the destroyed stream. The usual listener calls sendTrailers(), which throws ERR_HTTP2_INVALID_STREAM (uncaught)") states…

[robobun]

On the wantTrailers finding: confirmed, and it predates this change.

I reproduced it on 1.4.3-canary.1+367d939d9 and on main. The server advertises a 10 byte stream window, the client POSTs 100 bytes with waitForTrailers: true, and the server answers the first DATA frame with HEADERS 200 and a WINDOW_UPDATE in one write. With req.destroy() in 'response':

• node v26.3.0: response, close
• Bun: response, close, wantTrailers (destroyed=true), then an uncaught ERR_HTTP2_INVALID_STREAM from sendTrailers()

The stack shows the emit in the client wantTrailers handler, called from the native side. With req.close() in place of req.destroy() the same repro emits no 'wantTrailers' on Bun, so only the destroyed case is live. The likely reason: _final checks the Closed bit and calls noTrailers() (http2.ts:2706), and a destroyed writable never calls _final.

I left it out of this PR. The fix belongs in both wantTrailers handlers (client and server), and it needs a flow-control test on each side, which this PR's harness does not have. The repro is in the PR notes under "Same family, not in this PR".