Bun.serve: route Response body stream errors to error() until the first byte is written by robobun · Pull Request #35229 · oven-sh/bun

robobun · 2026-07-23T04:31:04Z

What does this PR do?

When a Response body is driven by an async iterable (async generator or [Symbol.asyncIterator] object) and that iterable throws, Bun.serve produced three inconsistent client-visible outcomes and never invoked the server's error() callback:

Bun.serve({
  port: 3000,
  fetch(req) {
    async function* first() { throw new Error('boom'); }
    async function* fast() { yield 'AAAA'; yield 'BBBB'; throw new Error('boom'); }
    return new Response((new URL(req.url).pathname === '/fast' ? fast() : first()));
  },
  error(e) { return new Response('E:' + e.message, { status: 500 }); }, // never called
});
// curl -i localhost:3000/     -> HTTP/1.1 200 OK + Transfer-Encoding: chunked + 0\r\n\r\n
// curl -i localhost:3000/fast -> curl: (56) Recv failure: Connection reset by peer

throw before the first yield: a complete 200 OK + empty chunked body (0\r\n\r\n), cacheable as a successful response even though the failure happened before any byte was committed
synchronous yields then throw: connection reset with zero bytes; the already-yielded chunks and the status line were discarded
awaited yields then throw: the chunked body was left unterminated (the only outcome a client could tell apart from success)

The same applies to a default ReadableStream whose pull rejects.

Cause

do_render_stream wrote the user's status line and headers (render_metadata) before assign_to_stream started the body, so has_written_status() was already true by the time the body failed and the error() path was skipped. The sync-yield-then-throw case additionally reached force_close() while the status and yielded chunks were still in the uWS cork buffer, which discarded them.

Fix

do_render_stream no longer writes the status line up front. The existing on_first_write hook on the sink fires render_metadata just before the first body byte reaches uWS; end_from_js's empty-body path now fires it too so an empty stream still sends the user's status/headers.
handle_reject_stream routes to handle_reject (the user's error() handler, or the default 500) when has_written_status() is still false; when true, it reports the failure and force-closes without the terminating chunk, uncorking first so bytes written inside the cork reach the socket.
run_error_handler_with_status_code_dont_check_responded now protects error()'s Response the same way process_on_error_promise and the normal fetch() path already do, so the deferred render_metadata can still read it.

After:

case	before	after
throw before first yield	200 OK, empty chunked body, `error()` not called	`error()` called; its Response is sent
await then throw (no yield)	200 OK, empty chunked body, `error()` not called	`error()` called; its Response is sent
sync yields then throw	RST, 0 bytes, `error()` not called	200 + yielded chunks, unterminated; reported to stderr
awaited yields then throw	200 + chunks, unterminated	unchanged; now also reported to stderr

How did you verify your code works?

New fixture serve-body-error-before-first-byte-fixture.ts covers each async-iterable case plus a default ReadableStream pull rejection, observed over a raw socket so the exact wire framing is asserted; the existing stream-body-error tests are updated to expect error() for pre-first-byte failures.

All tests in serve-stream-body-error.test.ts, serve-direct-readable-stream.test.ts, async-iterator-stream.test.ts, serve-error-handler-stream.test.ts, serve-stream-reject-flush-leak.test.ts, serve-http3.test.ts, and the streaming section of serve.test.ts pass; the new tests fail on the system bun and pass with this change.

Note: this overlaps with #33660, which wants the header block flushed immediately when a stream body goes pending (for SSE onopen). That change commits the status before the first body byte, which would close the window in which error() can replace the response for a stream that fails after going async but before yielding. Whichever lands second will need to reconcile the Pending branch.

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

…st byte is written When a Response body is an async iterable (or ReadableStream) that errors, Bun.serve previously committed the user's status line before the body was started, so error() could never replace the response. The three observable outcomes were all wrong or inconsistent: - throw before the first yield: a complete 200 OK with an empty chunked body (0\r\n\r\n), indistinguishable from a successful empty response - synchronous yields then throw: the connection was reset with zero bytes; the already-yielded chunks and the status line were discarded - awaited yields then throw: the chunked body was truncated (the only case a client could tell apart from success) and error() was never invoked in any of them. do_render_stream now defers render_metadata() to on_first_write, which the sink fires just before the first body byte reaches uWS (end_from_js' empty body path now fires it too). handle_reject_stream routes to handle_reject (the user's error() handler, or the default 500) when has_written_status is still false; otherwise it reports the failure and force-closes without the terminating chunk, uncorking first so body bytes written inside the cork are not discarded. run_error_handler_with_status_code_dont_check_responded now protects error()'s Response the same way process_on_error_promise and the normal fetch() path already do, so the deferred render_metadata can still read it. Existing tests that locked in the previous behaviour are updated to assert error() is invoked for pre-first-byte failures.

robobun · 2026-07-23T04:31:15Z

^{Updated 4:07 AM PT - Jul 23rd, 2026}

✅ @robobun, your commit 8ffb18562d7713bd01ce33782e8f8d4e7c94fd38 passed in Build #78497! 🎉

🧪 To try this PR locally:

bunx bun-pr 35229

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

bun-35229 --bun

coderabbitai · 2026-07-23T04:39:24Z

Warning

Review limit reached

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

Next review available in: 10 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: b74a00ae-424f-40c9-aa66-00ba04166715

📥 Commits

Reviewing files that changed from the base of the PR and between 2cc60f9 and 8ffb185.

📒 Files selected for processing (5)

src/runtime/server/RequestContext.rs
src/runtime/webcore/streams.rs
test/js/bun/http/serve-body-error-before-first-byte-fixture.ts
test/js/bun/http/serve-stream-body-error.test.ts
test/js/bun/http/serve-stream-reject-flush-leak.test.ts

Walkthrough

Streaming response handling now delays status commitment until the first body output, routes pre-byte failures through error(), flushes committed responses before closure, and updates JavaScript response protection. Tests cover async iterables, readable streams, empty bodies, wire output, and process behavior.

Changes

Streaming rejection handling

Layer / File(s)	Summary
Lazy status commitment and empty-stream completion `src/runtime/server/RequestContext.rs`, `src/runtime/webcore/streams.rs`	Status and metadata rendering remain deferred until the first write or empty response completion.
Pre-commitment and committed rejection paths `src/runtime/server/RequestContext.rs`	Pre-byte failures invoke rejection handling, while committed responses run the error handler and uncork buffered data before closing.
Error-handler response rooting `src/runtime/server/RequestContext.rs`	Error-handler responses replace the stored JavaScript value and are protected according to their body type before rendering.
Streaming error coverage `test/js/bun/http/*`	Tests and fixtures validate replacement responses, default errors, post-byte failures, empty bodies, framing, stderr, and server liveness.

Possibly related PRs

oven-sh/bun#32746: Related streaming termination and status/write-boundary cleanup in RequestContext.rs.
oven-sh/bun#33810: Related rejection handling after the HTTP status line is written.
oven-sh/bun#34610: Related HTTPServerWritable.end_from_js first-byte and response-ending behavior.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly summarizes the main change: routing stream body errors to error() until the first byte is written.
Description check	✅ Passed	The description includes both required sections and gives a detailed change summary plus verification notes.

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

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/runtime/server/RequestContext.rs (1)
936-947: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting the duplicated committed-status failure path into a shared helper. Both handle_reject and handle_reject_stream encode the identical "status already committed → optionally run the VM error() handler, then uncork() + force_close() when http_write_called && response_pending, else end_stream()" contract. These must remain byte-for-byte in sync (they already differ slightly in guard/borrow shape), so a small helper taking the rejection value would reduce the divergence risk.

src/runtime/server/RequestContext.rs#L936-L947: replace the inline report-and-close block with the shared helper.

src/runtime/server/RequestContext.rs#L3009-L3033: replace the inline report-and-close block with the same helper.
🤖 Prompt for 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.

In `@src/runtime/server/RequestContext.rs` around lines 936 - 947, Extract the
duplicated committed-status rejection handling into a shared helper that accepts
the rejection value and preserves the existing VM error-handler,
uncork/force-close, and end_stream behavior. Replace the inline blocks in
src/runtime/server/RequestContext.rs:936-947 and
src/runtime/server/RequestContext.rs:3009-3033 with calls to this helper,
keeping both paths behaviorally and byte-for-byte consistent.

🤖 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.

Outside diff comments:
In `@src/runtime/server/RequestContext.rs`:
- Around line 936-947: Extract the duplicated committed-status rejection
handling into a shared helper that accepts the rejection value and preserves the
existing VM error-handler, uncork/force-close, and end_stream behavior. Replace
the inline blocks in src/runtime/server/RequestContext.rs:936-947 and
src/runtime/server/RequestContext.rs:3009-3033 with calls to this helper,
keeping both paths behaviorally and byte-for-byte consistent.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 89b4d1ac-6f83-4561-a41d-f2da08385ce9

📥 Commits

Reviewing files that changed from the base of the PR and between e383be4 and 2cc60f9.

📒 Files selected for processing (9)

src/runtime/server/RequestContext.rs
src/runtime/webcore/streams.rs
test/js/bun/http/async-iterator-stream.test.ts
test/js/bun/http/async-iterator-throws.fixture.js
test/js/bun/http/serve-body-error-before-first-byte-fixture.ts
test/js/bun/http/serve-direct-readable-stream.test.ts
test/js/bun/http/serve-stream-body-error.test.ts
test/js/bun/http/serve-stream-reject-flush-leak.test.ts
test/js/bun/http/serve.test.ts

…ommitted_status()

claude

Additional findings (outside current diff — PR may have been updated during review):

🔴 src/runtime/server/RequestContext.rs:2147-2157 — Removing the eager render_metadata() from the Pending branch regresses the direct-stream sibling of the empty-body case: an async pull() that resolves without writing or calling controller.end() reaches handle_resolve_stream → sink.finalize() with on_first_write still armed, and finalize()'s !done branch calls res.end_stream(false) — which triggers uWS sendTerminatingChunk() → writeStatus(HTTP_200_OK) — before the user's status/headers are ever written; handle_resolve_stream's subsequent render_metadata() then appends the user's headers after the 0\r\n\r\n terminator. The fix in end_from_js's empty-body branch (handle_first_write_if_necessary() before res.end()) needs mirroring in finalize()'s !done branch before res.end_stream(false) (or fired in handle_resolve_stream before finalize()). The new iter-empty-ok/rs-empty-ok tests cover the async-iterable and default-ReadableStream siblings — both route through end_from_js — but not the direct-stream sibling.
Extended reasoning...

What the bug is

This PR defers writing the user's status line/headers from do_render_stream to the sink's on_first_write hook, so a stream that fails before its first byte can be routed to error(). To keep an empty successful body from losing the user's status, the PR adds self.handle_first_write_if_necessary() to end_from_js()'s empty-body branch in streams.rs before res.end(b"", false).

But there is a sibling site that ends the response with an empty body and was not patched: HTTPServerWritable::finalize()'s !done branch, which handle_resolve_stream reaches whenever the pump promise resolves with sink.done still false. That branch calls flush_no_wait() (which returns 0 on an empty buffer without firing on_first_write) and then res.end_stream(false), which drives uWS's sendTerminatingChunk() → writeStatus(HTTP_200_OK) before Bun's render_metadata() ever runs.

Concrete trigger
```
Bun.serve({
  fetch() {
    return new Response(
      new ReadableStream({
        type: 'direct',
        async pull(c) { await Bun.sleep(5); /* no write, no end */ },
      }),
      { status: 202, headers: { 'x-custom': 'yes' } },
    );
  },
});
```
Step-by-step trace
1. readDirectStream (BunStreamSource.cpp:939–942): pull() returns a Promise, so it's wrapped via onReturnUndefined into a result promise and returned to assign_to_stream.
2. do_render_stream: promise.unwrap() → Pending. Before this PR, this branch did on_first_write = None; render_metadata() (the block removed at diff lines 2141–2145), writing 202 + x-custom immediately. After this PR, on_first_write stays armed and nothing is written.
3. pull() resolves → onReturnUndefined resolves the result promise → ON_RESOLVE_STREAM → handle_resolve_stream.
4. Line 2807–2840: sink.pending_flush is None (nothing was written) → falls through.
5. Line 2845–2852: sink.done == false, wrote == 0, ended_response == false → wrapper.sink.finalize().
6. finalize() !done branch (streams.rs): flush_no_wait() sees readable_slice().len() == 0 and returns 0 — it does not call send_readable, so handle_first_write_if_necessary() never fires. Then res.end_stream(false) → uws_res_end_stream → sendTerminatingChunk() → writeStatus(HTTP_200_OK). uWS's HTTP_STATUS_CALLED was not yet set, so it writes HTTP/1.1 200 OK\r\n + Date + Transfer-Encoding: chunked\r\n\r\n0\r\n\r\n and sets HTTP_STATUS_CALLED + HTTP_WRITE_CALLED.
7. Back in handle_resolve_stream, line 2891: ended_response is false → skip. Line 2895: !req.flags.has_written_status() is true — Bun's own flag is only set by do_write_status, which never ran — so req.render_metadata() runs.
8. do_write_status(202) → resp.write_status("202 Accepted") → uWS writeStatus sees HTTP_STATUS_CALLED (set in step 6) and no-ops. The user's 202 is silently dropped.
9. do_write_headers() → uWS writeHeader("x-custom", "yes") has no guard on HTTP_WRITE_CALLED and appends x-custom: yes\r\n via AsyncSocket::write — after the 0\r\n\r\n body terminator.
Wire before the PR: HTTP/1.1 202 Accepted\r\nx-custom: yes\r\n…\r\n\r\n0\r\n\r\n
Wire after the PR: HTTP/1.1 200 OK\r\n…Transfer-Encoding: chunked\r\n\r\n0\r\n\r\nx-custom: yes\r\n… — the user's status is silently replaced with 200 and their headers are spliced after the body terminator (garbage before the next keep-alive response).

Why existing code doesn't prevent it

The PR patched end_from_js()'s empty-body branch (handle_first_write_if_necessary() before res.end(b"", false)). Async iterables and default ReadableStreams both route through readStreamIntoSink, which calls sink.end() → end_from_js, so the new iter-empty-ok/rs-empty-ok tests pass. Direct streams route through readDirectStream; when async pull() resolves without controller.end(), end_from_js is never called and the parallel res.end_stream(false) in finalize() is reached instead. The existing direct-stream test "sync pull() that writes nothing and ends later still responds" uses a sync pull, which takes the closePromise path (BunStreamSource.cpp:944–947) and only resolves after end() sets sink.done = true, so finalize()'s !done branch is never entered there.

Fix

Mirror the end_from_js fix in finalize()'s !done branch: call self.handle_first_write_if_necessary() before res.end_stream(false) (or fire it in handle_resolve_stream before wrapper.sink.finalize() when !req.flags.has_written_status()). And add the direct-stream variant to the *-empty-ok test matrix, e.g.:
```
if (p === '/direct-empty-ok') {
  return new Response(
    new ReadableStream({ type: 'direct', async pull(c) { await Bun.sleep(1); } }),
    { status: 202, headers: { 'x-custom': 'yes' } },
  );
}
```
Per REVIEW.md — "fix the whole class in the same PR: grep for every sibling site sharing the pattern" and "cover the variant matrix, not just the repro" — this is the missed sibling of the exact fix pattern the PR introduces.

…e previous Response before error() Promise branch, keep error()'s own status when its body stream fails Also makes the iter-yield-then-throw assertion tolerant of Windows' SO_LINGER{1,0} reset discarding the uncorked bytes (the behavior there is unchanged from before this PR), adds direct-empty-ok and async-error-handler coverage, and tightens the two stderr filters per review.

…ver reached the client, so error() replaces the response An async fetch() that goes Pending past the initial drain reaches do_render_stream with is_async()=true, so its drain_microtasks is a no-op: sub-highWaterMark yields stay in the sink buffer, and on_reject_stream can fire in the same microtask drain as the yields, before the deferred auto-flusher runs. The buffered bytes are discarded either way; routing to error() is what the boundary ('until the first byte is written') means.

claude

No new issues found on this pass — the three earlier rounds of findings (async error() unprotect, finalize() empty-body path, Winsock RST assertions, and the !buffer.is_empty() disjunct) have all been addressed. Deferring to a human because this is a user-visible behavior change to Bun.serve's error contract in memory-sensitive native code, and the description notes an unresolved overlap with #33660 that whichever lands second must reconcile.

What was reviewed:

handle_reject_stream re-routing to handle_reject when !has_written_status(), including the has_marked_pending reset and the has_called_error_handler() guard so error()'s own failing stream keeps its status.
The unprotect hoisted before the as_any_promise/as_response split, and the new protect for error()'s Response in the sync arm — checked both Fulfilled and Pending arms of process_on_error_promise no longer see a stale response_protected.
The deferred render_metadata via on_first_write: the two new handle_first_write_if_necessary() calls in end_from_js and finalize() cover the empty-body paths; the removed eager write in the Pending arm keeps the sink's ctx pointer alive via the existing ref_().
The removed DEBUG_MODE dev-server fallback-page block in handle_reject_stream — its reporting is now covered by handle_reject/terminate_body_after_committed_status, but the HTML fallback rendering for a rejecting stream under bun dev is dropped; worth a human confirming that's intended.

Extended reasoning...

Overview

This PR changes Bun.serve's handling of Response bodies backed by streams/async-iterables that error: instead of eagerly writing the user's status line before the body starts (which committed a 200 even when the body immediately failed), the status write is deferred to the sink's on_first_write hook. A stream that rejects before any body byte reaches uWS is now routed through handle_reject → the user's error() handler; one that rejects after is reported to stderr and force-closed without the terminating chunk. Touches src/runtime/server/RequestContext.rs (~140 net lines: do_render_stream, handle_reject_stream, handle_reject, run_error_handler_with_status_code_dont_check_responded, new terminate_body_after_committed_status), src/runtime/webcore/streams.rs (two handle_first_write_if_necessary() calls in empty-body end paths), and seven test files including inverted assertions in serve.test.ts and serve-direct-readable-stream.test.ts.

Security risks

None identified. The change does not touch auth, TLS, or input parsing. The force_close path already existed; the new uncork() before it is defensive. The GC-protection changes were reviewed for balance across all branches (sync Response, async Fulfilled, async Pending) and look correct after the hoist in 37392c4.

Level of scrutiny

High. This is production-critical HTTP server code with hand-managed uWS response lifetimes, GC-root protect/unprotect pairing, and a raw ctx back-pointer from the sink into RequestContext that is now held longer (across the Pending branch instead of being disarmed there). It is also a user-visible behavior change: existing tests that asserted "throw on pull renders headers, does not call error handler" now assert the opposite. That's the right behavior, but it changes what users observe.

Other factors

Three prior automated review rounds each found real issues (per-request GC-root leak on async error(), missing on_first_write in finalize(), Windows-only test failure, and the !buffer.is_empty() disjunct that defeated the fix under async fetch()); all four were addressed with commits and matching test coverage.
The PR description explicitly flags overlap with #33660 (SSE header flushing on Pending), which wants the opposite timing for the status write. A maintainer should decide the reconciliation strategy before either lands.
The removed DEBUG_MODE block rendered a dev-server fallback HTML page for stream rejections; the reporting half of it is subsumed by the new paths, but the fallback page rendering is not, and the description doesn't mention it.
Test coverage for the new behavior is thorough (raw-socket wire assertions across sync/async fetch, sync/async error(), no-error-handler, empty-body direct/default streams, and the yield-then-throw platform-variant case).

robobun · 2026-07-23T07:02:08Z

On the removed DEBUG_MODE dev-server fallback block in handle_reject_stream: when a stream body rejects before any byte reaches uWS, it now routes through handle_reject → run_error_handler, which (with no user error()) reaches finish_running_error_handler's DEBUG_MODE branch and calls render_default_error. So a dev error page is still rendered; the difference is it is the generic render_default_error page rather than the bake-specific Fallback::render_backend one. When the status is already committed the old block would have spliced the fallback HTML into the in-flight chunked body, which the new path avoids by force-closing instead.

robobun · 2026-07-23T08:07:46Z

CI on 8ffb185 (build 78497): the six red tests are all flaky-on-retry and unrelated to this diff (webview-chrome animation timing, bun-install-registry hoisting, bun-upgrade ETXTBSY, napi string snapshot, node test-fs-read-stream-pos, serve-protocols HTTP/3 echo). serve-stream-body-error.test.ts and the other files this PR touches are green on every lane including both Windows targets. Ready for review.

robobun · 2026-08-27T07:05:36Z

Closing in favor of #40596. The symptom this PR fixes, a complete 200 OK with an empty chunked body for a body that failed before its first byte, is handled there by closing the connection without the terminating chunk. I ran the repro from the description against the #40596 build: / (throw before the first yield) gets an empty reply, /fast (synchronous yields, then throw) gets an empty reply, an awaited throw before the first yield gets the headers and then a reset, and every case exits curl with 56. No case is a complete message any more, and the server stays up.

What this PR adds beyond that, deferring the status line so that error() can answer such a failure with a 500, is a design change to do_render_stream and the sink that also conflicts with #33660 (flush the headers before the first chunk). It is still a possible follow-up, and it can be proposed on top of #40596, which does not decide when the status line is committed.

robobun · 2026-08-27T12:49:28Z

Correction: the framing change that handles the complete-200 symptom moved from #40596 to #40648 (stacked on #40596). #40648 names the design here (defer the status line so error() can answer with a 500) as the alternative for the maintainer to weigh.

…#40596) ### Problem - A `Bun.serve` Response whose body stream fails before any body byte is written is delivered to the client as a complete message: the Response's status and headers, `Transfer-Encoding: chunked`, an empty body and a clean `0\r\n\r\n`. `curl` exits 0. A proxied upstream body that dies before its first chunk is forwarded the same way, and nothing is reported. Only a failure after the first body byte closed the connection without the terminator (#32842). - Cause: three sites in `src/runtime/server/RequestContext.rs` handle a body failure after the status line is committed (`handle_reject` for a direct stream's synchronous `pull()` throw, `handle_reject_stream` for a JS stream, `end_chunk` for a native byte stream). Each force-closed only when `is_http_write_called()` and otherwise called `end_stream()`, which writes the terminating chunk. ### Fix - The three sites share a new `close_incomplete_stream`: while the response is still pending, `force_close()` the connection, whether or not body bytes went out first. Once the sink has already ended the response, `end_stream()` only releases the context, as before. - Correct because a chunked message is complete only with its terminator (RFC 9112 section 7). An empty chunked body with a terminator is as complete as a truncated one. Closing without it is the only way HTTP/1.1 can signal an incomplete message. When the status is still in the cork buffer, the close discards it and the client sees an empty reply, the same as Node's `res.destroy()` before the headers flush. - `end_chunk` (HTMLRewriter output, a proxied `fetch()` body) no longer drops the producer's error: it reports it in both modes through the shared `report_committed_body_error`, which also gives native bodies the bake dev server error page that JS streams already had. This carries the remaining parts of #39442, closed in favor of this PR. - Verified: `test/js/bun/http/serve-stream-body-error.test.ts` (JS stream variants, pending error after the headers, HTMLRewriter and proxied fetch bodies in both modes, two dev server rows; 17 of 26 fail on stock bun). Also `serve.test.ts`, `serve-direct-readable-stream.test.ts`, `async-iterator-stream.test.ts`, `serve-http3.test.ts`, `serve-error-handler-stream.test.ts`, `serve-stream-reject-flush-leak.test.ts`, `text-encoder-stream.test.ts`, `html-rewriter.test.js`. ### Background - `RequestContext` is the per-request state of `Bun.serve`. `do_render_stream` writes the Response's status and headers into the corked uWS response before it attaches the body stream, so by the time a body error arrives `has_written_status()` is already true and `error()` cannot supply a replacement. That contract is unchanged here: `error()` is still not called once the status is committed. - uWS corks a socket while a request handler runs: writes go to a per-socket buffer that is flushed when the handler returns. `force_close()` closes the socket directly and drops that buffer, so a synchronous failure leaves nothing on the wire. An asynchronous failure arrives after the flush, so the client gets the headers and then a reset. - Behavior change: the tests that pinned the old contract ("throw on pull renders headers", "async generator ... continues to send the headers", the pre-first-byte variants in `serve-stream-body-error.test.ts`) now expect the connection to close without a complete response. <details><summary>Notes</summary> Wire before the fix, for `new Response(new ReadableStream({ start(c) { c.enqueue(chunk); c.error(new Error("boom")); } }))`: ``` HTTP/1.1 200 OK Content-Type: text/plain;charset=utf-8 Date: ... Transfer-Encoding: chunked 0 ``` After: the connection closes with 0 bytes sent (the status was still corked). For a stream whose first `pull()` is asynchronous, the headers were already flushed, so the client gets `HTTP/1.1 200 OK` plus headers and then a connection reset with no terminating chunk. Both are incomplete messages. The error reaches stderr through the existing reporters; the production-mode asynchronous JS path stays quiet, as today. The forced close sends a RST, and a RST discards data the peer has not read yet (always on Windows). The tests that expect the status line on the wire before the failure therefore trigger the failure from the client's data handler, once the status line has arrived. uWS terminates the header block only with the first body byte, so those tests wait for the status line, not for a blank line. Dev server exception: under a bake dev server (`development: true` plus an HTML route), a body that fails after the status is committed gets the dev error page appended and the response ends normally, so the browser shows the error after what was already streamed. JS streams always did this; native bodies now take the same branch. Probed sources, all now closed incomplete: `controller.error` in `start`, synchronous and asynchronous `pull()` throw, async generator throw, `Readable.toWeb` of an erroring node stream, a throwing `TransformStream`, `DecompressionStream` on bad bytes, a `type: "direct"` stream that throws, an HTMLRewriter handler that throws or rejects after the first byte, a proxied upstream fetch body reset before and after its first chunk. `Bun.file()` on a missing path already reaches `error()` and answers 500 (unchanged). A user `Content-Length` header on a stream Response is ignored in favor of chunked framing, so it does not change the outcome. Related PRs in this area: #35229 deferred the status line until the first body byte so that `error()` can answer a pre-first-byte failure with a 500 (a larger change to `do_render_stream` and the sink), closed in favor of this PR; that design remains a possible follow-up. #38003 touches the same sites for the stored-ByteStream-error case. This PR only enforces the framing invariant; it does not decide whether `error()` should be reachable for these failures. HTTP/3 is not fixed by this change: `uws_h3_res_force_close` closes the QUIC stream with a FIN, which is a complete message in HTTP/3. That is tracked separately in #40598. </details>  --- **no test proof** · iteration 5 · 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 assigned Jarred-Sumner Jul 23, 2026

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

[autofix.ci] apply automated fixes

afd3be5

coderabbitai Bot reviewed Jul 23, 2026

View reviewed changes

extract the committed-status failure path into terminate_body_after_c…

8956468

…ommitted_status()

claude Bot reviewed Jul 23, 2026

View reviewed changes

Comment thread src/runtime/server/RequestContext.rs Outdated

Comment thread test/js/bun/http/serve-stream-reject-flush-leak.test.ts Outdated

Comment thread test/js/bun/http/serve-stream-body-error.test.ts Outdated

robobun and others added 2 commits July 23, 2026 05:17

[autofix.ci] apply automated fixes

312bc9b

claude Bot reviewed Jul 23, 2026

View reviewed changes

Comment thread src/runtime/server/RequestContext.rs

Comment thread test/js/bun/http/serve-stream-body-error.test.ts

ci: retrigger

14019e7

claude Bot reviewed Jul 23, 2026

View reviewed changes

Comment thread src/runtime/server/RequestContext.rs Outdated

claude Bot reviewed Jul 23, 2026

View reviewed changes

This was referenced Jul 23, 2026

test(serve): run serve-error-handler-stream cases in one subprocess #35262

Open

Bun.serve: honour handler-set Content-Length for ReadableStream bodies #35682

Closed

robobun mentioned this pull request Aug 18, 2026

Bun.serve: report a native body error that arrives after the status was sent #39442

Closed

robobun mentioned this pull request Aug 27, 2026

Bun.serve: never terminate a failed body stream as a complete message #40596

Merged

robobun closed this Aug 27, 2026

robobun mentioned this pull request Aug 27, 2026

Bun.serve: close the connection when a body stream fails before its first chunk is written #40648

Closed

Navigation Menu

Bun.serve: route Response body stream errors to error() until the first byte is written - #35229

Bun.serve: route Response body stream errors to error() until the first byte is written#35229
robobun wants to merge 7 commits into
mainfrom
farm/9aa8e219/serve-async-iterable-body-error

robobun commented Jul 23, 2026 •

edited

Loading

Uh oh!

robobun commented Jul 23, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading

Review limit reached

Uh oh!

coderabbitai Bot left a comment

Uh oh!

claude Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot left a comment

Uh oh!

robobun commented Jul 23, 2026

Uh oh!

robobun commented Jul 23, 2026

Uh oh!

robobun commented Aug 27, 2026

Uh oh!

robobun commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

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

Uh oh!

What does this PR do?

Cause

Fix

How did you verify your code works?

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Review limit reached

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

What the bug is

Concrete trigger

Step-by-step trace

Why existing code doesn't prevent it

Fix

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

robobun commented Jul 23, 2026

Uh oh!

robobun commented Jul 23, 2026

Uh oh!

robobun commented Aug 27, 2026

Uh oh!

robobun commented Aug 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Jul 23, 2026 •

edited

Loading

robobun commented Jul 23, 2026 •

edited

Loading

coderabbitai Bot commented Jul 23, 2026 •

edited

Loading