node:http2: give pending requests cancelled by session.destroy() node's rstCode

[robobun]

Problem

• A node:http2 client request that is still queued (no stream id yet) when session.destroy() is called with no code reports rstCode 8 (NGHTTP2_CANCEL); node reports 2 (NGHTTP2_INTERNAL_ERROR). The error, ERR_HTTP2_STREAM_CANCEL, already matches.
• The same path ignores the code of a GOAWAY the session received and overwrites the code of a request that was already close(code)d, so both report the destroy code where node reports the GOAWAY or close code.
• Cause: on session destroy, bun assigns every queued request a code unconditionally (the destroy code, else NGHTTP2_CANCEL) instead of letting the stream's own teardown pick one.

Fix

• A queued request now only gets a code pre-set when the session has one (a received GOAWAY's code, else the destroy code) and the request has none yet; otherwise it is left unset.
• Left unset, the stream's own _destroy derives NGHTTP2_INTERNAL_ERROR from the cancel error, as node does and as bun's destroy(0) and destroy(err) already did. Active streams already follow this rule on session destroy.
• The reported error is unchanged: every shape still gets ERR_HTTP2_STREAM_CANCEL with the session error as its cause.
• Verification: four new tests cover both queue reasons across seven destroy() shapes plus the close(code) and GOAWAY cases; all four fail on current bun and pass with this change. The existing http2 suites and the vendored node teardown tests still pass on a debug build.

Background

• rstCode is the RST_STREAM error code an Http2Stream ended with. Codes seen here: 2 INTERNAL_ERROR, 7 REFUSED_STREAM, 8 CANCEL, 11 ENHANCE_YOUR_CALM.
• Bun's client session queues requests that have no stream id yet: those made before the socket connected, and those waiting for a SETTINGS_MAX_CONCURRENT_STREAMS slot (node hands the latter to nghttp2). Both report pending === true and go through the changed path.
• In node, a stream destroyed with an error that is not an AbortError gets NGHTTP2_INTERNAL_ERROR unless it already has a code; NGHTTP2_CANCEL is reserved for AbortSignal cancellations.
• A GOAWAY frame is the peer shutting the connection down with an error code; the session remembers it, and node gives it precedence over a later destroy(code).
• close(code) on a pending request records the code and waits for a stream id before sending RST_STREAM, so the request is still queued when the session goes away.

Original description

Repro

A request that is still pending (no stream id yet) when its client session is destroyed without a code reports rstCode 8 (NGHTTP2_CANCEL); node reports 2 (NGHTTP2_INTERNAL_ERROR). The error object already matches (ERR_HTTP2_STREAM_CANCEL on both).

const http2 = require("node:http2"), net = require("node:net");
const server = net.createServer(() => {});
server.listen(0, "127.0.0.1", () => {
  const client = http2.connect(`http://127.0.0.1:${server.address().port}`);
  client.on("error", () => {});
  const req = client.request({ ":path": "/" }); // still connecting, so the request is queued
  let error;
  req.on("error", e => (error = e.code));
  req.on("close", () => { console.log(req.rstCode, error); server.close(); });
  client.destroy();
});

bun (main):   8 ERR_HTTP2_STREAM_CANCEL
node v26.3.0: 2 ERR_HTTP2_STREAM_CANCEL

Full table for a pending request, bun main vs node v26.3.0 (the same script with the destroy() arguments varied; the close()d and GOAWAY rows are separate scripts):

session teardown	bun main	node	this PR
destroy()	8	2	2
destroy(null)	8	2	2
destroy(0)	2	2	2
destroy(7) / destroy(null, 7) / destroy(err, 7)	7	7	7
destroy(err)	2	2	2
req.close(5) first, then destroy() or destroy(7)	8 / 7	5	5
GOAWAY(11) received, 'goaway' listener calls destroy() or destroy(7)	8 / 7	11	11

Cause

ClientHttp2Session#destroy() (src/js/node/http2.ts) cancels the requests still queued in #pendingRequests with

req.rstCode = code !== undefined ? code : constants.NGHTTP2_CANCEL;
req.destroy(cancelError);

Node's closeSession destroys its pending streams with new ERR_HTTP2_STREAM_CANCEL(error) and lets Http2Stream._destroy pick the code: the session's code (goawayCode || destroyCode) when non-zero, otherwise NGHTTP2_INTERNAL_ERROR because the stream is being destroyed with an error that is not an AbortError (NGHTTP2_CANCEL is reserved for AbortSignal cancellations). It also never overwrites the code of a stream that was already close()d. Our unconditional assignment hard-coded CANCEL for the no-code case, ignored a received GOAWAY's code, and clobbered a close(code).

Fix

Only pre-set rstCode when there is a session code (this[kGoawayCode] || code, the expression the active-stream teardown already uses) and the stream has none yet; otherwise leave it unset so Http2Stream#_destroy derives NGHTTP2_INTERNAL_ERROR from the cancel error, which is the same derivation node performs (and the one destroy(0) and destroy(err) were already going through). This is the same if (rstCode && !stream.rstCode) rule as destroyStreamForSessionDestroy.

The reported error is unchanged: every shape still gets ERR_HTTP2_STREAM_CANCEL with the session error as its cause. Bun also queues requests that are waiting for a SETTINGS_MAX_CONCURRENT_STREAMS slot in #pendingRequests (node hands those to nghttp2, where they already have an id); they report pending === true and get the cancel error here today, so they follow the same rule. The rstCode they get matches what node reports for those streams as well (the table's GOAWAY row was measured that way).

Related open PRs touch neighbouring paths but not this one: #33802 changes how active streams are torn down by the client destroy(), and #37698 makes destroy(undefined, code) behave like destroy(); this change composes with both (once #37698 lands, destroy(undefined, 7) also yields 2 here, as in node).

Verification

New describe in test/js/node/http2/node-http2.test.js covers both queue reasons (request before connect, request behind maxConcurrentStreams) for seven destroy() shapes, plus the close(code)-then-destroy and GOAWAY-precedence cases. All four tests fail on the current release (rstCode 8/7/11 where node gives 2/5/11 as in the table) and pass with this change. node-http2.test.js (360 pass), h2-conformance.test.ts (61 pass) and the vendored node tests that exercise pending streams and session teardown (test-http2-client-destroy, test-http2-client-stream-destroy-before-connect, test-http2-client-rststream-before-connect, test-http2-stream-removelisteners-after-close, test-http2-propagate-session-destroy-code, test-http2-max-concurrent-streams, test-http2-goaway-delayed-request, ...) still pass with a debug build.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 11 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9d6276d6-fb44-429c-aec7-9267b3c98bea

📥 Commits

Reviewing files that changed from the base of the PR and between f59f705 and 1d40bae.

📒 Files selected for processing (2)

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

---

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

[robobun]

^{Updated 3:05 AM PT - Aug 12th, 2026}

❌ @robobun, your commit 1d40bae has some failures in Build #92834 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 37744

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

bun-37744 --bun

[robobun]

Status

Reproduced on the current release and on main: a request queued before the session connected (or behind maxConcurrentStreams) reports rstCode 8 after a bare session.destroy(), node v26.3.0 reports 2; a close(5)'d pending request reported the destroy code instead of 5, and a received GOAWAY's code was ignored (node uses it). Verified each row of the table in the description against node v26.3.0 directly.

Fix is in ClientHttp2Session#destroy() only (the pending-request cancel loop); active-stream teardown is untouched (that is #33802's area). Tests: the new describe in test/js/node/http2/node-http2.test.js fails 4/4 on the release binary and passes with this change; the rest of node-http2.test.js, h2-conformance.test.ts and the pending-stream / teardown vendored node tests still pass with a debug build.

CI (build 92834): every lane that ran is green (192/192, the new tests included; the handful of retried tests are known flaky ones unrelated to http2 teardown, plus the pre-existing 10k-request maxSessionMemory stress test timing out once on darwin 14 before passing). The build is marked failed only because the two darwin 26 aarch64 - test-bun jobs never got an agent and expired after three re-queues; that lane is currently sitting unstarted on every PR build, so re-running would not change the outcome. Ready for review as is.

[claude]

LGTM — small Node-compat fix that mirrors the existing destroyStreamForSessionDestroy rule for pending requests.

What was reviewed:

• Traced all seven destroy() shapes plus close-first and GOAWAY-first through the new sessionCode = this[kGoawayCode] || code / if (sessionCode && !req.rstCode) logic and the _destroy fallback at http2.ts:2793-2805 — each yields the node-matching rstCode from the PR table.
• Confirmed the guard is byte-identical to destroyStreamForSessionDestroy (http2.ts:2454) and the active-stream teardown's this[kGoawayCode] || code expression (http2.ts:4967).
• Tests: hermetic (local http2.createServer, port 0), await the close event via a resolver, clean up in try/finally, and cover both queue reasons plus the two precedence cases.

Extended reasoning...

Overview

Two-line behavioral change in ClientHttp2Session#destroy() (src/js/node/http2.ts): pending (never-sent) requests torn down by session destroy now get rstCode = this[kGoawayCode] || code only when that value is truthy and the stream hasn't already recorded one, instead of unconditionally code ?? NGHTTP2_CANCEL. When no session code applies, rstCode is left unset so Http2Stream#_destroy (lines 2793-2805) derives NGHTTP2_INTERNAL_ERROR from the non-abort cancel error — exactly what node does. The comment is updated to explain the derivation. A new 4-test describe.concurrent block in node-http2.test.js exercises seven destroy() argument shapes across two pending-queue reasons, plus close(code)-then-destroy and GOAWAY-precedence.

Security risks

None. This adjusts which numeric constant is reported on stream.rstCode for already-cancelled client requests. No parsing, no auth, no resource limits, no new external input handling.

Level of scrutiny

Low-to-medium. The change is a Node-compat correctness fix in a JS builtin, and the new guard is the same if (rstCode && !stream.rstCode) pattern already used by destroyStreamForSessionDestroy (line 2454) and the same this[kGoawayCode] || code precedence used for active streams (line 4967). The error object emitted (ERR_HTTP2_STREAM_CANCEL with cause) is unchanged. I traced every row of the PR's behavior table through the code path (including the typeof error === 'number' and code === undefined && error != null normalization at lines 5995-6001) and each produces the stated node-matching value.

Other factors

Tests are well-structured per the repo guidelines: local server with port: 0, resolve on the close event (no sleeps), try/finally cleanup, describe.concurrent for independent cases, and an expect(active.pending).toBe(false) precondition assert that proves the concurrency-queued case is actually queued. The PR description documents that all four new tests fail on the current release and that the surrounding suites (node-http2.test.js, h2-conformance.test.ts, and the vendored node http2 tests) still pass. The bug-hunting system found nothing.