Repository navigation
fix(ext/http): keep request body readable after response is sent - #36629
Merged
bartlomieju merged 4 commits intoAug 19, 2026
Merged
Conversation
) Since 2.8.3 the Deno-owned HTTP/1.1 serve path shared a single connection between the request-body reader and the response writer, so responding before `request.body` was fully read could stall the read or fail it with `Cannot read request body as underlying resource unavailable`. Two problems, both fixed here: 1. While a streaming response is held open, the response writer's `poll_peer_closed_with` reads bytes off the socket (including request-body bytes) into a buffer but only woke itself, leaving the body reader parked with its bytes stranded. The body reader now registers a waker in `SharedConn` that `poll_peer_closed_with` wakes when it buffers data. 2. `InnerRequest.close()` eagerly closed the request-body resource when the response was dispatched, cutting off a background read. It no longer force-closes a body that is still being consumed; ownership passes to the stream (`autoClose: true`), which closes the resource on end-of-stream, cancel, or error. The connection loop also waits for the reader to finish (via a new `reader_done` signal) before reclaiming the connection, so it doesn't take the shared connection out from under an in-flight read. Adds a spec test covering both facets (streaming response held open, and a completed response while the body is read in the background).
Address review feedback on the raw H1 request-body lifetime: - Waiting for the reader to finish before reclaiming the connection parks the connection task forever when a handler abandons `req.body` after a partial read (nothing ever closes the resource), which also blocks `server.shutdown()`. Instead, leave the connection with the body resource: a request whose body was not fully read never reuses its connection anyway, and the socket is closed when the stream reaches end-of-stream or is closed, cancelled or dropped. - Make body reads cancellable via the connection cancel handle, so a read that outlives its response cannot keep a torn-down server alive. - Don't reclaim the connection on the synchronous native-response path either while a reader still owns the body. - Clear the body-read waker once a read completes, so `poll_peer_closed_with` doesn't wake a task that is no longer waiting. Adds an http_h1 unit test for the waker hand-off and extends the spec test with the fast static response and abandoned-reader cases, plus byte counts so a truncated body fails as loudly as a stalled one.
Both specs listened on the fixed port 12345, so they fail whenever they run at the same time (or alongside anything else holding that port). They now bind port 0 and take the assigned port from `onListen`, which also removes the fetch retry loop and lets the windows/unix variants of `basic` collapse into one test. `--allow-net` is needed now because `deno serve` derives its implicit net grant from the `--port` value.
nathanwhit
approved these changes
Aug 19, 2026
bartlomieju
added a commit
that referenced
this pull request
Aug 27, 2026
dinwwwh
added a commit
to middleapi/standard-server
that referenced
this pull request
Sep 4, 2026
Deno 2.8.3 through 2.9.5 stopped delivering request body chunks to a Deno.serve handler once it started writing a streaming response, so the deno-fetch event stream echo tests deliberately asserted the stall. Deno 2.9.6 fixed it (denoland/deno#36629), which made those assertions fail in CI. Both Deno adapters are full-duplex now, so the FULL_DUPLEX_ADAPTERS gate and its stall branches are removed.
dinwwwh
added a commit
to middleapi/standard-server
that referenced
this pull request
Sep 5, 2026
…pnpm (#82) Bun 1.4 and Deno 2.9.6 both fixed stream behavior that the Bun and Deno test suites deliberately asserted as broken, so CI on main has been red since Bun 1.4.0 landed and the Deno failures surfaced right after. The failures are runtime behavior changes, not standardserver bugs, and the tests now assert the fixed behavior instead of skipping anything. Alongside that, `@types/node` and `@types/bun` are bumped, `EventStreamDecoderStream` no longer declares `implements ReadableWritablePair`, pnpm is now declared through `devEngines` so it self-provisions, and Node 20 is dropped from the CI matrix. ## Bun 1.4: stream cancellation now propagates Pure-Bun repros with no standardserver code show the same change between Bun 1.3.14 and 1.4.0: - Cancelling a response body now closes the connection, so the server signal aborts and the stream cancel fires. - A streaming request body is now cancelled on the client once the server has responded, with or without keep-alive. - An errored server response stream now reaches the client as an `ECONNRESET` error instead of a clean end of body. Both Bun adapters propagate request and response cancellation, so the `REQUEST_STREAM_CANCEL_ADAPTERS` and `RESPONSE_STREAM_CANCEL_ADAPTERS` gates and their else-branches are gone. One difference remains behind a new `RESPONSE_STREAM_ERROR_ABORT_ADAPTERS` set: `Bun.serve` only aborts `request.signal` when the client goes away, not when the server itself closes the connection after its response stream errors. The test asserts that current behavior so a future Bun change surfaces as a failure, same convention as before. ## Deno 2.9.6: request body stays readable while streaming a response Deno 2.8.3 through 2.9.5 stopped delivering request body chunks to a `Deno.serve` handler once it started writing a streaming response, so the `deno-fetch` event stream echo tests asserted the stall. Deno 2.9.6 fixed it (denoland/deno#36629), and CI picks up the latest Deno 2, so those assertions started failing. Both Deno adapters are full-duplex now, so the `FULL_DUPLEX_ADAPTERS` gate and its stall branches are removed, with the minimum Deno version noted in the test. ## Types - `@types/node` 26.1 → 26.4.1 across the workspace, `@types/bun` 1.3 → 1.4.0 for the Bun tests. - `EventStreamDecoderStream` drops `implements ReadableWritablePair`. Node's types only expose that name under `stream/web`, not as a global, so the clause failed to resolve for consumers type-checking against Node types alone. The class still exposes the same `readable`/`writable` pair structurally, so nothing changes at runtime or for callers. ## Toolchain and CI - pnpm is declared via `devEngines.packageManager` (`^11.0.0`, `onFail: download`) instead of a `packageManager` pin, so any pnpm on a contributor's machine or in CI provisions a compatible 11.x automatically. The lockfile records the resolved 11.25.0 and picks up refreshed resolutions within the existing dependency ranges. - Node 20 is removed from the CI matrix. pnpm 11 requires Node 22.13+ and crashed on `node:sqlite` before running a single test on Node 20, and Node 20 reached end-of-life in April 2026. The matrix is now 22, 24 and 26. ## Testing - CI is green on Node 22, 24 and 26 at the current head: lint and typecheck, tests with coverage, publish preview, benchmarks and codecov. - Full `pnpm -w test` passes locally on the refreshed lockfile with pnpm 11.25.0: Bun 50 pass, Deno 4 suites / 50 steps pass, vitest 1076 tests across 59 files pass. - `bun test` on Bun 1.4.1 and `deno test` on Deno 2.9.6 each pass 3 of 3 repeated runs. - `pnpm run type:check` passes for all 11 workspace projects with the new types. - Bun older than 1.4.0 or Deno older than 2.9.6 will now fail these tests, matching the CI baseline (`pnpm runtime set bun 1` / `deno 2`).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #36624.
What
Since 2.8.3,
Deno.serveties the request-body stream to the responselifecycle on the Deno-owned HTTP/1.1 path (#34446). A handler that responds
before fully consuming
request.bodycan no longer finish reading it:boundary and then returns a streaming
Responsewhilerequest.bodyisstill being piped, the remaining request-body chunks and EOS were never
delivered — the read stalled forever.
201) response while the request body is read in the background, the readfailed with
Cannot read request body as underlying resource unavailable.Both work on ≤ 2.8.2 (hyper) and now work again.
Why
The raw H1 path shares a single connection between the request-body reader
and the response writer. Two problems fell out of that:
While a streaming response is held open, the response writer's
poll_peer_closed_withreads bytes off the socket — including request-bodybytes — into an internal buffer, but only woke itself. The body reader
parked forever with its bytes stranded. It now registers a waker in
SharedConnthatpoll_peer_closed_withwakes whenever it buffers data(client-disconnect detection is preserved).
perf(ext/http): add Deno-owned HTTP/1.1 serve path #34446 added an eager
core.tryClose(#streamRid)toInnerRequest.close(),which closed the request-body resource out from under an in-flight
background read. Hyper never hit this because it defers cleanup to
HttpRequestBodyAutocloser(fires on record drop, after the readcompletes).
InnerRequest.close()no longer force-closes a body that isstill being consumed; ownership passes to the stream (
autoClose: true),which closes the resource on end-of-stream, cancel, or error. The
connection loop also waits for the reader to finish (a new
reader_donesignal) before reclaiming the shared connection.
Tests
New spec test
tests/specs/serve/request_body_outlives_response/coveringboth facets (with a timeout watchdog so a future regression fails fast rather
than hanging CI).
Verified locally: all serve specs,
serve_test/http_test/fetch_test/websocket_testunits,unit_node::http_test/http2_test,deno_http_h1(83 tests), and serve integration tests pass. Also checked manually:
keep-alive with fully-read bodies, never-read body + keep-alive (no hang),
the microtask variant, and a 20-chunk / 10 KB streamed body under interleaved
reads.