Skip to content

fix(http): sendWebResponse no longer hangs when the client disconnects during backpressure - #303

Merged
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/send-web-response-drain-hang
Aug 11, 2026
Merged

fix(http): sendWebResponse no longer hangs when the client disconnects during backpressure#303
ryansolid merged 1 commit into
solidjs:nextfrom
brenelz:fix/send-web-response-drain-hang

Conversation

@brenelz

@brenelz brenelz commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Problem

sendWebResponse (the node-to-web bridge used by the turnkey dev/preview middlewares) waits out backpressure with:

if (!res.write(value)) {
  await new Promise((resolve) => res.once('drain', resolve));
}

A response whose client has gone away never emits 'drain' — the disconnect surfaces as 'close' — and the existing 'close' handler only cancels the body reader, which unblocks a pending read() but not this wait. So every streamed SSR response aborted mid-stream (closed tab, slow mobile client, navigation away) parks the promise chain, the reader, and the Response object forever. Over a dev/preview session these leaks accumulate with no visible cause.

Fix

The backpressure wait now settles on 'drain', 'close', or 'error' (listeners removed either way), and the write loop bails out early when the response is already destroyed. On disconnect the function returns and the existing 'close' handler's reader cancellation completes cleanup; the success path is unchanged.

Verification

Two scripts driving a real node:http server through sendWebResponse:

  • Abort repro — client connects, never reads (TCP backpressure makes res.write return false), then destroys the socket while the server awaits drain. Before: the sendWebResponse promise never settles. After: it settles promptly.
  • Happy path — a slow consumer reads a 4 MB stream to completion through repeated drain cycles: all bytes delivered, promise resolves.

🤖 Generated with Claude Code

… backpressure — the 'drain' wait had no other way to resolve, but a closed response never emits 'drain', so every streamed response aborted mid-stream leaked its promise chain, reader, and Response for the rest of the dev/preview session; the wait now races 'close'/'error' and the loop bails once the response is destroyed

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Aug 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: e0de170

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
vite-plugin-solid Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Aug 11, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/vite-plugin-solid@303

commit: e0de170

@ryansolid ryansolid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced the hang on next.24 with a raw-socket client that stops reading and then disconnects mid-backpressure: the sendWebResponse promise never settles, exactly as described. On this branch it settles promptly; a slow-consumer happy path (4MB through repeated drain cycles) delivers every byte in both versions, and I also probed the other disconnect shape (parked in reader.read()) — that path already settled correctly via the close handler's reader cancellation, so scoping the fix to the drain wait is complete. Turnkey e2e passes 328/328 with the change. Approving. Non-blocking: this has no automated regression test — understandable since we have no node-only harness for src/http.ts — but the repro is small and browserless, so it would slot into a suite easily; we'll take it as a follow-up.

@ryansolid
ryansolid merged commit 2e7b63c into solidjs:next Aug 11, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants