node:http2: let close() and destroy() win over the refusal of a request

[robobun]

Problem

• req.close() in the same turn as a request() that Bun refuses ends the process: error: Invalid stream id at rstNextTick (node:http2). Bun 1.3.14 does not crash. 1.4.0 to 1.4.3 do (node:http/https/http2: raise Node v26.3.0 compat to ~94%, sync the upstream suites, and fix the Windows/macOS transport-layer teardown bugs they exposed #32488).
• request() refuses a :path with a space, and content-length on a DELETE. The stream gets no id and still emits 'ready'. close() waits for 'ready', then sendRstOnReady (src/js/node/http2.ts:2249) calls rstStream(undefined, code).
• The refusal also replaces the code of close() or destroy() and emits 'error'. With no 'error' listener, that ends the process too.

Fix

• sendRstOnReady returns when the stream has no id. node has the same check in submitRstStream (core.js:497).
• The refusal runs one tick after request(). A stream that user code closed or destroyed before that keeps its code and gets no 'error', as in node's onStreamClose.
• Verified: test/js/node/http2/node-http2-client-close.test.ts (8 new tests, main fails all, node v26.3.0 passes all). Also test/js/node/http2/ and five vendored test-http2-*.js.

Background

• A client stream has no id until its HEADERS frame is sent. close() before that sends RST_STREAM after 'ready'.
• axios reaches the crash: axios.delete(url, { httpVersion: 2, signal, data }), then abort() in the same tick. No user reported it. node:http2 client: close() on a request that was rejected before it got a stream id throws an uncaught "Invalid stream id" #44479 describes it.
• No other design was weighed: each check is missing in the function that acts on the stream. node:http2: fail a dropped write with ECANCELED, keep no write callback in native #44881 has the same id check.

Downsides

• After close() or destroy() in the same tick, a refused request emits no 'error' for the refusal, only 'close'.
• For a refused :path, req.rstCode is 1 one tick after request(), not at once.

Notes

Repro (bun repro.cjs, exit code 1 on Bun 1.4.3 and on main, 0 on Bun 1.3.14 and on node v26.3.0):

const http2 = require("node:http2");
const server = http2.createServer((req, res) => res.end("ok"));
server.listen(0, "127.0.0.1", () => {
  const client = http2.connect("http://127.0.0.1:" + server.address().port);
  client.on("connect", () => {
    const req = client.request({ ":path": "/a b" });
    req.on("error", () => {});
    req.on("close", () => {
      client.close();
      server.close();
    });
    req.close();
  });
});

error: Invalid stream id
      at rstNextTick (node:http2:1510:34)

What the stream reports. error is the 'error' event, the number is rstCode at 'close'. The two refusals give the same rows.

call in the tick of request()	'error' listener	node v26.3.0	Bun 1.4.3	this PR
close()	yes	0	error, 1, then the uncaught Invalid stream id	0
close()	no	0	uncaught Invalid stream id, or an uncaught ERR_HTTP2_STREAM_ERROR (content-length)	0
destroy()	yes	0	error, 1 (:path). 1 (content-length)	0
close(2)	yes	error, 2	error, 1, then uncaught	error, 2
none	yes	error, 1	error, 1	error, 1
none	no	uncaught ERR_HTTP2_STREAM_ERROR	the same	the same

Which requests. Only two paths of request() emit 'ready' for a stream with no id: the :path check (a character <= 0x20 or 0x7f) and the content-length check for GET, HEAD and DELETE. The other returns with no id (a destroyed session, an aborted signal, no stream id left) emit no 'ready'. Bun 1.3.14 did not refuse these requests. It sent them.

One function for both refusals. rejectRequestNT(session, req) replaces rejectNoPayloadContentLengthNT and the direct emitStreamErrorNT call of the :path check. The :path check set rstCode inside request(). The function sets it one tick later, so that a destroy() in between sees no code.

axios. Checked with axios 1.20.0 and httpVersion: 2 against a local server: axios.delete(url, { signal, data }) and axios.get(url, { signal, data }) with abort() in the same tick, and a paramsSerializer that leaves a space in the query. Each ends the process on Bun 1.4.3. With this change each rejects with ERR_CANCELED, as on node.

The test is not a spawned fixture. An uncaught exception fails the test in the same process, and the file also runs under node --test, so node checks the expected events. A child that loads node:http2 needs 4.6 s to 5.0 s on a debug build, against the 5 s limit of a test.

Suites. Debug build: test/js/node/http2/ (624 pass, 0 fail, with a 60 s limit: on this debug build the tests that start a second bun need more than the default 5 s). Vendored: test-http2-client-unescaped-path.js, test-http2-multi-content-length.js, test-http2-client-destroy.js, test-http2-head-request.js, test-http2-client-request-options-errors.js. node --test on the test file: node v26.3.0 and v26.11.1 pass all 76.

Left alone.

• A refused request still emits 'aborted' and 'ready' and no 'finish' (node: 'finish', no 'aborted', and 'ready' inside request()).
• A refused request does not check options.signal: a value that is not a signal does not throw, and an aborted signal gives ERR_HTTP2_STREAM_ERROR, not an AbortError. main does the same.
• A peer RST_STREAM or GOAWAY after close() still replaces the code of the stream. That is the same node rule at other call sites.

---

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

[robobun]

Status

• Reproduced with the 8 new tests in test/js/node/http2/node-http2-client-close.test.ts ("a request that is refused after close() or destroy()"): main fails all 8, this branch and node v26.3.0 pass them.
• Also reproduced with axios 1.20.0 (httpVersion: 2): axios.delete(url, { signal, data }), then abort() in the same tick. Bun 1.4.3 exits with code 1 (error: Invalid stream id at rstNextTick). node v26.3.0 and this branch reject with ERR_CANCELED.
• PR: node:http2: let close() and destroy() win over the refusal of a request #45076

[robobun]

^{Updated 11:10 AM PT - Oct 12th, 2026}

✅ @robobun, your commit 72a2d3979e3898ff5015803403703754d2472580 passed in Build #124885! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 45076

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

bun-45076 --bun

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Essentials
• Run ID: 460551b5-45a5-4e78-a1cc-3f77294d505d

---

📥 Commits

Reviewing files that changed from the base of the PR and between 5d6bbe6 and 72a2d39.

---

📒 Files selected for processing (1)

• src/js/node/http2.ts

---

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

---

---

Walkthrough

HTTP/2 client request rejection handling now accounts for requests that are already closed and avoids scheduling RST_STREAM when a stream ID is not numeric. Regression tests cover invalid-path and no-payload content-length rejections.

Changes

HTTP/2 refused request close handling

Layer / File(s)	Summary
Handle closed refused requests src/js/node/http2.ts, test/js/node/http2/node-http2-client-close.test.ts	A shared rejection handler destroys already-closed requests without an error. Invalid-path and no-payload content-length rejections use this handler. Reset scheduling skips non-numeric stream IDs. Tests cover immediate close() and destroy() for both rejection cases, with and without an error listener.

Suggested reviewers: jarred-sumner

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 72a2d

The rejected-request handling avoids emitting a protocol error after immediate cancellation. No merge-blocking concern was identified.

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly summarizes the main change: user calls to close() and destroy() take precedence when a request is refused.
Description check		The description explains the problem, fix, affected behavior, and verification. It does not use the template headings, but it provides the required information in equivalent sections.

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[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 — Callers that pass options.signal to a request Bun refuses get neither validation nor abort wiring, unlike every other request() path. src/js/node/http2.ts:6209 and http2.ts:6224 return before validateAbortSignal at http2.ts:6239 and setupRequestEndAndSignal at http2.ts:6283/6315. A non-signal value does not throw ERR_INVALID_ARG_TYPE and endStream is never applied, so an aborted signal reports a stream error instead of an AbortError. Fix: run the signal validation and the already-aborted check before the two refusal returns so refused requests honor the same signal contract as accepted ones.

  Why this was flagged

  Trigger: client.request({':path': '/a b'}, { signal }) or the content-length refusal with any options.signal. Both early returns at http2.ts:6209 and http2.ts:6224 sit above the signal block at http2.ts:6232-6248. A caller passing signal: 123 gets no synchronous ERR_INVALID_ARG_TYPE; a caller passing an already-aborted signal gets ERR_HTTP2_STREAM_ERROR from http2.ts:3752 rather than the AbortError the path at http2.ts:6245 produces for every other request. The PR description names axios with signal plus abort() as the motivating population, so this is exactly the input mix that reaches these lines. The base branch has the same ordering, but the base crashed the process on this path, so the inconsistency is newly observable. Remedy: hoist validateAbortSignal and the signal.aborted fast path above both refusal returns.

  Verification: pre-existing — the mechanism is real but lives in request() code this diff does not modify (the base file at 772f87f has the identical ordering). Trigger: client.request(headers, { signal }) where headers hit either refusal path. In src/js/node/http2.ts the :path refusal returns at line 6209 and the content-length refusal at 6224, both above validateAbortSignal at 6239 and both setupRequestEndAndSignal calls at 6283 and 6315.

[claude]

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

LGTM — the second commit addresses everything from the earlier review (test moved into the module file, and a stream the user already closed or destroyed now keeps its rstCode and gets no 'error' on both refusal paths), and the remaining inline comment is a small duplication nit.

Extended reasoning...

The diff touches src/js/node/http2.ts (a typeof-id guard in sendRstOnReady plus a req.closed early-destroy guard in the two up-front refusal helpers, with the invalid-:path refusal moved into a new rejectInvalidPathNT) and adds a 2x2x2 matrix to test/js/node/http2/node-http2-client-close.test.ts; no security-sensitive surface is involved. I traced both the close() path (markStreamClosed is synchronous, the nextTick helper then calls destroy() with rstCode 0 and no id, so no RST and exactly one 'close') and the destroy() path (_destroy runs synchronously, the helper's second destroy() is a no-op), and the closed predicate matches node's onStreamClose rule. The change is small and self-contained, the test file also runs under node as an oracle, no CODEOWNERS entry covers these paths, and the only open inline finding is a nit.

[claude]

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

The single rejectRequestNT helper and the move of the matrix into the module's test file address both of my earlier nits. Beyond the inline finding, I also checked that the req.closed guard is live for both calls: close() sets the Closed bit at src/js/node/http2.ts:2591 and destroy() sets it synchronously via _destroy at http2.ts:2655, so the next-tick guard sees it either way; and that routing the content-length refusal through emitStreamErrorNT still yields ERR_HTTP2_STREAM_ERROR with no 'end' (the readable is errored inside the synchronous _destroy before endReadableNT runs), which is what the ported test-http2-multi-content-length.js asserts.

Extended reasoning...

The diff touches two refusal sites in ClientHttp2Session.request() and the deferred RST sender in src/js/node/http2.ts, plus a new 2x2x2 test matrix; no security-sensitive surface. An inline pre-existing finding is already posted, so this body only records what else was examined and ruled out. I could not run the file under node locally to confirm the close:0/no-error contract on real node, so the existing node cross-check at the bottom of the test file is what settles that in CI.

[robobun]

Follow-up that is not in this PR

This PR fixes only the refused request. The defect that led to it is still on main: a bare close() on a request whose body user code did not end sends no RST_STREAM while the body waits for WINDOW_UPDATE. When the peer opens the window, Bun uploads the rest of the body first (983,041 bytes of 1 MiB in my run). node v26.3.0 resets at once. Issue #43588 (not planned) describes it.

A fix is on the branch robobun/1fa91a25/http2-close-rst-at-once. I did not open it as a PR:

• It is not correct alone. For sequences such as close(); destroy() in a listener it sends frames that main and node do not send. A clean version needs two rules under it that other open PRs hold: node:http2: store the code of close(code) before its listeners run #44848 (store the code before the listeners run) and node:http2: free a request's slot when its stream closes on the wire, abort request({ signal }) from JS only #42763 (free the request slot when RST_STREAM is written).
• It needs a decision. With node's rule, close() or destroy() in a 'response' listener resets a response that is still in flight, so one RST_STREAM leaves for each such request. main lets the response complete. A server that limits resets then ends the session. 3,000 sequential GETs with destroy() in 'response' against a node v26.3.0 server with default options: Bun 1.4.3 completes 3,000, node as the client gets GOAWAY after 1,001, the branch after 1,216.
• It edits the same lines as node:http2: refuse a PUSH_PROMISE on a request stream that user code already closed #43465, node:http2: emit no header events on a client stream that user code closed #43525, node:http2: do not reset a stream the native layer already closed #43539, node:http2: send RST_STREAM when a server stream is reset #33380, node:http2: store the code of close(code) before its listeners run #44848, node:http2: free a request's slot when its stream closes on the wire, abort request({ signal }) from JS only #42763, node:http2: fail a dropped write with ECANCELED, keep no write callback in native #44881, node:http2: do not hold a stream's DATA behind another stream's queued frames #43422, node:http2: never send or accept DATA before the response HEADERS #43558 and node:http2: emit 'aborted' only while the writable side is open #45056.

What the branch holds, for whoever merges that group into one change:

• test/js/node/http2/node-http2-client-close.test.ts: 146 more tests against a hand-written HTTP/2 peer. node v26.3.0 passes them under node --test (one is for Bun only).
• test/js/node/http2/close-matrix/: two probes that generate close() / destroy() cells (1,020 and 7,930) and print the wire, the events and the final state of each cell. tri.py compares node, main and a build. On the 1,020 cells the branch has 0 cells where main equals node and the branch does not. On the wider set it has such cells.