fix(build): rename AnyResponse::resume_() callers to resume() by robobun · Pull Request #36072 · oven-sh/bun

robobun · 2026-07-27T08:39:48Z

Main fails to build on all lanes since bfd1e92 (Buildkite main #83211, #83215).

Cause

#36067 renamed AnyResponse::resume_ to AnyResponse::resume in src/uws_sys/Response.rs. #36006, merged just before it, added six new callers of the old name. Both passed CI on their own branches; combined on main:

error[E0599]: no method named `resume_` found for enum `AnyResponse` in the current scope
    --> src/runtime/server/RequestContext.rs:2382:22
    --> src/runtime/server/RequestContext.rs:4126:18
    --> src/runtime/server/RequestContext.rs:4182:22
    --> src/runtime/webcore/streams.rs:1408:25
    --> src/runtime/webcore/streams.rs:1785:17
    --> src/runtime/webcore/streams.rs:1873:21

Fix

Rename the six call sites to resume().

Verification

cargo check -p bun_runtime on main: 6× E0599; with this change: clean.
bun bd builds.
bun bd test test/js/bun/http/serve.test.ts -t 'request body backpressure': 5/5 pass (the tests Bun.serve: apply TCP backpressure to a request body the handler reads slowly #36006 added that exercise these call sites).

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve.test.ts

#36067 renamed AnyResponse::resume_ to AnyResponse::resume in src/uws_sys/Response.rs. #36006, merged just before it, added six new callers of the old name. Both passed CI on their own branches; combined on main the build fails with E0599 on all lanes (builds #83211, #83215). Rename the six call sites in RequestContext.rs and streams.rs.

robobun · 2026-07-27T08:39:51Z

Build-break fix; the added test in test/js/bun/http/serve.test.ts (releases a paused request body when the handler responds without reading it) exercises the detach_response() resume path at RequestContext.rs:2382. The other five backpressure tests from #36006 exercise the remaining call sites; all six pass under bun bd.

CI: all Buildkite build lanes green (was: every lane failed on main #83211/#83215). Source lints red is pre-existing on main since bfd1e92 (node_fs.rs dead_code inventory, unrelated file; handed off separately). Self-review found no surviving concerns.

coderabbitai · 2026-07-27T08:39:59Z

Warning

Review limit reached

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

Next review available in: 21 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: 05de2ffc-27bd-4b20-b08a-f88437ddc242

📥 Commits

Reviewing files that changed from the base of the PR and between e77f5a9 and bbcf82a.

📒 Files selected for processing (3)

src/runtime/server/RequestContext.rs
src/runtime/webcore/streams.rs
test/js/bun/http/serve.test.ts

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

claude

LGTM — mechanical rename to unbreak main after the #36067/#36006 merge race.

What was reviewed:

Confirmed AnyResponse::resume(self) exists in src/uws_sys/Response.rs:829 and dispatches to the per-variant resume().
Grepped src/ for remaining resume_() callers — none left.
All six call sites go through AnyResponse (Copy), so the by-value self receiver is fine.

Extended reasoning...

Overview

Six .resume_() → .resume() renames across src/runtime/server/RequestContext.rs (3 sites) and src/runtime/webcore/streams.rs (3 sites). #36067 renamed AnyResponse::resume_ to resume; #36006 landed just before it with six new callers of the old name. Both were green on their own branches; combined on main they produce 6× E0599. This PR is the trivial reconciliation.

Security risks

None. Pure identifier rename with no semantic change; the method body is unchanged and the callers are the same expressions with a different name.

Level of scrutiny

Minimal. This is a build-break fix for a merge race — the compiler already enforces correctness here (E0599 on main, clean with the change). I verified the target method exists (src/uws_sys/Response.rs:829, pub fn resume(self) dispatching via any_dispatch!) and that no stragglers remain (grep resume_() over src/ is empty). The three streams.rs sites and three RequestContext.rs sites all obtain an AnyResponse (a Copy enum of raw handles) and call resume() on it, matching the by-value receiver.

Other factors

No new tests are expected: the fail-before is the build itself, and the behavior is already covered by the request body backpressure tests #36006 added. No prior reviewer comments to address. The change is already on main HEAD (841b99a), consistent with an urgent build fix.

…eading it Covers the detach_response() resume path at RequestContext.rs:2382 that the existing backpressure tests did not exercise.

robobun · 2026-07-27T08:58:59Z

^{Updated 3:05 AM PT - Jul 27th, 2026}

@robobun, your commit bbcf82a is building: #83229

claude

LGTM — mechanical build-break fix; the one inline nit on the new test's resolve condition is worth tightening but shouldn't block unbreaking main.

Extended reasoning...

Overview

Main is broken on all lanes since bfd1e92: #36067 renamed AnyResponse::resume_ → resume while #36006 landed six new callers of the old name. This PR renames those six call sites in src/runtime/server/RequestContext.rs and src/runtime/webcore/streams.rs, and adds one test to test/js/bun/http/serve.test.ts covering the detach_response() resume path (the one call site #36006's own tests didn't exercise).

Verified: AnyResponse::resume(self) exists at src/uws_sys/Response.rs:829 and dispatches via any_dispatch! to per-variant resume(); a repo-wide grep for resume_( finds no remaining native callers (the only hit is an unrelated JS-internal function resume_ in readable.ts). The six edits are one-token renames with no other diff on those lines.

Security risks

None. Pure identifier rename to match the already-merged definition; no new logic, no data handling, no auth/crypto surface.

Level of scrutiny

Low for the Rust changes — this is the canonical "two PRs raced a rename" build break, and cargo check / the type system prove the fix is complete. Medium for the new test, which adds ~45 lines of raw-socket harness; it reuses the existing pumpUploadUntilPlateau helper and mirrors the four sibling tests from #36006 in the same describe block.

Other factors

The bug hunter flagged one nit: the response promise resolves on buf.includes("\r\n\r\n") but then asserts toContain("ignored") (body). If headers and body arrive as separate data events the assertion could fail spuriously. In practice uWS corks a ~100-byte new Response("ignored") into one loopback write, so flake risk is very low, and the primary assertion (toStartWith("HTTP/1.1 200 ")) already proves the resume fired. It's a one-line tighten (includes("ignored") as the resolve condition, or drop line 3543) that can land here or as a follow-up — not worth leaving main broken over. Approving with the nit posted inline.

claude · 2026-07-27T09:09:18Z

+      const response = new Promise<string>((resolve, reject) => {
+        let buf = "";
+        sock.removeAllListeners("data");
+        sock.on("data", d => {
+          buf += d.toString("latin1");
+          if (buf.includes("\r\n\r\n")) resolve(buf);
+        });
+        sock.once("error", reject);
+        sock.once("close", () => resolve(buf));
+      });
+
+      gate.resolve();
+      await serverDone.promise;
+      const resp = await response;
+      expect(resp).toStartWith("HTTP/1.1 200 ");
+      expect(resp).toContain("ignored");


🟡 The response promise resolves on buf.includes("\r\n\r\n") (header terminator) but the test then asserts resp.toContain("ignored"), which is body content — if headers and body arrive in separate data events, the promise resolves with a headers-only snapshot and the body assertion fails spuriously. Resolve on buf.includes("ignored") instead (or drop the body assertion, since the HTTP/1.1 200 line already proves the resume path fired).

Extended reasoning...

What the bug is

The data handler at line 3533 resolves the response promise as soon as buf.includes("\r\n\r\n") — i.e., once the HTTP header terminator has been seen. But line 3543 then asserts expect(resp).toContain("ignored"), which is the response body. The resolve condition does not cover everything that is asserted.

The code path that triggers it

sock.on("data", d => { buf += d.toString("latin1"); if (buf.includes("\r\n\r\n")) resolve(buf); });

JS strings are immutable: buf += d rebinds buf to a new string, and resolve(buf) captures the string value at that instant. If the ~100-byte response is delivered as two data events — first the status line + headers ending in \r\n\r\n, then the 7-byte ignored body — the first event satisfies buf.includes("\r\n\r\n") and resolves the promise with the headers-only snapshot. The second event appends to the closure-local buf, but the promise is already settled (the second resolve() is a no-op), and the close fallback at line 3536 is likewise a no-op on a settled promise. resp at line 3542 is then the headers-only string, and toContain("ignored") fails.

Why existing code doesn't prevent it

There is no framing that guarantees the body is present when the header terminator is seen. The close handler cannot rescue it because the promise has already resolved. Neighboring raw-socket tests in this file that assert on the body accumulate until close (e.g., toEndWith("\r\n\r\nhey") after the socket closes) rather than resolving on the header terminator.

Impact

REVIEW.md is explicit under Tests reviewers reject: "Buffer raw socket/stdout chunks to the protocol's framing before asserting." This is exactly the pattern that rule targets. In practice, uWS corks new Response("ignored") into a single ~100-byte write and loopback almost always delivers it in one segment, so the flake probability is very low — but it is non-zero across platforms/ASAN/debug scheduling, and it violates a rule that has blocked merges.

Step-by-step proof

Server sends HTTP/1.1 200 OK\r\nContent-Type: text/plain;charset=utf-8\r\nContent-Length: 7\r\n\r\nignored in one res.end().

Kernel delivers first data event containing everything up through ...\r\n\r\n (headers only). buf = headers; buf.includes("\r\n\r\n") → true → resolve(buf) with headers only.

Kernel delivers second data event with "ignored". buf += "ignored" rebinds the local; resolve(buf) on a settled promise is a no-op.

const resp = await response → headers-only string.

expect(resp).toStartWith("HTTP/1.1 200 ") passes; expect(resp).toContain("ignored") fails.

Fix

Change the resolve condition to match what is asserted:

if (buf.includes("ignored")) resolve(buf);

Or drop line 3543 entirely — the HTTP/1.1 200 status line already proves detach_response()'s resume fired (without it the client would never see any response), so the body assertion adds no coverage.

Main fails to build on all lanes since bfd1e92 (Buildkite main #83211, #83215). ## Cause #36067 renamed `AnyResponse::resume_` to `AnyResponse::resume` in `src/uws_sys/Response.rs`. #36006, merged just before it, added six new callers of the old name. Both passed CI on their own branches; combined on main: ``` error[E0599]: no method named `resume_` found for enum `AnyResponse` in the current scope --> src/runtime/server/RequestContext.rs:2382:22 --> src/runtime/server/RequestContext.rs:4126:18 --> src/runtime/server/RequestContext.rs:4182:22 --> src/runtime/webcore/streams.rs:1408:25 --> src/runtime/webcore/streams.rs:1785:17 --> src/runtime/webcore/streams.rs:1873:21 ``` ## Fix Rename the six call sites to `resume()`. ## Verification - `cargo check -p bun_runtime` on main: 6× E0599; with this change: clean. - `bun bd` builds. - `bun bd test test/js/bun/http/serve.test.ts -t 'request body backpressure'`: 5/5 pass (the tests #36006 added that exercise these call sites).  --- **no test proof** · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/bun/http/serve.test.ts

robobun · 2026-07-27T10:39:33Z

Heads-up: the releases a paused request body when the handler responds without reading it test this PR added is red on darwin (main #83238, this PR's #83229 as a retry-flake). With Connection: close the server's close() RSTs the unread body bytes on macOS, which EPIPEs the still-armed upload pump into sock.once('error', reject) and can drop the response. Fixed in #36088 by switching that test to keep-alive.

…kpressure test (#36088) `serve.test.ts` has been red on the darwin lanes since #36072 landed (main #83238, also the PR's own build #83229 as a retry-flake). The subtest is `request body backpressure > releases a paused request body when the handler responds without reading it`, which #36072 added to cover the `detach_response()` resume at `RequestContext.rs:2382`. ``` EPIPE: broken pipe, write syscall: "write", errno: -32, code: "EPIPE" ✗ request body backpressure > releases a paused request body when the handler responds without reading it ``` ## Cause The test sends a 32 MiB `PUT` over a raw `net.Socket` with `Connection: close`, lets the server's pre-stream pause stall the upload, then has the handler respond without touching `req.body` and waits for the client to receive the response. Once the handler returns, `render_bytes()` writes the response via `try_end()` and uWS does `shutdown(SHUT_WR)` immediately followed by `close()` (`HttpResponse.h:860-871`). Because the socket was paused, the unread body bytes are still sitting in the kernel recv buffer, and `close()` with unread recv data sends **RST**. On macOS that RST races two things: 1. the upload pump's `sock.once('drain', writeMore)`, which is still armed from `pumpUploadUntilPlateau`; when it fires and writes into the reset peer the socket emits EPIPE, which the test's `sock.once('error', reject)` turns into a failure, and 2. delivery of the already-written response bytes, which the RST can drop before the client's `'data'` handler sees them. On Linux the `'data'` event reliably wins the race, so the test passed there; on darwin it loses ~70-80% of the time (verified on `darwin-test-arm64-3` and `darwin-test-x64-2` with the main #83238 binary). `detach_response()`'s `resp.resume()` is also a no-op on this path: for a simple text body `try_end()` has already closed the socket by the time it runs (`us_socket_resume` early-returns on a closed socket). So with `Connection: close` the test was not actually observing the code it was written to cover. ## Fix Switch this one test to `Connection: keep-alive`. The server does not close after responding, so there is no RST, the response is delivered deterministically, and `detach_response()`'s resume runs on a live socket. The test now additionally asserts that the client's upload progresses past the plateau after the response, which is the direct observable of the resume (without it the keep-alive socket would stay paused and `getSent()` would never move). Also: resolve the response promise on the body content rather than the header terminator, and drop the `sock.once('error', reject)`; neither race applies on a keep-alive connection but both were latent flake on the old path. The `pumpUploadUntilPlateau` helper gains a `connection` parameter (defaulting to `"close"`, so the other five callers are untouched) and returns a `getSent()` accessor. ## Verification With the main #83238 `bun` binary: | lane | before | after | | --- | --- | --- | | darwin 26 aarch64 (`darwin-test-arm64-3`) | 7/10 fail (EPIPE) | 50/50 pass | | darwin 14 x64 (`darwin-test-x64-2`) | 8/10 fail (EPIPE) | 20/20 pass | | linux x64 debug+ASAN | 10/10 pass | 10/10 pass | `USE_SYSTEM_BUN=1 bun test test/js/bun/http/serve.test.ts -t 'releases a paused'` still fails on `toBeLessThan(TOTAL)` (no #36006 backpressure), so the test still exercises the same `src/` change. This is a test-only diff; fail-before is against a Bun without #36006, not against this PR's own `src/`.  --- **no test proof** · iteration 0 · Platform-specific test-only change; deferring to CI.

github-actions Bot added the claude label Jul 27, 2026

claude Bot reviewed Jul 27, 2026

View reviewed changes

test: paused request body is released when handler responds without r…

bbcf82a

…eading it Covers the detach_response() resume path at RequestContext.rs:2382 that the existing backpressure tests did not exercise.

This was referenced Jul 27, 2026

source-lints: add node_fs.rs to dead-code escape inventory #36073

Merged

node:http: skip TCP_DEFER_ACCEPT so server.setTimeout fires on time for silent clients #36074

Open

claude Bot reviewed Jul 27, 2026

View reviewed changes

Jarred-Sumner merged commit 29c3914 into main Jul 27, 2026
30 of 39 checks passed

Jarred-Sumner deleted the farm/66d86ee2/fix-resume-rename branch July 27, 2026 09:09

robobun mentioned this pull request Jul 27, 2026

test(serve): fix darwin flake in the respond-without-reading-body backpressure test #36088

Merged

This was referenced Jul 27, 2026

node:crypto: reject RSA modulusLength that BoringSSL would round down #36096

Open

node:async_hooks: align AsyncLocalStorage.bind()/snapshot() shape and run()/exit() thisArg with Node #36114

Open

Navigation Menu

Uh oh!

fix(build): rename AnyResponse::resume_() callers to resume() - #36072

fix(build): rename AnyResponse::resume_() callers to resume()#36072
Jarred-Sumner merged 2 commits into
mainfrom
farm/66d86ee2/fix-resume-rename

robobun commented Jul 27, 2026 •

edited

Loading

Uh oh!

robobun commented Jul 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

Review limit reached

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jul 27, 2026 •

edited

Loading

Uh oh!

claude Bot left a comment

Uh oh!

claude Bot Jul 27, 2026

Uh oh!

Uh oh!

robobun commented Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

robobun commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Cause

Fix

Verification

Uh oh!

robobun commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

robobun commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

claude Bot Jul 27, 2026

Choose a reason for hiding this comment

What the bug is

The code path that triggers it

Why existing code doesn't prevent it

Impact

Step-by-step proof

Fix

Uh oh!

Uh oh!

robobun commented Jul 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Jul 27, 2026 •

edited

Loading

robobun commented Jul 27, 2026 •

edited

Loading

coderabbitai Bot commented Jul 27, 2026 •

edited

Loading

robobun commented Jul 27, 2026 •

edited

Loading