[robobun]

Repro

import http2 from "node:http2";
import net from "node:net";
const srv = http2.createServer();
srv.on("stream", s => { s.respond({ ":status": 200 }); s.end("ok"); });
srv.listen(0, "127.0.0.1", () => {
  const ses = http2.connect(`http://127.0.0.1:${srv.address().port}`);
  ses.on("connect", () => {
    void ses.socket;   // cache the proxy
    ses.destroy();
    setTimeout(() => {
      console.log(typeof ses.socket);                 // node: undefined   bun: object
      console.log(ses.socket instanceof net.Socket);  // node: false       bun: throws ERR_HTTP2_SOCKET_UNBOUND
      process.exit(0);
    }, 100);
  });
});

Node documents that session.socket is undefined once the session is destroyed. Bun kept returning the cached Proxy wrapper, whose traps (including getPrototypeOf, which instanceof calls) throw ERR_HTTP2_SOCKET_UNBOUND once the underlying socket is detached. The standard post-teardown guard if (session.socket && !session.socket.destroyed) therefore either saw a live-looking object or threw from inside cleanup code.

Cause

Both ServerHttp2Session and ClientHttp2Session implement the getter as:

get socket() {
  if (this.#socket_proxy) return this.#socket_proxy;
  const socket = this[bunHTTP2Socket];
  if (!socket) return null;
  ...
}

destroy() nulls bunHTTP2Socket but never touches #socket_proxy, so once the proxy was cached the first branch won forever. (And the uncached path returned null, not undefined.)

Fix

Check bunHTTP2Socket first and return undefined when it is gone, in both session classes. A proxy reference captured before destroy still throws ERR_HTTP2_SOCKET_UNBOUND on access, matching Node (test/js/node/test/parallel/test-http2-unbound-socket-proxy.js, which still passes).

Verification

New test in test/js/node/http2/node-http2.test.js:

USE_SYSTEM_BUN=1 bun test node-http2.test.js -t "session.socket is undefined"
  → fail (Received: <Proxy>)
bun bd test node-http2.test.js -t "session.socket is undefined"
  → pass

Full node-http2.test.js: 294 pass, 6 skip, 1 pre-existing timeout (maxSessionMemory 10k-request stress test under debug+ASAN, same on main).

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 4 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: f86a430a-84b9-4f6a-b973-c7e850ead986

📥 Commits

Reviewing files that changed from the base of the PR and between 3981e9b and 21ef4ad.

📒 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 9:49 PM PT - Jul 8th, 2026}

❌ @robobun, your commit 21ef4ad has 5 failures in Build #70778 (All Failures):

• test/js/bun/webview/webview.test.ts - code 1 on 🍎 14 aarch64
• test/js/sql/sql-prepare-false.test.ts - timeout on 🍎 14 x64
• test/cli/run/no-orphans.test.ts - code 1 on 🍎 26 aarch64
• test/js/bun/http/proxy-stress-concurrent.test.ts - code 1 on 🍎 14 x64
• test/js/workerd/html-rewriter-leak.test.ts - code 1 on 🐧 13 x64-asan

---

🧪   To try this PR locally:

bunx bun-pr 33791

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

bun-33791 --bun

[claude]

LGTM — small, well-tested Node compat fix.

Extended reasoning...

Overview

This PR fixes session.socket on both ServerHttp2Session and ClientHttp2Session to return undefined after the session is destroyed, matching Node.js's documented behavior. Previously, if the proxy had been cached before destroy, the getter kept returning it (whose traps then throw ERR_HTTP2_SOCKET_UNBOUND on instanceof/property access), and the uncached path returned null instead of undefined. The fix is a 2-line reorder in each getter: check this[bunHTTP2Socket] first and return undefined when it's gone. A new test in test/js/node/http2/node-http2.test.js covers both the post-destroy undefined return and that a proxy reference captured before destroy still throws ERR_HTTP2_SOCKET_UNBOUND (preserving existing behavior verified by test-http2-unbound-socket-proxy.js).

Security risks

None. This is a getter return-value change in the Node http2 compat layer with no auth, crypto, or input-parsing implications.

Level of scrutiny

Low. The change is mechanical (reorder a guard, swap null → undefined), applied identically to both session classes, and bunHTTP2Socket is set synchronously in both constructors (lines 3811, 4761, 4786) and nulled on destroy/close (lines 4075, 4512, 4516, 4892) — so the only time the new early-return fires is post-teardown, exactly the case being fixed. No CODEOWNER covers these files.

Other factors

The test is well-constructed: port: 0, error handlers wired to reject awaited promises, awaits the close event instead of sleeping, cleanup in finally, and asserts both the new behavior and the preserved throwing behavior of pre-captured proxy references. The PR description confirms it fails on system Bun and passes on the debug build, and that the full http2 suite still passes. No outstanding reviewer comments.

[claude]

LGTM — small, well-scoped Node compat fix with a solid test.

Extended reasoning...

Overview

This PR fixes session.socket on both ServerHttp2Session and ClientHttp2Session to return undefined after the session is destroyed, matching Node.js's documented behavior. Previously, once the proxy was cached in #socket_proxy, the getter would keep returning it even after destroy() nulled bunHTTP2Socket, causing instanceof checks and truthiness guards in cleanup handlers to throw ERR_HTTP2_SOCKET_UNBOUND. The fix is a two-line reorder in each getter: check this[bunHTTP2Socket] first and return undefined when it's gone. It also corrects the uncached fallback from null to undefined.

Security risks

None. This is a pure JS getter reorder in the Node compat layer with no auth, crypto, or untrusted-input parsing involved.

Level of scrutiny

Low. The change is mechanical and symmetric across both session classes, directly implements documented Node semantics, and preserves the existing behavior for proxy references captured before destroy (they still throw ERR_HTTP2_SOCKET_UNBOUND, verified by both the new test and the existing test-http2-unbound-socket-proxy.js). I confirmed bunHTTP2Socket is nulled on the destroy/close paths, so the guard is live.

Other factors

The new test is well-constructed: port: 0, error events wired to reject the awaited promises, try/finally cleanup, and it exercises both the fresh-getter and stale-captured-proxy paths. The PR description shows it fails under USE_SYSTEM_BUN=1 and passes on the debug build. The bug-hunting system found no issues, and there is no CODEOWNERS entry for this file.

[claude]

LGTM — small, well-scoped Node compat fix with a test that covers both the new undefined return and the preserved ERR_HTTP2_SOCKET_UNBOUND behavior on captured proxies.

Extended reasoning...

Overview

This PR fixes session.socket on both ServerHttp2Session and ClientHttp2Session to return undefined after the session is destroyed, matching Node.js's documented behavior. Previously, if the proxy had been cached, the getter kept returning the stale Proxy whose traps throw ERR_HTTP2_SOCKET_UNBOUND (breaking instanceof and truthiness guards in cleanup code); if uncached, it returned null instead of undefined. The fix reorders the getter to check this[bunHTTP2Socket] first and return undefined when it's gone. A new test in test/js/node/http2/node-http2.test.js verifies the post-destroy getter is undefined, that instanceof net.Socket no longer throws, and that a proxy reference captured before destroy still throws ERR_HTTP2_SOCKET_UNBOUND (preserving the behavior tested by Node's test-http2-unbound-socket-proxy.js).

Security risks

None. This is a pure getter-ordering change in the Node compat layer that only affects what session.socket returns after teardown. No auth, crypto, permissions, or untrusted-input parsing is touched.

Level of scrutiny

Low. The diff is ~8 lines of logic across two symmetric getters plus a focused test. bunHTTP2Socket is assigned synchronously in both constructors (lines 3811, 4761/4786) and nulled on destroy/close (4075, 4512, 4516, 4892), so the reordered check cannot cause undefined to be returned before destroy. The null → undefined change for the uncached path is a strict improvement toward Node parity.

Other factors

• Both session classes are updated identically, covering the whole class of the bug.
• The test wires error events to reject and awaits close rather than sleeping, follows harness conventions (port: 0, try/finally cleanup), and asserts both the fix and the preserved unbound-proxy contract.
• No CODEOWNERS entry matches src/js/node/http2.ts.
• Bug hunter found no issues; PR description confirms the test fails on system Bun and passes on the debug build, and the full http2 suite still passes.

[robobun]

Diff is green. The only red lane on build #70778 is test/js/workerd/html-rewriter-leak.test.ts on debian-13-x64-asan (leak threshold: expected <25 MB, got 40.67 MB), which is unrelated to this http2 change. No http2 test failed on any lane. Ready for review.

[robobun]

Closing: this change is already on main.

#34432 (a07354a) made both session.socket getters in src/js/node/http2.ts return undefined once the session has detached from its socket. The check runs before the cached proxy is returned, which is the same fix as this PR. This PR has no other change under src/.

The repro from this PR prints undefined and false on 1.4.3-canary.1+4ff919377, with no ERR_HTTP2_SOCKET_UNBOUND throw. The result is the same for the client session and for the server session, and it matches Node v26.3.0. A proxy captured before destroy() still throws ERR_HTTP2_SOCKET_UNBOUND, as in Node. The test from this PR passes on that build without the src/ change.

Main has no test for this behavior. #42255 carries the test from this PR over and extends it to the server session.