websocket: fix idleTimeout: 0 to actually disable the idle timeout

[hughescr]

What does this PR do?

Fixes websocket: { idleTimeout: 0 } on Bun.serve() so it actually disables the idle timeout, as its 0 value is intended to mean: App.h's ws() validation terminates with "Error: idleTimeout must be either 0 or greater than 8!" for anything in (0, 8) (packages/bun-uws/src/App.h:414-416), proving 0 is an intentional, distinct off value rather than an ordinary small timeout; WebSocketServerContext.rs already exempts 0 from its "round up to 8" clamp for the same reason; and Bun's own inspector/debugger connection (src/js/internal/debugger.ts:316) relies on idleTimeout: 0 to mean "never time out".

Root cause: WebSocketContextData::calculateIdleTimeoutComponents() (packages/bun-uws/src/WebSocketContextData.h) computes:

idleTimeoutComponents = {
    idleTimeout - (sendPingsAutomatically ? margin : 0),
    margin
};

For idleTimeout == 0 (with sendPingsAutomatically defaulting to true, margin == 4), 0 - 4 underflows the unsigned short field to 65532. That value flows straight into us_socket_timeout() (packages/bun-usockets/src/socket.c), which computes a real, finite tick-wheel slot: ((65532 + 3) >> 2) % 240 == 63 ticks * 4s/tick ≈ 252 seconds. So every idleTimeout: 0 websocket — including Bun's own inspector/debugger connection (src/js/internal/debugger.ts:316), which sets this explicitly to mean "never time out" — got an automatic ping at ~248s and, if unanswered, a force-close (ERR_WEBSOCKET_TIMEOUT) 4s later, instead of never timing out.

This is purely an arithmetic bug, not a design gap: us_socket_timeout(s, 0) already means "disabled" — it sets a 255 sentinel timeout slot that the tick-wheel sweep (packages/bun-usockets/src/loop.c's us_internal_timer_sweep, short_ticks always < 240) can never match. calculateIdleTimeoutComponents just never gets to pass the caller's 0 through — the subtraction corrupts it first. Bun's own config-translation layer already treats 0 as a distinct sentinel, not an ordinary small timeout: WebSocketServerContext.rs explicitly exempts 0 from its "round up to 8" clamp ("uws does not allow idleTimeout to be between (0, 8)... "). This PR finishes that intent at the layer where it actually breaks.

Fix (least invasive layer): special-case idleTimeout == 0 in calculateIdleTimeoutComponents to skip the underflowing subtraction and set the idle-detection component (.first) to 0 directly. The ping/force-close-after-end() component (.second) is left untouched — still the normal margin — because WebSocket::end() (WebSocket.h) also uses .second as a short grace period to force-close hung sockets after an app-initiated graceful close, an unrelated concern that must keep working regardless of idleTimeout. (My first draft of this fix zeroed both components and would have silently disabled that grace period too — caught by re-reading every caller of idleTimeoutComponents before finalizing.)

I checked upstream uWebSockets (uNetworking/uWebSockets@master, fetched via the GitHub contents API) and its calculateIdleTimeoutComponents (src/WebSocketContextData.h) has the identical bug, unfixed — so there's no newer upstream special-case to mirror; this fixes it directly in Bun's vendored copy, which is where the arithmetic actually breaks. I considered instead papering over this in the Rust config-translation layer (picking some other magic idleTimeout value that happens to zero out .first given the current margin formula), but rejected it: it would depend on both sendPingsAutomatically and uWS's internal margin-selection algorithm, and would break silently and non-obviously if either changes.

Blast radius (grepped idleTimeout across src/ and test/): the only in-tree code relying on this specific websocket-level idleTimeout: 0 semantics — as opposed to the separate, already-correct HTTP-level idleTimeout: 0 (a sibling field of websocket:, not inside it, which passes straight through AsyncSocket::timeout()/us_socket_timeout() with no arithmetic in between and was never affected) — are:

• src/js/internal/debugger.ts:316 — the inspector/debugger websocket, which explicitly wants "never time out" via idleTimeout: 0. This is a welcome, straightforward correctness improvement: this connection was previously living on the ~252s ping/force-close cycle described above. (Side note, not fixed here: stryker-bun-runner's prior investigation into intermittent debugger-socket silence flagged this exact idleTimeout: 0 config as a contributing-conditions candidate; this fix removes one such candidate but doesn't itself resolve that investigation.)
• test/js/bun/websocket/websocket-server.test.ts:1368 ("publish() return value reflects subscriber backpressure" suite) — uses idleTimeout: 0 to mean "won't time out during the test's own runtime"; it currently passes only because 252s exceeds the test's duration. Still passes after this fix — now correctly, not just by luck.

Everything else in the tree (dozens of occurrences across test/js/bun/http/*, test/js/web/fetch/*, src/js/node/_http_server.ts, test/js/sql/*) is either the HTTP-level idleTimeout or an unrelated SQL connection-pool idle timeout; neither goes through calculateIdleTimeoutComponents, so neither is affected by this change.

How did you verify your code works?

• bun bd -p "Bun.version" builds clean.

• cd src/js && bun x tsc --noEmit: 1628 errors — identical to the unmodified-main baseline (this PR touches no .ts production files, only a vendored C++ header and a test file).

• New regression coverage, appended to the existing describe/it.concurrent blocks in test/js/bun/websocket/websocket-server.test.ts (describe.concurrent("websocket idleTimeout: 0", ...)) rather than a new file, per this repo's "extend the module's existing test file" convention. Two tests, against a deliberately unresponsive raw-socket client (manual HTTP Upgrade handshake, then silence — no pong, no data):

  1. Sanity: idleTimeout: 8 (uWS's minimum reliable granularity, same value used by the existing "should allow use of custom timeout" test in serve.test.ts) still sends an unmasked ping frame (0x89 0x00) and then force-closes the idle connection, within a bounded 20s window — proves the ping/close mechanism itself functions correctly against this harness.
  2. Regression: idleTimeout: 0, under the exact same conditions, produces neither a ping nor a close within that same 20s window.
     bun bd test test/js/bun/websocket/websocket-server.test.ts -t "idleTimeout: 0", run 3 times: 2 pass / 0 fail every time (6/6 assertions total), ~20.5s per run.

• Honesty about this test's limits (asked for explicitly, so stating plainly): I also ran the new tests against system Bun 1.3.14 — USE_SYSTEM_BUN=1 bun test test/js/bun/websocket/websocket-server.test.ts -t "idleTimeout: 0" — as an unpatched-vs-patched sanity check. Both tests pass there too. This is expected, not a red flag: the historical bug's actual timeout is ~252 seconds, far longer than any wait a test suite should perform, so a 20-second window cannot distinguish "genuinely disabled" (this fix) from "still broken but with a timeout longer than 20s" (the original bug). Only the literal ~252s wait, or a C++-level unit test of calculateIdleTimeoutComponents in isolation, could produce a true red-before/green-after contrast — and Bun has no unit-test harness for the vendored uWS/uSockets C++ sources today (no CMakeLists.txt/gtest target exists for packages/bun-uws or packages/bun-usockets), so that seam isn't available. Given that, this test's actual job — and what it does prove — is to catch a class of future regression (e.g. idleTimeout: 0 again being routed through the general subtraction, or the ping/close mechanism itself breaking) via the sanity comparison, not to reproduce the original forensic timing measurement. The fix itself is a 3-line, self-evidently-correct arithmetic special case, reviewed against every caller of idleTimeoutComponents in WebSocket.h/WebSocketContext.h/HttpResponse.h to confirm no other code path is affected.

• Blast-radius gates, all green:

  • bun bd test test/js/bun/http/serve.test.ts (full file, 255 tests): 0 fail, 1 todo (pre-existing, unrelated).
  • bun bd test test/js/bun/websocket/websocket-server.test.ts (full file, 110 tests): 1 fail — "should work fine if you repeatedly call methods on closed websockets" (spawns a subprocess with GuardMalloc forced on via forceGuardMalloc), exit code 137 (SIGKILL). Verified this is a pre-existing environment issue, not caused by this PR: it reproduces identically (same exit 137, ~70ms) when run directly against a debug binary built from unmodified main at the same base commit (cc0c1e8355), with none of this PR's changes present — and it passes cleanly against system Bun 1.3.14 (release build). This looks like a debug-build-vs-GuardMalloc memory-overhead interaction specific to this machine, unrelated to idle timeouts, websockets, or anything this PR touches. 3 todo, 106 pass otherwise.
  • bun bd test test/cli/inspect/ (41 tests across 4 files): 0 fail, 4 todo (pre-existing).

https://claude.ai/code/session_013DexKQwDASzoRVDFS8Jtyn

[claude]

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 13f131a1-b871-42ee-a680-3d9a0e79fb72

📥 Commits

Reviewing files that changed from the base of the PR and between 70c5bea and 5ce3376.

📒 Files selected for processing (1)

• test/js/bun/websocket/websocket-server.test.ts

---

Walkthrough

Changes

The WebSocket idle-timeout calculation now preserves idleTimeout: 0 without unsigned underflow. A testing-only binding exposes calculated components, and raw-socket regression tests verify ping/close behavior for enabled and disabled idle timeouts.

Suggested reviewers: jarred-sumner, robobun

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: fixing websocket idleTimeout: 0 so it disables timeout behavior.
Description check	✅ Passed	The description includes both required sections and provides detailed implementation and verification notes.
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.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 2

🤖 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/bun/websocket/websocket-server.test.ts`:
- Around line 1664-1665: Remove both explicit 30_000 timeout arguments from the
affected websocket server tests in
test/js/bun/websocket/websocket-server.test.ts at lines 1664-1665 and 1698-1699,
leaving each test invocation to use the repository runner’s build-aware timeout
budget.
- Around line 1549-1567: Replace the current bounded-duration WebSocket timing
assertions with a regression test that directly exercises
calculateIdleTimeoutComponents(0), or expose a deterministic timer/test hook
that makes the underflow observable without waiting for the historical timeout.
Ensure the test fails against the underflowing implementation, passes with the
fix, and awaits the actual observable condition rather than relying on
setTimeout or a fixed 20-second window; apply the same correction to the related
test blocks.

🪄 Autofix (Beta)

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: 0f1fbb17-b784-4ab1-9663-f76d13fb71b2

📥 Commits

Reviewing files that changed from the base of the PR and between be77b65 and e3dd833.

📒 Files selected for processing (2)

• packages/bun-uws/src/WebSocketContextData.h
• test/js/bun/websocket/websocket-server.test.ts

[hughescr]

Housekeeping for reviewers: I've just merged latest main (bbe3f6a262) into this branch — a clean merge (commit 489a267256, no conflicts), as a normal merge commit with no force-push, so existing review comments and their anchors are intact. Re-ran the full test/js/bun/websocket/websocket-server.test.ts after the merge: same picture as documented in the PR body — 117 pass, with the single failure being the pre-existing GuardMalloc/debug-build environment issue on my machine (reproduces identically on unmodified main; unrelated to this change — see the "blast-radius gates" section of the description).

Since I have four PRs open (#34219, #34220, #34221, #34222), a quick map so nobody has to reconstruct it: this one shares nothing with the other three — no source or test file overlap — so it can land at any time, in any order relative to them. (The other three overlap each other only in shared coverage added to test/cli/inspect/test-reporter.test.ts; details in a matching comment on each.)