test(http2): give the detached-payload subprocess cases a debug-scaled timeout

[robobun]

Problem

• bun bd test test/js/node/http2/node-http2.test.js intermittently fails in the describe.concurrent block "http2 DATA payload survives its ArrayBuffer being detached/resized by transport JS mid-send" with this test timed out after 5000ms. Reported independently from four debug-build runs (1 to 4 cases per run); here, 10 runs of the block on a debug build had a timeout in 6 of them, 12 timeouts across 5 of the 9 cases.
• Each of the 9 cases spawns a bun subprocess (run() in the block, node-http2.test.js:3407) and none of the 7 it() / it.each() calls passes a timeout, so they get the 5 s default.
• On a debug build a subprocess spends ~2 s before the scenario starts (bun-debug -e 'require("node:http2")' takes 1.9 s here, 2.8 s on the reporting machine, against 0.3 s for an empty -e), and the 9 subprocesses start at once, so the cases complete in 2.5 to 6.8 s. On a release build the same cases take 47 to 81 ms each.

Fix

• Define timeout = 15_000 * ASAN_MULTIPLIER in the block and pass it to its 7 it() / it.each() calls (9 cases). ASAN_MULTIPLIER is the file's existing constant (isDebug ? 15 : isASAN ? 3 : 1), and 15_000 * ASAN_MULTIPLIER is the value the file already uses for its other debug-slow test (node-http2.test.js:2339); the sibling node-http2-streams-rehash.test.ts uses the same pattern. The remaining lines in the diff are prettier moving the callbacks out of the hugged form once a third argument is present (git diff -w is 36 lines).
• Raising a timeout is the right fix here rather than shrinking the workload: the time is debug-build process start plus loading node:http2, which every subprocess pays before the test's own code runs, and the subprocesses exist so that a crash or a stale read in the native send path fails one case instead of the runner. Running the cases serially would not help either: the TLS cases take ~4 s on their own under debug, and the block would take ~30 s instead of ~6 s.
• Values per build: 225 s debug, 45 s ASAN, 15 s release. These are ceilings for a hung case, not durations; the cases still finish in the times above. In CI the explicit value replaces the runner's --timeout (90 s, 270 s on the ASAN lane) for these 9 cases; the whole file takes 6.8 s on the release lane and 21 s on the ASAN lane, so they stay far inside it.
• test(http2): measure the HPACK overhead instead of sweeping it in the DATA header cork straddle test #37832 replaces the pad-length sweep in one of the failing cases ("DATA frame header straddling the cork flush") and deliberately leaves the others' timeouts alone; the other four cases that timed out here have nothing to shrink. The two PRs are complementary; whichever lands second has a few-line conflict on that one case.
• Left alone on purpose: the file's standalone subprocess tests (padded DATA write, setNextStreamID at the edges, header names longer than 4096 bytes, session teardown from a socket write, socket chunk transferred by a frame event handler). They take 2.7 to 3.7 s under debug, run one at a time, and have not been seen timing out in any of the runs above.
• Verification (debug build, 16 vCPU container with a 12 CPU quota):
  • 10 runs of the block without the change: 6 runs with at least one timeout, 12 in total (DATA frame header x4, TLSSocket ... (straddle) x3, TLSSocket ... (tail) x2, flow-control-limited tail x2, native writer (transfer) x1).
  • 10 runs of the block with the change: 90 of 90 cases pass; the slowest completions were 6.8 s, 6.1 s, 5.4 s and 5.2 s, i.e. four cases that would have timed out.
  • bun bd test test/js/node/http2/node-http2.test.js -t "DATA payload survives" --timeout 1: 9 pass with the change (a per-test timeout overrides the CLI default, so this shows the value reaches all 9 cases, the it.each ones included); 9 time out without it.
  • Whole file with the change on the debug build: 357 pass, 6 skip, 0 fail. The block under the release binary: 9 pass in 0.4 s.

Background

• bun:test resolves a test's timeout as: the test's own argument, else setDefaultTimeout(), else the --timeout flag, whose default is 5000 ms (src/runtime/test_runner/ScopeFunctions.rs:746, src/options_types/context.rs:483). Bun's CI runner always passes --timeout (half the per-file budget, 3x that on ASAN; scripts/runner.node.mjs:1964), so the 5 s default only applies to plain bun test / bun bd test runs, which is why the failure shows up locally and not in CI.
• ASAN_MULTIPLIER at the top of the file scales timeouts for builds that are slower than release: 15x for a debug build, 3x for the release+ASAN build CI uses. Those ratios match this file's measured runtimes (~105 s debug and 21 s ASAN against 6.8 s release).
• The block's cases run in subprocesses because they detach or shrink an ArrayBuffer while the native http2 code is still sending from it; if that ever reads freed memory or crashes, the subprocess dies and the case fails, instead of taking the test runner down with it.

Per-case durations, debug build, unfixed, max over 10 runs of the block

5013 ms  TLSSocket over a JS Duplex against a real server (straddle)      (timed out 3/10)
5011 ms  TLSSocket over a JS Duplex against a real server (tail)          (timed out 2/10)
5006 ms  flow-control-limited tail queued after a flush                   (timed out 2/10)
5001 ms  DATA frame header straddling the cork flush                      (timed out 4/10)
5000 ms  another session's transport JS ... native writer (transfer)      (timed out 1/10)
4721 ms  another session's transport JS ... native writer (resize0)
4368 ms  single DATA frame straddling the cork flush (transfer)
3915 ms  another session's transport JS running on cork handover
3743 ms  single DATA frame straddling the cork flush (resize0)

Same block with the change, max over 10 runs (all pass):

6816 ms  DATA frame header straddling the cork flush
6107 ms  TLSSocket over a JS Duplex against a real server (straddle)
5443 ms  TLSSocket over a JS Duplex against a real server (tail)
5186 ms  flow-control-limited tail queued after a flush
4811 ms  another session's transport JS ... native writer (transfer)
4652 ms  single DATA frame straddling the cork flush (transfer)
4644 ms  another session's transport JS ... native writer (resize0)
4471 ms  another session's transport JS running on cork handover
4042 ms  single DATA frame straddling the cork flush (resize0)

Release binary, one run of the block: 47 to 81 ms per case, 0.3 s for the block.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 19 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: 83fce735-07d4-4a33-9d7b-1e792d907ecf

📥 Commits

Reviewing files that changed from the base of the PR and between b7a0431 and 55ba9b1.

📒 Files selected for processing (1)

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

---

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

[claude]

LGTM — test-only de-flake that passes the file's existing 15_000 * ASAN_MULTIPLIER timeout to the 6 it/it.each calls (9 cases) in the detached-payload describe.concurrent block.

What was reviewed:

• Confirmed ASAN_MULTIPLIER is defined at line 20 and the same 15_000 * ASAN_MULTIPLIER value is already in use at line 2339, so this matches the file's convention.
• Verified every it/it.each inside the block now receives the timeout (6 calls, none missed); the bulk of the diff is prettier un-hugging callbacks once the third arg is added.
• Checked that no assertions or test bodies changed — this only raises the per-test ceiling for debug/ASAN subprocess startup, with a comment stating why the workload can't be shrunk.

Extended reasoning...

Overview

This PR touches a single test file, test/js/node/http2/node-http2.test.js, and adds an explicit timeout argument to the 6 it()/it.each() calls inside the describe.concurrent block that spawns subprocesses to exercise ArrayBuffer detach/resize during http2 DATA sends. A local const timeout = 15_000 * ASAN_MULTIPLIER is introduced with a comment explaining that debug-build subprocesses spend ~2 s just loading node:http2 before the scenario begins, pushing concurrent cases past the 5 s default. The rest of the diff is prettier reflowing the calls from hugged to non-hugged form once a third argument appears. No test bodies, assertions, or fixtures change.

Security risks

None. This is a timeout adjustment in a test file; no production code, no inputs, no auth/crypto/permissions surface.

Level of scrutiny

Low. Test-only, mechanical, and follows an established pattern already present in the same file (line 2339 uses the identical 15_000 * ASAN_MULTIPLIER value) and in the sibling node-http2-streams-rehash.test.ts. REVIEW.md cautions against raising timeouts instead of shrinking workload, but the PR description directly addresses this: the cost is debug-build process start plus require("node:http2"), paid before any test code runs, and subprocess isolation is load-bearing (crash/UAF detection). That reasoning is sound and the added comment records it in-file.

Other factors

• Verified all it/it.each calls in the block are covered — 3 plain it + 3 it.each × 2 params = 9 cases (the description says 7 calls, but the block actually has 6; all 6 are updated and the 9-case count is correct).
• The PR description includes measured before/after data (10 runs each) and a --timeout 1 sanity check confirming the argument reaches every case including it.each.
• No prior reviews from me or other humans on the timeline; only a coderabbit rate-limit notice.
• The change does not weaken any assertion, skip any test, or alter what the tests verify — it only prevents debug-build local runs from spuriously timing out.

[robobun]

Status: reproduced and fixed; waiting on CI.

• Reproduced on a debug build with bun bd test test/js/node/http2/node-http2.test.js -t "DATA payload survives": 10 runs of the unfixed block, 6 with at least one timed out after 5000ms, 12 timeouts across 5 of the 9 cases. The same cases take 47 to 81 ms on the release binary.
• With this change: 10 runs, 90 of 90 cases pass (slowest completions 5.2 to 6.8 s); the whole file passes on the debug build (357 pass, 6 skip); --timeout 1 shows the explicit value reaches all 9 cases.
• Related: test(http2): measure the HPACK overhead instead of sweeping it in the DATA header cork straddle test #37832 shrinks one of these cases and leaves the other timeouts alone; the two are complementary.

[robobun]

^{Updated 4:30 AM PT - Aug 13th, 2026}

✅ @robobun, your commit 55ba9b158d180d22c523a4fa31e7f82796e5585e passed in Build #94254! 🎉

---

🧪   To try this PR locally:

bunx bun-pr 38078

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

bun-38078 --bun