test(serve-body-leak): give release-asan the same 60s per-test timeout as debug - #34297
Conversation
…t as debug The streaming-echo case runs at ~31s median (27-39s across 35 CI runs) on the debian 13 x64-asan lane, so the 40s per-test timeout leaves next to no margin. On a slower-than-typical EC2 instance (build 73611) every case in the file ran ~35% slower and streaming-echo timed out on all four retries. The test file already scales the end_memory threshold for ASAN (PR #32520), and scripts/runner.node.mjs scales the default per-test timeout 3x on ASAN for the same reason; this explicit per-test timeout simply was not given the same treatment.
|
Warning Review limit reached
Next review available in: 1 minute Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
|
Updated 11:05 PM PT - Jul 15th, 2026
⏳ @robobun, your commit b44cd96 is still building in |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-only timeout bump for the ASAN lane, matching the existing isDebug || isASAN convention.
What was reviewed:
isASANis already imported from harness and used in this file for theend_memorythreshold, so no new import needed.- Release (non-ASAN, non-debug) lanes keep the 40s budget unchanged.
- Operator precedence:
isDebug || isASAN ? 60_000 : 40_000parses as(isDebug || isASAN) ? ..., which is the intended grouping.
Extended reasoning...
Overview
Single-line change in test/js/bun/http/serve-body-leak.test.ts: the per-test timeout expression isDebug ? 60_000 : 40_000 becomes isDebug || isASAN ? 60_000 : 40_000, plus a two-line explanatory comment. No production code, no assertion changes, no test logic changes.
Security risks
None. Test-only timeout constant; no auth, crypto, network surface, or user-facing code touched.
Level of scrutiny
Low. This is a mechanical CI-budget adjustment following an existing pattern (the same file already branches on isASAN for the end_memory threshold, and the PR description cites 16 other files using isDebug || isASAN for the same purpose). The change only loosens a timeout on an instrumented build lane; the plain release lane keeps its 40s budget, so this cannot mask a regression there. Verified that isASAN is already in the import list at the top of the file.
Other factors
- Operator precedence checked:
||binds tighter than?:, so no parentheses are needed. - The added comment is concise and records the measured CI data justifying the number, which is exactly what CLAUDE.md asks for (durable non-obvious content).
- No prior reviewer comments to address; no bugs found by the bug-hunting pass.
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
|
Self-review clean. Build 73652: all 20 x64-asan test shards passed (serve-body-leak included), so the target lane is green. Remaining reds are unrelated to this one-line test-file change: |
* upstream/main: (57 commits) 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 (oven-sh#32488) expect: fix panic in toBeArrayOfSize/toHaveBeenCalledTimes with length > i32 max (oven-sh#32266) lexer: fix TOKEN_TO_STRING[TColon] showing " =" instead of ":" (oven-sh#34253) Bun.Terminal: write() returns bytes accepted, fire drain on POSIX (oven-sh#34289) test(serve-body-leak): give release-asan the same 60s per-test timeout as debug (oven-sh#34297) worker: mark the context terminating before the final concurrent-queue drain (oven-sh#34278) buffer: wrap negative ucs2 indexOf offset against raw byte length for Buffer needles (oven-sh#34273) fs.promises.watch: yield events with a null prototype (oven-sh#34279) child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED (oven-sh#34268) Fix asString assertion when passing String objects as signals (oven-sh#34265) Buffer: carry size_t through toString/write so length 2^32 doesn't wrap to 0 (oven-sh#34274) test: use tempDir in log-test.test.ts instead of hardcoded /tmp path (oven-sh#34294) tty: track raw mode per handle instead of per process (oven-sh#33527) test: expect the bumped mimalloc SHA in process.versions Return freed memory to the OS on a background thread instead of the JS thread (oven-sh#34181) Move WTFTimer out of the shared timer heap to fix a cross-thread race (oven-sh#33131) test: update block-scoped enum lowering expectations to let (oven-sh#34287) Error.captureStackTrace: install .stack as non-enumerable (oven-sh#34259) js_parser: treat "async as T" / "async satisfies T" as a cast, not an arrow (oven-sh#34246) js_parser: accept `!`, `#name`, and `export @dec` in standard decorator grammar (oven-sh#34245) ...
* upstream/main: (70 commits) 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 (oven-sh#32488) expect: fix panic in toBeArrayOfSize/toHaveBeenCalledTimes with length > i32 max (oven-sh#32266) lexer: fix TOKEN_TO_STRING[TColon] showing " =" instead of ":" (oven-sh#34253) Bun.Terminal: write() returns bytes accepted, fire drain on POSIX (oven-sh#34289) test(serve-body-leak): give release-asan the same 60s per-test timeout as debug (oven-sh#34297) worker: mark the context terminating before the final concurrent-queue drain (oven-sh#34278) buffer: wrap negative ucs2 indexOf offset against raw byte length for Buffer needles (oven-sh#34273) fs.promises.watch: yield events with a null prototype (oven-sh#34279) child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED (oven-sh#34268) Fix asString assertion when passing String objects as signals (oven-sh#34265) Buffer: carry size_t through toString/write so length 2^32 doesn't wrap to 0 (oven-sh#34274) test: use tempDir in log-test.test.ts instead of hardcoded /tmp path (oven-sh#34294) tty: track raw mode per handle instead of per process (oven-sh#33527) test: expect the bumped mimalloc SHA in process.versions Return freed memory to the OS on a background thread instead of the JS thread (oven-sh#34181) Move WTFTimer out of the shared timer heap to fix a cross-thread race (oven-sh#33131) test: update block-scoped enum lowering expectations to let (oven-sh#34287) Error.captureStackTrace: install .stack as non-enumerable (oven-sh#34259) js_parser: treat "async as T" / "async satisfies T" as a cast, not an arrow (oven-sh#34246) js_parser: accept `!`, `#name`, and `export @dec` in standard decorator grammar (oven-sh#34245) ...
* upstream/main: (52 commits) 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 (oven-sh#32488) expect: fix panic in toBeArrayOfSize/toHaveBeenCalledTimes with length > i32 max (oven-sh#32266) lexer: fix TOKEN_TO_STRING[TColon] showing " =" instead of ":" (oven-sh#34253) Bun.Terminal: write() returns bytes accepted, fire drain on POSIX (oven-sh#34289) test(serve-body-leak): give release-asan the same 60s per-test timeout as debug (oven-sh#34297) worker: mark the context terminating before the final concurrent-queue drain (oven-sh#34278) buffer: wrap negative ucs2 indexOf offset against raw byte length for Buffer needles (oven-sh#34273) fs.promises.watch: yield events with a null prototype (oven-sh#34279) child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED (oven-sh#34268) Fix asString assertion when passing String objects as signals (oven-sh#34265) Buffer: carry size_t through toString/write so length 2^32 doesn't wrap to 0 (oven-sh#34274) test: use tempDir in log-test.test.ts instead of hardcoded /tmp path (oven-sh#34294) tty: track raw mode per handle instead of per process (oven-sh#33527) test: expect the bumped mimalloc SHA in process.versions Return freed memory to the OS on a background thread instead of the JS thread (oven-sh#34181) Move WTFTimer out of the shared timer heap to fix a cross-thread race (oven-sh#33131) test: update block-scoped enum lowering expectations to let (oven-sh#34287) Error.captureStackTrace: install .stack as non-enumerable (oven-sh#34259) js_parser: treat "async as T" / "async satisfies T" as a cast, not an arrow (oven-sh#34246) js_parser: accept `!`, `#name`, and `export @dec` in standard decorator grammar (oven-sh#34245) ...
* upstream/main: (52 commits) 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 (oven-sh#32488) expect: fix panic in toBeArrayOfSize/toHaveBeenCalledTimes with length > i32 max (oven-sh#32266) lexer: fix TOKEN_TO_STRING[TColon] showing " =" instead of ":" (oven-sh#34253) Bun.Terminal: write() returns bytes accepted, fire drain on POSIX (oven-sh#34289) test(serve-body-leak): give release-asan the same 60s per-test timeout as debug (oven-sh#34297) worker: mark the context terminating before the final concurrent-queue drain (oven-sh#34278) buffer: wrap negative ucs2 indexOf offset against raw byte length for Buffer needles (oven-sh#34273) fs.promises.watch: yield events with a null prototype (oven-sh#34279) child_process: latch stdin write EPIPE as 'error' + destroy, fail later writes with ERR_STREAM_DESTROYED (oven-sh#34268) Fix asString assertion when passing String objects as signals (oven-sh#34265) Buffer: carry size_t through toString/write so length 2^32 doesn't wrap to 0 (oven-sh#34274) test: use tempDir in log-test.test.ts instead of hardcoded /tmp path (oven-sh#34294) tty: track raw mode per handle instead of per process (oven-sh#33527) test: expect the bumped mimalloc SHA in process.versions Return freed memory to the OS on a background thread instead of the JS thread (oven-sh#34181) Move WTFTimer out of the shared timer heap to fix a cross-thread race (oven-sh#33131) test: update block-scoped enum lowering expectations to let (oven-sh#34287) Error.captureStackTrace: install .stack as non-enumerable (oven-sh#34259) js_parser: treat "async as T" / "async satisfies T" as a cast, not an arrow (oven-sh#34246) js_parser: accept `!`, `#name`, and `export @dec` in standard decorator grammar (oven-sh#34245) ... # Conflicts: # test/js/bun/websocket/websocket-server.test.ts
2 participants
test/js/bun/http/serve-body-leak.test.tswent red on the debian 13 x64-asan lane in build 73611: the "should not leak memory when streaming the body and echoing it back" case timed out at 40s on all four retries. The PR under test (#33580, ttysetRawMode) does not touch anything related, so this is the test's own budget.Cause
This is not a hang and not a code regression. Scraping the timestamped logs from 35 recent x64-asan runs (builds 73562-73623) plus 4 pre-#33193 runs:
callIgnorecallStreamingEchoThe streaming-echo case has been running at ~31s median on ASAN since well before the webstreams rewrite (pre-#33193 samples: 28.2 / 30.9 / 28.2 / 31.6s), so the 40s budget has always been tight there. On build 73611 every case in the file ran ~35% slower than typical (the shard landed on a slower EC2 instance;
callIgnore23.6s vs a typical ~17s), which is enough to push echo past 40s. A local 15000-request/streaming-echoprobe against a debug build runs at a flat ~395 req/s with no stalls, confirming throughput, not a hang.The file already scales its
end_memorythreshold for ASAN (#32520), andscripts/runner.node.mjsalready applies a 3x ASAN multiplier to the default--timeoutfor the same reason, but that multiplier does not reach tests that pass their own explicit third-argument timeout. Skipping on ASAN was tried in #28301 and reverted in #28337; this change keeps the test running there with a budget that matches the measured cost.Fix
Matches the
isDebug || isASANconvention already used by 16 other test files for timeouts/iteration counts. 60s is ~2x the ASAN median and ~1.4x the extrapolated worst case (73611). Release lanes stay at 40s.Verification
USE_SYSTEM_BUN=1 bun test test/js/bun/http/serve-body-leak.test.ts: 8 pass, echo 11.4s (release, budget unchanged at 40s).bun bd test test/js/bun/http/serve-body-leak.test.ts -t 'ignoring the body': passes; file parses and the unchanged isDebug=60s branch applies under the debug build./tmp/echo-probe.tsagainst debug build: 15000/streaming-echorequests at a steady ~395 req/s, no stalls.no test proof · iteration 0 · Platform-specific test-only change; deferring to CI.