Skip to content

quic: fix readable stream truncation on stop-sending, abort & timeout#63967

Open
pimterry wants to merge 1 commit into
nodejs:mainfrom
pimterry:fix-reader-after-timeout
Open

quic: fix readable stream truncation on stop-sending, abort & timeout#63967
pimterry wants to merge 1 commit into
nodejs:mainfrom
pimterry:fix-reader-after-timeout

Conversation

@pimterry

Copy link
Copy Markdown
Member

Currently in ~all cases where a QUIC stream is closed abruptly, we still exposed the data as a successfully completed stream on the iterable - in effect truncating it without any notice. This includes:

  • Idle timeouts. When you hit the timeout, the readable stream just ends abruptly.
  • Local destroy() or stopSending(). This is a no-error abort, but that doesn't mean we should expose the received data as a successfully received & complete stream.
  • Remote reset (clean or error). Ditto.

This PR changes the iterable to throw in any scenario where you don't receive the full stream. If the remote peer doesn't send a happy FIN successfully for some reason, you haven't received the full data, and you should know that.

This matches the behaviour for Node streams in all our other APIs, including TCP sockets, which expose various PREMATURE_CLOSE and similar errors when you try to read a stream that is closed but hasn't actually received a clean end signal. All data up to the stream abort is delivered successfully, it's just the final read at the end that exposes the error.

This modifies blob.js generally, but only createBlobReaderIterable which is exclusively used by QUIC, and I think the behaviour is correct for an iterable reader in any scenarios.

This change doesn't touch closed, which I did consider for a while. The docs say:

A promise that is fulfilled when the stream is fully closed. It resolves
when the stream closes cleanly (including idle timeout). It rejects with
an ERR_QUIC_APPLICATION_ERROR or ERR_QUIC_TRANSPORT_ERROR when the
stream is closed due to a QUIC error (e.g., stream reset by the peer,
CONNECTION_CLOSE with a non-zero error code).

I'm treating that as "is there an actual definite error" as opposed to "the readable or the writable actually completed successfully". This diverges from our other stream semantics, but I think it's reasonable. Just might be worth considering if we want a separate way to wait for "readable ended OK" later on, currently those aren't part of the stream API so only the consumer itself knows this and there's no way to check the stream result externally. I'd rather leave that till later and just fix the clear bugs for now.

Signed-off-by: Tim Perry <pimterry@gmail.com>
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/quic

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Jun 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants