http: couple fetch() receive backpressure to JS body consumption (h1/h2/h3)#29831
Open
robobun wants to merge 46 commits into
Open
http: couple fetch() receive backpressure to JS body consumption (h1/h2/h3)#29831robobun wants to merge 46 commits into
robobun wants to merge 46 commits into
Claude / Claude Code Review
completed
Apr 28, 2026 in 46m 40s
Code review found 2 important issues
Found 7 candidates, confirmed 2. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 2 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | src/http/h2_client/ClientSession.zig:411-417 |
Abandoned/cancelled h2 bodies and S3 streaming downloads stall at 16 MiB under the new streaming gate |
| 🔴 Important | src/bun.js/webcore/Body.zig:523-528 |
Response.clone() over h2 stalls at 16 MiB: tee() enables response_body_streaming without wiring drain_handler |
Annotations
Check failure on line 417 in src/http/h2_client/ClientSession.zig
claude / Claude Code Review
Abandoned/cancelled h2 bodies and S3 streaming downloads stall at 16 MiB under the new streaming gate
The new `response_body_streaming` gate assumes every path that sets that signal also drives `consumed_bytes`, but several don't: `ignoreRemainingResponseBody()` (reached via `reader.cancel()` and `Response` GC) calls `enableResponseBodyStreaming()` then nulls `drain_handler`, and `S3HttpDownloadStreamingTask` (s3/client.zig:615) sets the signal at creation but never posts `scheduleResponseBodyConsumed`. In both cases `avail = @min(0, unacked_bytes) = 0` forever, so the server stalls at the 16 Mi
Check failure on line 528 in src/bun.js/webcore/Body.zig
claude / Claude Code Review
Response.clone() over h2 stalls at 16 MiB: tee() enables response_body_streaming without wiring drain_handler
`Body.Value.tee()` (reached via `res.clone()` on a `.Locked` fetch body) calls `onStartStreaming` — flipping `response_body_streaming` true — and then constructs a `ByteStream.Source` at Body.zig:1039-1044 without setting `reader.drain_handler`, unlike the `toReadableStream` path patched here. With the new `replenishWindow` gate, `avail = @min(consumed_bytes=0, unacked_bytes) = 0` forever, so no per-stream WINDOW_UPDATE is ever sent and `res.clone()` over HTTP/2 permanently stalls once the serve
Loading