Repository navigation
Conversation
…m is cancelled (ie, out of order packet delivery)
🦋 Changeset detectedLatest commit: 27aa5b8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
size-limit report 📦
|
| cancel: () => { | ||
| // The consumer side of the pipe chain errored (e.g. `ensureOrderedChunks` saw a gap) or | ||
| // was cancelled, which cancels this source stream. Unregister it so later chunks and the | ||
| // trailer for this stream are dropped rather than hitting a closed controller. | ||
| if ( | ||
| this.textStreamControllers.get(streamHeader.streamId)?.controller === streamController | ||
| ) { | ||
| this.textStreamControllers.delete(streamHeader.streamId); |
There was a problem hiding this comment.
🟡 Aborted reads keep receiving chunks
When readAll({ signal }) aborts, cancel never runs because reader cleanup only releases the lock. Both stream controllers keep accepting chunks into an unread queue until the trailer arrives.
Learn more
The manager registers an incoming stream until its source ReadableStream is canceled or a trailer arrives. The new source cancel hook unregisters it when cancellation propagates through the pipe. However, TextStreamReader and ByteStreamReader only release their reader locks when an abort or premature iteration ends. Releasing a lock does not cancel the stream, so its source remains registered and continues buffering incoming packets despite the caller having stopped reading. The same omission affects both new cancellation hooks.
Example: A client aborts reader.readAll({ signal }) during a large text transfer. The promise rejects, but later chunks continue to enter the text stream's unread queue until a trailer arrives; the byte reader behaves the same way.
Recommended fix: On abort or early iteration exit, cancel the underlying reader before releasing its lock, and test that both text and byte controllers drop subsequent packets immediately. Preserve the existing error propagation for failed reads.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I think this is outside the scope of this fix, and I will make a follow up pull request fixing this: #2168
Fixes #2166
#2166 describes an issue where if a data stream has packets that are delivered out of order and the stream controller is cancelled, further packets that are delivered are attempted to be enqueued on a closed controller, resulting in unexpected errors thrown by the sdk.
This pull request fixes this issue by ensuring that when a stream controller is cancelled (ie, due to out of order packets, inflate/deflate error, or some other means), the stream controller is removed from the
textStreamControllers/byteStreamControllersmaps, and that if any further packets are will be delivered, they will be treated as if the data stream does not exist.