node:http2: validate goaway() arguments the same way on client and server sessions

[robobun]

Repro

const http2 = require("node:http2");
const server = http2.createServer();
server.on("session", session => {
  for (const fn of [() => session.goaway(-1), () => session.goaway(1.5), () => session.destroy(new Error("x"), -1)]) {
    try { fn(); console.log("server: ok, destroyed =", session.destroyed); } catch (e) { console.log("server:", e.code, "destroyed =", session.destroyed); }
  }
});
server.listen(0, "127.0.0.1", () => {
  const client = http2.connect(`http://127.0.0.1:${server.address().port}`);
  client.on("error", () => {});
  client.on("connect", () => {
    for (const fn of [() => client.goaway(true), () => client.goaway(0, {}), () => client.goaway(0, 0, "x")]) {
      try { fn(); console.log("client: no throw"); } catch (e) { console.log("client:", e.code, e.message); }
    }
    client.destroy(); server.close();
  });
});

Bun 1.4.0 / main:

client: undefined Expected errorCode to be a number
client: undefined Expected lastStreamId to be a number
client: no throw
server: ERR_OUT_OF_RANGE destroyed = false
server: ERR_OUT_OF_RANGE destroyed = false
server: ERR_OUT_OF_RANGE destroyed = false

Node v26.3.0: the three client calls throw ERR_INVALID_ARG_TYPE (The "code" argument must be of type number. Received type boolean (true), the same for "lastStreamID", and The "opaqueData" argument must be an instance of Buffer, TypedArray, or DataView. Received type string ('x')); the three server calls succeed, the GOAWAY carries 0xffffffff / 1 / 0xffffffff, and after the destroy() the session reports destroyed = true.

Cause

Node implements goaway() once, on Http2Session (lib/internal/http2/core.js, Http2Session#goaway): validateBuffer(opaqueData) when given, then validateNumber(code), then validateNumber(lastStreamID). The binding converts the code to a uint32 on the way out, so any number is accepted. Bun has one goaway() per session class and the two had drifted in opposite directions:

• ClientHttp2Session#goaway() validated nothing and passed the arguments to the native parser. A non-numeric code or lastStreamID surfaced as a plain Error with no .code, and anything that is not an ArrayBuffer view as opaqueData ("x", null) was treated as absent: no throw, and a GOAWAY with empty debug data still went out.
• ServerHttp2Session#goaway() had the right shape but used validateInteger(code, "code", 0, kMaxUint32) (it was validateNumber until node:http2: rewritten inbound engine, batched write path, server push, +290 node v26.3.0 tests (79% passing) #31584), so goaway(-1), goaway(1.5) and goaway(NaN) threw ERR_OUT_OF_RANGE. destroy(error, code) sends its code through goaway(), so a server destroy() with such a code threw from the middle of the teardown and left the session with destroyed === false. The client already behaved like Node on all of these.

Fix

One module-level validateGoawayArguments() with Node's validators, next to the existing shared validateWindowSize(), called by both goaway() methods. Copying the block between the two classes is what let them drift in the first place, so they now share it. kMaxUint32 had no other user and is removed; the two destroy() comments that named validateInteger are updated.

Observable changes: the client now throws Node's ERR_INVALID_ARG_TYPE errors (including for a raw ArrayBuffer or null opaqueData, which it used to accept), and the server now accepts any numeric code like Node and the client. Internal callers (close(), destroy()) always pass a number, 0 and a Buffer. The native side still saturates codes above 2^31 - 1; #37558 fixes that read separately, and the negative-code rows in the new test below pin that the conversion has to keep Node's uint32 wrap (-1 goes out as 0xffffffff).

Verification

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

• "http2 client and server sessions validate goaway() arguments identically": the same invalid-argument matrix (Node's types plus a string and an ArrayBuffer, for each of the three parameters) against the client and the server session of one connection, expecting identical name/code/message. Then one valid GOAWAY from the client, which has to be the first one the server receives (frames are ordered, so this proves the rejected calls sent nothing), and the destroyed check still coming before validation. On main the client half fails with the output above.
• "http2 session.goaway() and destroy() accept any numeric code like node and send it as a uint32": goaway(-1), goaway(1.5), goaway(NaN) and destroy(error, -1) issued from each side, checking what the peer's 'goaway' event reports and whether the issuing session is destroyed. The expected table is what Node v26.3.0 produces for the same script. On main the four server rows fail (threw: "ERR_OUT_OF_RANGE", and destroyed: false for the destroy row).
• "http2 client session.goaway() sends opaqueData given as any Buffer, TypedArray, or DataView view": the views validateBuffer admits (offset views and a DataView) still reach the peer intact.

The existing ported server test (test-http2-server-shutdown-options-errors) only uses non-number inputs, so it passes before and after. The rest of node-http2.test.js, h2-conformance.test.ts, and the ported Node tests around goaway/destroy/close (test-http2-client-destroy, test-http2-propagate-session-destroy-code, test-http2-server-session-destroy, test-http2-goaway-opaquedata, test-http2-server-shutdown-*, and others) pass with the debug build.

---

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

fails on main (without fix)

ASAN without fix: 2 failed, 6 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/node-http2.test.js"
bun test v1.4.1 (a6c4cc276)

test/js/node/http2/node-http2.test.js:
(pass) node none > Client Basics > should be able to send a GET request [803.85ms]
(pass) node none > Client Basics > should be able to send a POST request [536.24ms]
(pass) node none > Client Basics > constants [18.00ms]
(pass) node none > Client Basics > getDefaultSettings [6.56ms]
(pass) node none > Client Basics > getPackedSettings/getUnpackedSettings [16.56ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is too small [5.07ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a multiple of 6 bytes [2.94ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a buffer [4.91ms]
(pass) node none > Client Basics > should be able to send data using end [558.76ms]
(pass) node none > Client Basics > should be able to mutiplex GET requests [549.30ms]
(pass) node none > Client Basics > http2 should receive remoteSettings when receiving 
... (truncated)

release without fix: 10 failed, 6 skipped
bun test v1.4.1-canary.1 (a6c4cc276)

test/js/node/http2/node-http2.test.js:
(pass) node none > Client Basics > constants [0.81ms]
(pass) node none > Client Basics > getDefaultSettings [0.17ms]
(pass) node none > Client Basics > getPackedSettings/getUnpackedSettings [0.37ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is too small [0.15ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a multiple of 6 bytes [0.05ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a buffer [0.09ms]
(pass) node none > Client Basics > is possible to abort request [1.57ms]
(pass) node none > Client Basics > aborted event should work with abortController [0.66ms]
(pass) node none > Client Basics > aborted event should work with aborted signal [0.60ms]
(pass) node none > Client Basics > signal validation matches node: non-signal objects throw, duck-typed { aborted } is accepted [1.05ms]
(pass) node none > Client Basics > should fail to connect over HTTP/1.1 [27.38ms]
(skip) node none > Client Basics > should not leak memory
(pass) node none > Client Basics > headers cannot be bigge
... (truncated)

passes on PR (with fix)

ASAN with fix: 6 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/node-http2.test.js"
bun test v1.4.1 (a6c4cc276)

test/js/node/http2/node-http2.test.js:
(pass) node none > Client Basics > should be able to send a GET request [821.66ms]
(pass) node none > Client Basics > should be able to send a POST request [546.51ms]
(pass) node none > Client Basics > constants [18.14ms]
(pass) node none > Client Basics > getDefaultSettings [6.64ms]
(pass) node none > Client Basics > getPackedSettings/getUnpackedSettings [16.78ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is too small [5.19ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a multiple of 6 bytes [3.23ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is not a buffer [4.87ms]
(pass) node none > Client Basics > should be able to send data using end [568.64ms]
(pass) node none > Client Basics > should be able to mutiplex GET requests [558.75ms]
(pass) node none > Client Basics > http2 should receive remoteSettings when receiving 
... (truncated)

release with fix: 6 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 584ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/21] gen JS modules (bundle-modules)
Preprocess modules (7983ms)
Bundle modules (31ms)
Postprocesss modules (24ms)
Bundle Functions (457ms)
Generate Code (23ms)

[8.53s] Bundled "src/js" for production
  2595 kb
  197 internal modules
  13 native modules
  50 internal functions across 16 files
[1/5] cargo bun_runtime → libbun_runtime.a
�[1m�[92m   Compiling�[0m bun_runtime v0.0.0 (/workspace/bun/src/runtime)
�[1m�[92m    Finished�[0m `release` profile [optimized + debuginfo] target(s) in 4m 25s
[2/5] link bun-profile
[4/5] strip bun
[4/5] bun-profile --revision
1.4.1-canary.1+0be49fed7
[build] done
bun test v1.4.1-canary.1 (0be49fed7)

test/js/node/http2/node-http2.test.js:
(pass) node none > Client Basics > constants [0.81ms]
(pass) node none > Client Basics > getDefaultSettings [0.14ms]
(pass) node none > Client Basics > getPackedSettings/getUnpackedSettings [0.28ms]
(pass) node none > Client Basics > getUnpackedSettings should throw if buffer is too small [0.10ms]
(pass) node none > Client Basics > 
... (truncated)

diff hotspot

src/js/node/http2.ts                  |  26 +++---
 test/js/node/http2/node-http2.test.js | 171 ++++++++++++++++++++++++++++++++++
 2 files changed, 186 insertions(+), 11 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file

file                                   reads  edits  tests
src/js/node/http2.ts                       0      0      0
test/js/node/http2/node-http2.test.js      0      0      0

root cause · written by the author bot

Bun's client and server HTTP/2 sessions validated goaway() arguments inconsistently: the server rejected numeric codes outside an integer range that Node accepts, while the client performed no validation at all, so invalid inputs produced errors that diverged from Node's behavior on both paths. The fix extracts a shared validateGoawayArguments helper that checks the opaque data, code, and last stream ID in Node's order using the same validators Node applies, and both session types now call it before dispatching the frame. This makes out-of-range numeric codes coerce to uint32 in the native …

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: fc42e9de-e425-4bf8-8f81-286733201a65

📥 Commits

Reviewing files that changed from the base of the PR and between e1c1325 and 0be49fe.

📒 Files selected for processing (2)

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

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

Walkthrough

HTTP/2 client and server goaway() methods now use shared numeric argument validation. Tests cover invalid types, numeric-to-uint32 conversion, destroyed sessions, and opaque payloads from Buffer, Uint8Array, and DataView inputs.

Changes

HTTP/2 GOAWAY behavior

Layer / File(s)	Summary
Shared GOAWAY validation and session wiring src/js/node/http2.ts	Adds shared validation for opaque data and numeric codes and stream IDs. Server and client goaway() methods use the validator.
GOAWAY validation and wire behavior tests test/js/node/http2/node-http2.test.js	Tests synchronous errors, numeric conversion, destroyed-session behavior, transmitted GOAWAY values, and opaque payload views.

Suggested reviewers: cirospaciari, jarred-sumner

Merge Risk: ⚪ Minimal · up to 0be49

This change aligns HTTP/2 GOAWAY argument validation across client and server sessions and preserves the documented numeric-code and opaque-data behavior. No actionable merge-blocking risk remains beyond normal checks and review.

🚥 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 primary change: consistent goaway() argument validation for client and server HTTP/2 sessions.
Description check	✅ Passed	The description provides a detailed reproduction, cause, fix, observable behavior, verification steps, test coverage, and build results. It does not use the exact template headings, but it includes th…

Full details: Description check

Explanation

The description provides a detailed reproduction, cause, fix, observable behavior, verification steps, test coverage, and build results. It does not use the exact template headings, but it includes the required information in equivalent sections.

---

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

[robobun]

^{Updated 5:29 PM PT - Aug 31st, 2026}

✅ @cirospaciari, your commit 0be49fed7e7e1faa26f2d9933b000832ebbf0bd7 passed in Build #108934! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 37554

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

bun-37554 --bun

[robobun]

Status: reproduced on Bun 1.4.0 and main (da3851e57a) with the script in the description. Client session: goaway(true) / goaway(0, {}) throw a plain Error without a code (Expected errorCode to be a number / Expected lastStreamId to be a number) and goaway(0, 0, "x") does not throw. Server session: goaway(-1), goaway(1.5) and destroy(err, -1) throw ERR_OUT_OF_RANGE, the last one leaving destroyed === false. Node v26.3.0 throws ERR_INVALID_ARG_TYPE for the client cases and accepts the server ones (verified with the same script).

Scope widened after review: the first revision copied the server's validateInteger range check onto the client, which would have extended the server's destroy() problem to the client. The current revision puts Node's validators (validateBuffer / validateNumber / validateNumber) in one shared validateGoawayArguments() used by both sessions. Overflowing codes (2 ** 32, Infinity, ...) are still converted by the native read that #37558 fixes; a note about the interaction is on that PR.

Tests: test/js/node/http2/node-http2.test.js, "http2 client and server sessions validate goaway() arguments identically" (client half fails on main) and "http2 session.goaway() and destroy() accept any numeric code like node and send it as a uint32" (server rows fail on main), plus the opaqueData view round-trip. Waiting on CI.

[claude]

LGTM — copies the server session's existing goaway() validation block verbatim onto the client session so both throw the same Node-compatible errors.

Checked that validateBuffer/validateInteger/validateNumber, NGHTTP2_NO_ERROR, and kMaxUint32 are already in scope (used by the server method at http2.ts:4794), and that the two method bodies are now byte-identical.
Traced the internal client callers — close() (http2.ts:5932) and destroy() (http2.ts:6023) — both pass a numeric code, 0, and Buffer.alloc(0), so the new validation cannot break them; destroy()'s user-supplied-number path matches the server's existing behavior.
The two new tests exercise both sessions against the same invalid-argument matrix, confirm rejected calls never hit the wire, and pin that Buffer/TypedArray/DataView (including offset views) still round-trip.

Extended reasoning...

Overview

The PR adds argument validation to ClientHttp2Session#goaway() in src/js/node/http2.ts so it matches the validation already present on ServerHttp2Session#goaway(). The change is 8 lines: rename parameters to Node's names (code, lastStreamID), add validateBuffer(opaqueData) when defined, validateInteger(code, ..., 0, kMaxUint32), and validateNumber(lastStreamID). After the change the client and server methods (lines 4794-4803 and 5720-5729) are byte-identical. Two tests are added to test/js/node/http2/node-http2.test.js.

Security risks

None. This adds input validation on a user-facing API, which strictly narrows accepted inputs. No auth, crypto, filesystem, or untrusted-data parsing paths are touched.

Level of scrutiny

Low-to-medium. This is a Node.js compat fix that copies an existing, already-shipped validation block from the sibling class in the same file. All helpers referenced (validateBuffer, validateInteger, validateNumber, NGHTTP2_NO_ERROR, kMaxUint32) are already imported and used by the server method a few hundred lines up, so there is no risk of unresolved references. The PR description is thorough about the one intentional divergence from Node (validateInteger vs Node's validateNumber for code), which mirrors what the server session already does — keeping the two Bun sessions consistent is the right call here per the repo's "fix the whole class" guidance.

Other factors

• I traced the two internal call sites on the client (close() at :5932 and destroy() at :6023). Both pass a numeric code, literal 0, and Buffer.alloc(0), so the new validators pass. destroy() can forward a user-supplied number as code, which validateInteger may now reject — but the server session's destroy() at :4927 already does the same thing, so this is not new behavior introduced by this PR; it's the client catching up.
• The tests are well-constructed: they run the identical matrix against both sessions (proving parity, not just "client now throws"), assert exact error name/code/message, verify rejected calls put nothing on the wire by checking the first GOAWAY the peer receives, confirm the destroyed check still runs before validation, and pin that offset TypedArray/DataView opaqueData still round-trips. They use port: 0, local servers, Promise.withResolvers() for events, and try/finally cleanup.
• No prior review comments to address; the bug-hunting system found nothing.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/js/node/http2/node-http2.test.js`:
- Around line 1942-1964: Extend the issues matrix in the test around
issueAndReadPeer to include goaway and destroy calls using Infinity, -Infinity,
2 ** 32, and 2 ** 32 + 1, then add expected wire-code results for each case
under both client and server. Preserve the existing destroyed status and verify
the native mappings produce codes 0, 0, 0, and 1 respectively.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5bb75828-ef8f-4c03-83e1-987c4e577f37

📥 Commits

Reviewing files that changed from the base of the PR and between ae33ace and f92bb0d.

📒 Files selected for processing (2)

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

[claude]

LGTM — focused Node-compat fix that consolidates duplicated validation.

Checked: validateBuffer/validateNumber are already imported and match Node's Http2Session#goaway order; kMaxUint32 has no remaining references after removal; the client's renamed parameters (code/lastStreamID) line up with the error-message argument names the new test asserts. The three new tests cover the invalid-type matrix on both session classes, the loosened numeric acceptance on the server (including the destroy(err, -1) half-teardown regression), and the opaqueData view round-trip. My earlier inline note about resolve-only connect promises is a nit, not a blocker.

Extended reasoning...

Overview

Two files. In src/js/node/http2.ts, a new module-level validateGoawayArguments(code, lastStreamID, opaqueData) helper (validateBuffer → validateNumber → validateNumber, matching Node's Http2Session#goaway) is called from both ServerHttp2Session#goaway and ClientHttp2Session#goaway. The server drops its stricter validateInteger(code, 0, kMaxUint32) in favor of Node's type-only validateNumber; the client gains validation it never had. kMaxUint32 is removed (no other users — verified via grep), the client's parameter names are aligned to code/lastStreamID so error messages match Node, and two destroy() comments are updated to name validateNumber. Three new tests in node-http2.test.js pin the identical rejection matrix across both session types, the accept-any-number + uint32-wrap behavior (including destroy(err, -1) leaving destroyed === true), and opaqueData round-tripping for Buffer/Uint8Array/DataView views.

Security risks

None. This is argument validation for an HTTP/2 control frame; the change loosens the server's code check to match Node (any number, uint32-wrapped by native) and tightens the client's opaqueData check. No auth, crypto, or untrusted-parse surface is touched.

Level of scrutiny

Low-to-medium. ~20 net lines of production change in a built-in JS module, extracting a shared helper next to the existing validateWindowSize precedent. The behavioral delta is deliberate, cited against Node v26.3.0 source, and each direction (server loosens, client tightens) is covered by a test that fails on main. The overflow-code interaction with the native int32 read is explicitly scoped out to #37558, and the new test's -1 → 0xffffffff rows pin the wrap contract that PR must preserve.

Other factors

All prior bot feedback is resolved: the comment-cop flags were addressed by removing the explanatory comment in 47823a7 (the test header now carries the Node reference), and the CodeRabbit suggestion to add Infinity/2**32 rows was declined with a sound reason (those depend on the native saturating read #37558 fixes). My own prior inline nit about wiring error to reject on the connect/goaway awaits is explicitly minor — the same resolve-only pattern already appears throughout this file for in-process loopback servers, and a failure would still surface as a timeout rather than a false pass. The existing ported test-http2-server-shutdown-options-errors only exercises non-number inputs, so it is unaffected.

[claude]

LGTM — the earlier nit about resolve-only awaits was addressed in 1cfab5f via nextEvent, and the comment-cop feedback was resolved in 47823a7.

What was reviewed:

• validateGoawayArguments matches Node's order (validateBuffer → validateNumber × 2) and both session classes now call it; kMaxUint32 had no other users.
• Client goaway() param rename to bare NGHTTP2_NO_ERROR is safe — it's destructured at module scope (same as the server method already used).
• nextEvent helper: verified that a late 'error' after 'goaway' resolves is a no-op reject on an already-settled promise, and the surviving client.on('error', () => {}) / server.on('sessionError', () => {}) listeners keep those emissions from becoming unhandled.
• Ruled out that loosening the server's code check breaks internal callers — close()/destroy() always pass a number.

Extended reasoning...

Overview

Two files: src/js/node/http2.ts gains a module-level validateGoawayArguments() helper (placed next to the existing validateWindowSize()) that both ClientHttp2Session#goaway() and ServerHttp2Session#goaway() now call. The server side switches from validateInteger(code, 'code', 0, kMaxUint32) to Node's validateNumber(code, 'code'); the client side gains validation it never had. The now-unused kMaxUint32 constant is removed and two destroy() comments are updated to reference validateNumber. test/js/node/http2/node-http2.test.js gains three tests plus a nextEvent helper.

Security risks

None. This is argument validation for goaway() — the change loosens the server-side range check to match Node (any number → uint32 wrap on the wire) and adds type checks on the client side. No auth, crypto, or untrusted-parse paths are touched.

Level of scrutiny

Low-to-medium. It's a focused Node.js compat fix (~15 source lines) that consolidates two drifted implementations into one shared helper matching Node v26.3.0's Http2Session#goaway exactly. The PR description cites the Node source, the validation order matches, and the observable-change surface is well characterized (client now throws ERR_INVALID_ARG_TYPE; server accepts any numeric code). I confirmed validateBuffer/validateNumber are already imported, NGHTTP2_NO_ERROR is destructured at module scope so the client's bare reference resolves, and kMaxUint32 had no remaining users.

Other factors

All prior review threads are resolved: the comment-cop long-comment warnings were addressed in 93bc3b2/47823a7f (comment removed, rationale lives in the test header); the coderabbit note about overflow values was explained as belonging to #37558 (native uint32 conversion); my earlier nit about resolve-only awaits was addressed in 1cfab5f with the nextEvent helper that rejects on 'error'/'close'. The tests are hermetic (loopback, listen(0)), assert exact error name/code/message, prove rejected calls sent nothing on the wire via frame ordering, and cover both session sides plus the destroy() half-teardown regression. The change follows the repo's "one source of truth" and "fix the whole class" review rules by design.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[claude]

LGTM — the nextEvent helper addresses the earlier nit about wiring error/close to reject the awaited promises, and nothing else has changed since besides the merge from main.

Checked: validateGoawayArguments matches Node's validation order (opaqueData → code → lastStreamID) and sits alongside validateWindowSize with the same hideFromStack treatment; the removed kMaxUint32 has no remaining references and validateInteger is still used elsewhere in the file so its import stays; the destroyed check still precedes validation on both sessions; tests use port: 0, clean up in try/finally, and await events rather than sleeping.

Extended reasoning...

Overview

This PR unifies goaway() argument validation between ClientHttp2Session and ServerHttp2Session in src/js/node/http2.ts by extracting a shared validateGoawayArguments() helper (mirroring the existing validateWindowSize pattern directly above it). The server path switches from validateInteger(code, "code", 0, kMaxUint32) to validateNumber(code, "code") to match Node, and the client path gains validation it previously lacked entirely. The now-unused kMaxUint32 constant is removed and two stale comments are updated. Three new tests in test/js/node/http2/node-http2.test.js cover the identical-validation contract, uint32 coercion of numeric codes on the wire from both sides, and opaqueData transmission for Buffer/Uint8Array/DataView views.

Security risks

None. This is argument-validation plumbing for an HTTP/2 control frame; the change relaxes the server's code check from integer-in-range to any-number (matching Node), letting the native layer coerce to uint32 as it already does for the client. No auth, crypto, filesystem, or untrusted-data parsing is involved.

Level of scrutiny

Low-to-moderate. The source change is ~15 lines, purely mechanical, and follows the exact pattern of the adjacent validateWindowSize helper (shared function + hideFromStack). The validation order (opaqueData first, then code, then lastStreamID) matches Node's Http2Session#goaway in lib/internal/http2/core.js. I confirmed kMaxUint32 has no remaining references and validateInteger remains used elsewhere in the file, so the import is not dead. No CODEOWNERS entry covers these paths.

Other factors

My only prior concern — resolve-only event awaits with no rejection path — was addressed in commit 1cfab5f by introducing nextEvent(), which wires error and close to reject and removes its listeners in .finally(). Since then the only new commit is a merge from main. The tests are well-structured: port: 0, try/finally cleanup, event-driven awaits (no sleeps), and a table-driven assertion that pins the exact uint32 wire values Node produces. The PR timeline shows no outstanding human reviewer objections; all resolved threads are from bots.

[cirospaciari]

@robobun please run the tests this PR adds or changes on Node.js v26.3.0 and report whether each one passes or fails.
They must pass on both Bun and Node v26.3.0, so fix any case that only passes on Bun.

[robobun]

Ran the three tests this PR adds on Node v26.3.0 and on the PR build of Bun (0be49fe). All pass on both. No test passes only on Bun, so there is nothing to change.

The test file imports bun:jsc and harness, so it cannot load under Node as a whole. I copied the three new tests and the nextEvent helper verbatim into a standalone file with a thin it / expect shim over node:test and node:assert, then ran that file on both runtimes.

Test	Node v26.3.0	Bun (PR build)
http2 client and server sessions validate goaway() arguments identically	pass	pass
http2 session.goaway() and destroy() accept any numeric code like node and send it as a uint32	pass	pass
http2 client session.goaway() sends opaqueData given as any Buffer, TypedArray, or DataView view	pass	pass

Node: node --test goaway.mjs, 3 pass, 0 fail, repeated 5 times with the same result.
Bun: bun-debug test goaway.test.mjs on the same file, 3 pass, 0 fail. The real file, bun bd test test/js/node/http2/node-http2.test.js -t goaway, gives 27 pass, 0 fail.

The PR does not change any existing test. The only other diff in the test file is the new nextEvent helper, which the three tests above use.