Skip to content

fix: end a streamed response after a backpressured write - #377

Open
mustafa519 wants to merge 1 commit into
dimdenGD:mainfrom
mustafa519:fix/end-after-backpressured-write
Open

mustafa519 wants to merge 1 commit into
dimdenGD:mainfrom
mustafa519:fix/end-after-backpressured-write

Conversation

@mustafa519

Copy link
Copy Markdown

Problem

A streamed response can hang forever: the whole body is sent, but the response never ends (no terminating chunk), so the client keeps waiting until a proxy or client timeout cuts it.

It happens when:

  1. the response is streamed with res.write() (for example piping React's renderToReadableStream / any ReadableStream into res), then res.end() is called;
  2. uWS reports backpressure (_res.write() returns false) on the last chunk, so writingChunk stays true until onWritable fires;
  3. Node's own Writable buffer is below its high-water mark (16 KB), so write() returned true and needDrain is not set.

end() then does:

if(this.writingChunk) {
    this.once('drain', () => {
        this.end(data, cb);
    });
    return;
}

Node only emits 'drain' after a write() that returned false. In case 3 it never does, so the deferred end() never runs, even though onWritable later completes the pending chunk.

In practice this needs a slow reader at the very end of the body, for example a reverse proxy with response buffering off that pauses reading once its buffer is full. Direct clients and buffering proxies drain fast enough that uWS rarely reports backpressure on the last chunk, which is probably why it's intermittent.

Fix

  • end() stores the deferred call in a private #endAfterWrite instead of waiting for 'drain'.
  • #continueEnd() runs it once no chunk is being written. It's called after the write callback in both onWritable paths (chunked and known content-length). Running it after the callback lets a callback that starts the next chunk keep the end waiting for that chunk too.

Test

tests/tests/res/res-end-after-backpressured-write.js makes uWS report backpressure on the last 3-byte chunk, then become writable 50 ms later, while end() is called with that chunk pending. Before the fix the response never ends (response never ended after 3 s). After it, the output matches Express.

After npm ci: node tests/index.js res 57/57 and node tests/index.js send-file 21/21 pass.

Since when

The 'drain' wait in end() is older, but it became reachable for chunked responses in 2.1.0 (45576f6, "remove timer based backpressure handling"). Before that, the chunked _write() ignored uWS backpressure and called the callback right away, so writingChunk was never true when end() ran.

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.

1 participant