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

Mapping to QUIC #137

Closed
aboba opened this issue Oct 1, 2020 · 6 comments · Fixed by #195
Closed

Mapping to QUIC #137

aboba opened this issue Oct 1, 2020 · 6 comments · Fixed by #195
Assignees

Comments

@aboba
Copy link
Collaborator

aboba commented Oct 1, 2020

Originally submitted by Luke Curley on the web-transport-dev mailing list.

I went through and enumerated the relevant BidirectionalStream methods and how I think they should map to QUIC:

abortReading() : send STOP_SENDING with code
abortWriting() : send RESET_STREAM with code

writable.abort() : send RESET_STREAM
writable.close() : send STREAM_FINAL
writable.getWriter().write() : send STREAM
writable.getWriter().close() : send STREAM_FINAL
writable.getWriter().abort() : send RESET_STREAM

readable.cancel() : send STOP_SENDING
readable.getReader().read() : receive STREAM or STREAM_FINAL
readable.getReader().cancel() : send STOP_SENDING

Unless I'm mistaken, there's quite a few ways to close a stream... It's another discussion but it might be worth consolidating abortReading() and abortWriting() into WHATWG streams' cancel() and abort() respectively.

Here's what I have tested and observed in the Chrome implementation:

writable.getWriter().write() : send STREAM
writable.getWriter().close() : send STREAM_FINAL, RESET_STREAM with code 268?, and STOP_SENDING with code 6?
readable.getReader().read() : receive STREAM or STREAM_FINAL

@aboba
Copy link
Collaborator Author

aboba commented Oct 1, 2020

Going through the WebTransport spec, here is the behavior that appears to be specified in various sections relating to closing or aborting.

QuicTransport interfaces

BiDirectionalStream or ReceiveStream.abortReading() : send STOP_SENDING with errorCode. (see: https://wicg.github.io/web-transport/#dom-incomingstream-abortreading). Note typo that refers to "abortwriting" instead of "abortreading" in this section.
BiDirectionalStream or SendStream.abortWriting() : send RESET_STREAM with errorCode (see: https://wicg.github.io/web-transport/#dom-outgoingstream-abortwriting)

QuicTransport.close() : send CONNECTION_CLOSE with errorCode, reason (see: https://wicg.github.io/web-transport/#dom-webtransport-close)

There is also WHATWG streams behavior from the cancel() and abort() methods which isn't mentioned in the document. It would seem reasonable to clarify that they are mapped as you indicated:

writable.abort() : send RESET_STREAM
writable.getWriter().abort() : send RESET_STREAM

readable.cancel() : send STOP_SENDING
readable.getReader().cancel() : send STOP_SENDING

and yes, all of this does imply quite a few ways to close a stream. Not sure it is too terrible assuming it is made clear what everything does.

@aboba
Copy link
Collaborator Author

aboba commented Oct 1, 2020

From Luke Curley:

Yep that's all accurate. Although you missed WritableStream.close(), which should behave like an end of stream marker (clean termination). This means sending a STREAM frame with the final bit sent and not a RESET_STREAM.

I think it would be a good idea to explicitly map the WHATWG stream methods to specific QUIC operations. I don't think there's any incompatibilities and they both have a surprisingly similar API.

@ricea
Copy link
Contributor

ricea commented Oct 1, 2020

This is very timely, as I am working on cleaning up Chrome's BidirectionalStream state machine right now.

Unless I'm mistaken, there's quite a few ways to close a stream... It's another discussion but it might be worth consolidating abortReading() and abortWriting() into WHATWG streams' cancel() and abort() respectively.

IIUC, QUIC supports resetting a stream even after closing it. Since abort() does nothing after close() on a WritableStream, we still need to have an abortWriting() method if we want to expose this functionality.

@jan-ivar
Copy link
Member

WebTransport TPAC meeting notes:

@martinthomson
Copy link
Member

This analysis seems OK to me. I would support trying to winnow the list a little. I see no value in abortBlah methods when the streams themselves have the capability.

I don't think that the QUIC facility that allows reset after close is necessary here, but it could be useful. I will open another issue on this point.

@aboba
Copy link
Collaborator Author

aboba commented Dec 11, 2020

Related Issue: #94

@jan-ivar jan-ivar linked a pull request Jan 6, 2021 that will close this issue
@aboba aboba mentioned this issue Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants