Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make underlying sink writes abortable #1015

Closed
ricea opened this issue Sep 13, 2019 · 5 comments · Fixed by #1132
Closed

Make underlying sink writes abortable #1015

ricea opened this issue Sep 13, 2019 · 5 comments · Fixed by #1132

Comments

@ricea
Copy link
Collaborator

ricea commented Sep 13, 2019

We should pass an AbortSignal object to underling sink write() calls and signal abort if the stream is aborted. write() implementations could either ignore it (giving the existing behaviour), or reject the promise they returned, making the abort operation complete more quickly than it would otherwise.

This would also work with pipeTo() abort (as long as preventAbort was not set).

@MattiasBuelens
Copy link
Collaborator

Yes, please! 😀

I've been trying to make a fully spec-compliant TransformStream polyfill that uses native ReadableStream and WritableStream, so only using their public API. However, I ran into problems with TransformStreamDefaultSinkWriteAlgorithm, which needs to check whether the stream is erroring (i.e. abort() has been called) before it transforms the chunk. Currently, this is impossible to know using only the public API. Having an AbortSignal passed to each sink write() call would solve this.

@MattiasBuelens
Copy link
Collaborator

Random thought: what about transform streams? Should we also pass a signal to transform(), so the underlying transformer can stop early if it knows the transformed chunk will no longer be used? 🤔

Slightly related: an underlying transformer currently has no way to know that its writable side has become aborted, or that its readable side has become cancelled. There's only flush(), which is called when the writable side becomes closed. Do we need an abort(reason) and/or cancel(reason) method on a transformer?

@MattiasBuelens
Copy link
Collaborator

MattiasBuelens commented Sep 24, 2019

We could also add it as a signal property on WritableStreamDefaultController, since the abort signal doesn't really need to change between two write() calls.

@yutakahirano
Copy link
Member

It's good to make close() potentially abortable too.

We are discussing whether we want an abort operation which takes effect immediately even when there is an in-flight write or close operation (w3c/webtransport#248), but this should be a solution for the problem.

cc: @jan-ivar

@ricea
Copy link
Collaborator Author

ricea commented May 11, 2021

Currently in Chrome SendStream's close() doesn't wait for anything, It appears the standard doesn't yet say what should happen, but it's possible we will wait for the data pipe to be empty before resolving the promise. In which case, being able to throw away that data would be useful.

In the general case, when close() is a non-atomic, it would be useful for it to be abortable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants