Skip to content

Clear WritableStreamSink queue on abort with compat flag#2467

Merged
jasnell merged 1 commit intomainfrom
jsnell/internal-streams-abort-queue
Aug 9, 2024
Merged

Clear WritableStreamSink queue on abort with compat flag#2467
jasnell merged 1 commit intomainfrom
jsnell/internal-streams-abort-queue

Conversation

@jasnell
Copy link
Copy Markdown
Collaborator

@jasnell jasnell commented Jul 31, 2024

The motivation here is to address an issue with WritableStream instances wrapping a WritableStreamSink implementation not processing abort() calls proactively. Without the flag the abort() will be handled lazily, meaning that a consumer must be actively consuming the data written to the stream in order to trigger the abort and close the stream. This is problematic if the consumer has dropped or otherwise stopped reading from the writable's internal buffer ... the pending abort will never be processed and resolved. This means that pending writes and the pending abort itself will never be resolved in those cases. With the flag, the queue of pending writes is drained immediately when abort() is called and the stream is transitioned to an error state immediately. This should cause the underlying WritableStreamSink to be aborted and dropped immediately and the pending write and abort promises should all get settled.

internal ticket: EW-8607

@jasnell jasnell requested review from a team as code owners July 31, 2024 14:19
@jasnell
Copy link
Copy Markdown
Collaborator Author

jasnell commented Aug 1, 2024

@anonrig ... just fyi on this PR. This is a good simple example of the introduction of a compat flag to preserve backwards compat behavior... just for illustration.

@jasnell
Copy link
Copy Markdown
Collaborator Author

jasnell commented Aug 9, 2024

Thank you for the review @anonrig ... going to wait for @kentonv to give another review before merging this.

@kentonv
Copy link
Copy Markdown
Member

kentonv commented Aug 9, 2024

Sorry for delay, I had started reviewing this a few days ago and then got distracted away.

@jasnell jasnell force-pushed the jsnell/internal-streams-abort-queue branch from 3bb757d to 58fc961 Compare August 9, 2024 19:27
@jasnell jasnell requested review from anonrig and kentonv August 9, 2024 19:50
@jasnell jasnell force-pushed the jsnell/internal-streams-abort-queue branch from 8165500 to fe09ba6 Compare August 9, 2024 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants