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

Remove dependency on "Data Recvd" state #581

Closed
martinthomson opened this issue Jan 23, 2024 · 3 comments · Fixed by #582
Closed

Remove dependency on "Data Recvd" state #581

martinthomson opened this issue Jan 23, 2024 · 3 comments · Fixed by #582
Assignees
Labels
PR exists A PR exists for this issue

Comments

@martinthomson
Copy link
Member

ietf-wg-webtrans/draft-ietf-webtrans-http2#103 has more background.

The text for session termination starts with this:

Let cleanly be a boolean representing whether transport.[[Session]]'s CONNECT stream is in the "Data Recvd" state. [QUIC]

Whether data has been acknowledged is not a useful signal. However, the point of this variable is to split the difference between a graceful and abrupt termination. That's not a concept that can be known in the way that is described. Though data might have been received by an immediate peer, it might not have been processed, so the "Data Recvd" signal is not really that useful.

I suggest that we strike this and it's dependent conditions.

martinthomson added a commit to martinthomson/webtransport that referenced this issue Jan 23, 2024
This is a simplification, which will cause applications to see errors
more often, but I think that it is a good one.

Closes w3c#581.
@martenrichter
Copy link

OK, could you also clarify/change §6.4 procedures for WebTransportSendStream of the point

Wait for stream.[[InternalStream]] to enter the "Data Recvd" state. [QUIC]

Because this is the one point that confused me, could you clarify that this will only take place for http/3?

@martinthomson
Copy link
Member Author

@martenrichter, see #580 for that part. I did separate pull requests because this one took a bit more thinking than the other.

@martenrichter
Copy link

Ah ok, I overlooked this.

@nidhijaju nidhijaju added PR exists A PR exists for this issue and removed Ready for PR labels Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR exists A PR exists for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants