Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → cancel()/remoteReset #248

jan-ivar · 2021-04-13T01:36:06Z

Fixes #242.

aboba

abortReading() is being removed because it is considered a duplicate of cancel(reason), correct?
With readingAborted removed, is the application to know that a RST_STREAM has been received via reader.closed? Does this provide an errorCode value?

line 796/797: I don't find "drop/dropped" to be more informative than "abortWriting/writingAborted".
line 810: HTTP/2 fallback would not involve a STOP_SENDING frame. So being specific about Http3Transport would be better.
Line 828: HTTP/2 fallback would not involve a RST_STREAM frame. So being specific about Http3Transport would be better.
line 974, 990, 994, 1002: can you add the code argument?

jan-ivar · 2021-04-13T14:25:26Z

@domenic do you know if the reason when calling cancel(reason) on a stream is surfaced anywhere else than in the promise it immediately returns?

domenic · 2021-04-13T15:35:47Z

It is sent to the underlying source, which can interpret it as desired.

index.bs

yutakahirano · 2021-04-28T11:01:51Z

Reg: remoteReset, I think we can add properties to the exception with which closed is rejected, instead of introducing remoteReset. What do you think about the idea?

jan-ivar · 2021-04-28T22:32:28Z

Rebased

jan-ivar · 2021-04-28T23:39:38Z

Reg: remoteReset, I think we can add properties to the exception with which closed is rejected, instead of introducing remoteReset. What do you think about the idea?

Good idea! I've opened #252 on it.

In the meantime I've updated this PR to s/readingAborted/remoteReset/ as well (we can reuse the same name on readable and writable now, since we got rid of mixins).

This seems like a reasonable incremental step, so I'm hoping we can merge this PR and treat #252 as a separate idea. PTAL

index.bs

yutakahirano · 2021-05-06T13:12:14Z

index.bs

+     1. Remove [=this=] from the transport's [=[[OutgoingStreams]]=].
+     1. Let |reason| be |abortInfo|.errorCode.
+     1. If it hasn't started already, start the closing procedure by sending a
+        RST_STREAM frame with its error code set to the value of |reason|.


nit; s/RST_STREAM/RESET_STREAM/

yutakahirano · 2021-05-06T13:20:36Z

index.bs

+         |reason|.
+
+The {{SendStream}}'s <dfn>abortAlgorithm</dfn> (TODO) is:
+     1. Let |stream| be the {{ReceiveStream}} object.


nit; SendStream

yutakahirano · 2021-05-06T13:51:36Z

Hmm, I'm confused (again).

Sending a STOP_SENDING to a ReceiveStream makes sense, sending a STOP_SENDING to a SendStream doesn't.
Isn't what you want to do removing abortWriting, not abortReading? The mapping table would be:

interface	method	action
ReceiveStream	cancel	Send `STOP_SENDING` with code
ReceiveStream	reset	Send `RESET_STREAM` with code
SendStream	abort	Send `RESET_STREAM` with code

OR, as commented above, we can remove reset by adding a field to the reason argument.

interface	method	action
ReceiveStream	cancel	Send `STOP_SENDING` or `RESET_STREAM` with code
SendStream	abort	Send `RESET_STREAM` with code

jan-ivar · 2021-05-11T04:46:58Z

Hmm, I'm confused (again).

I think so (my fault). Sending a STOP_SENDING to a SendStream makes sense. It's the other one that was nonsense. PTAL

jan-ivar · 2021-05-11T06:31:19Z

With readingAborted removed, is the application to know that a RST_STREAM has been received via reader.closed? Does this provide an errorCode value?

@aboba no it doesn't. Good observation. So I've reinstated readingAborted and calling it remoteReset for now, until we can figure out the right mapping in #257. On the sender-side writingAborted is now remoteCanceled.

jan-ivar · 2021-05-11T14:29:30Z

Meeting:

Merge PR for forward progress to not block on future nice things, ~~but with the intent that we would eventually be able to remove these methods in favor of making abort in the streams spec work for us.~~
Update PR to use Protocol concepts
Error the stream on receiving STOP_SENDING to ensure producers get canceled.

jan-ivar · 2021-05-12T00:24:45Z

@domenic is this the right way to error a stream?

…Aborted

Co-authored-by: Martin Thomson <[email protected]>

domenic · 2021-05-12T19:26:11Z

@domenic is this the right way to error a stream?

Not quite. In general you should use the algorithms from https://streams.spec.whatwg.org/#other-specs , so in this case https://streams.spec.whatwg.org/#readablestream-error and ... oops, I didn't create such a wrapper for WritableStream, let me do so right now. (It will be a wrapper around WritableStreamDefaultControllerError though, not StartErroring.

Needed for w3c/webtransport#248.

jan-ivar · 2021-05-13T23:51:30Z

@yutakahirano does it look good now?

jan-ivar · 2021-05-14T00:00:00Z

... I've reinstated readingAborted and calling it remoteReset for now, until we can figure out the right mapping in #257.

With our decision to error the stream on reception of STOP_SENDING and RESET_STREAM, the remoteCanceled and remoteReset promises seem redundant. Can we remove them?

yutakahirano · 2021-05-19T08:24:58Z

Still LGTM.

Let's land this change and continue the discussion (at #260 for example).

jan-ivar · 2021-05-25T18:54:52Z

Still LGTM.

Let's land this change and continue the discussion (at #260 for example).

I'll take this as a ✅ and merge this.

@jan-ivar

Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → cancel()/remoteReset SHA: a87f72a Reason: push, by @jan-ivar Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

Needed for w3c/webtransport#248.

jan-ivar requested review from vasilvv, aboba, yutakahirano and ricea April 13, 2021 01:36

aboba reviewed Apr 13, 2021

View reviewed changes

yutakahirano reviewed Apr 15, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

jan-ivar changed the title ~~Rename abortWriting() to drop()/dropped + remove abortReading/readingAborted~~ Rename abortWriting() to reset()/remoteReset + remove abortReading/readingAborted Apr 27, 2021

martinthomson reviewed Apr 27, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

jan-ivar commented Apr 28, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

jan-ivar commented Apr 28, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

jan-ivar force-pushed the drop branch from a51a1fc to f044fb5 Compare April 28, 2021 22:31

jan-ivar mentioned this pull request Apr 28, 2021

Add custom error property for Application Protocol Error Codes? #252

Closed

yutakahirano reviewed Apr 30, 2021

View reviewed changes

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

index.bs Show resolved Hide resolved

index.bs Outdated Show resolved Hide resolved

yutakahirano reviewed May 6, 2021

View reviewed changes

jan-ivar mentioned this pull request May 11, 2021

Need protocol mappings table for QUIC→streams #257

Closed

jan-ivar changed the title ~~Rename abortWriting() to reset()/remoteReset + remove abortReading/readingAborted~~ Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading/readingAborted → 🗑️ /remoteReset May 11, 2021

jan-ivar changed the title ~~Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading/readingAborted → 🗑️ /remoteReset~~ Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → 🗑️ /remoteReset May 11, 2021

jan-ivar changed the title ~~Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → 🗑️ /remoteReset~~ Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → cancel()/remoteReset May 11, 2021

yutakahirano mentioned this pull request May 11, 2021

Make underlying sink writes abortable whatwg/streams#1015

Closed

jan-ivar added the Editors can integrate label May 11, 2021

jan-ivar and others added 10 commits May 11, 2021 20:29

Rename abortWriting() to drop()/dropped + remove abortReading/reading…

e7504c1

…Aborted

Address feedback

2439eab

s/drop()/reset()/ and s/dropped/remoteReset/

55635b1

Update index.bs

e4a0563

Co-authored-by: Martin Thomson <[email protected]>

Fix [[PromiseIsHandled]] ref.

3349bb4

Add links to methods in Protocol Mappings table.

4532dd5

Address feedback.

60931f5

s/remoteReset/remoteCanceled/ on SendStream + address nits & confusion

07cdfa5

Error the stream.

0380199

Use protocol concepts.

2855433

jan-ivar force-pushed the drop branch from e7e5d75 to 2855433 Compare May 12, 2021 00:48

yutakahirano mentioned this pull request May 12, 2021

Do we need writable.reset() for "unreliable streams"? #260

Closed

domenic added a commit to whatwg/streams that referenced this pull request May 12, 2021

Add a wrapper for other specs to error WritableStreams

1bd8135

Needed for w3c/webtransport#248.

domenic mentioned this pull request May 12, 2021

Add a wrapper for other specs to error WritableStreams whatwg/streams#1127

Merged

domenic added a commit to whatwg/streams that referenced this pull request May 13, 2021

Add a wrapper for other specs to error WritableStreams

0ff6d45

Needed for w3c/webtransport#248.

Error the stream the right way.

dc1790c

jan-ivar merged commit a87f72a into w3c:main May 25, 2021

jan-ivar deleted the drop branch May 25, 2021 18:55

jan-ivar mentioned this pull request May 26, 2021

Remove sender.remoteCanceled and receiver.remoteReset? #271

Closed

yutakahirano pushed a commit to yutakahirano/streams that referenced this pull request Jun 3, 2021

Add a wrapper for other specs to error WritableStreams

50296b8

Needed for w3c/webtransport#248.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → cancel()/remoteReset #248

Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → cancel()/remoteReset #248

jan-ivar commented Apr 13, 2021 •

edited by pr-preview bot

Loading

aboba left a comment •

edited

Loading

jan-ivar commented Apr 13, 2021

domenic commented Apr 13, 2021

yutakahirano commented Apr 28, 2021

jan-ivar commented Apr 28, 2021

jan-ivar commented Apr 28, 2021

yutakahirano May 6, 2021

yutakahirano May 6, 2021

yutakahirano commented May 6, 2021

jan-ivar commented May 11, 2021 •

edited

Loading

jan-ivar commented May 11, 2021

jan-ivar commented May 11, 2021 •

edited

Loading

jan-ivar commented May 12, 2021 •

edited

Loading

domenic commented May 12, 2021

jan-ivar commented May 13, 2021

jan-ivar commented May 14, 2021

yutakahirano commented May 19, 2021

jan-ivar commented May 25, 2021

Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → cancel()/remoteReset #248

Rename abortWriting()/writingAborted → reset()/remoteCanceled & abortReading()/readingAborted → cancel()/remoteReset #248

Conversation

jan-ivar commented Apr 13, 2021 • edited by pr-preview bot Loading

aboba left a comment • edited Loading

Choose a reason for hiding this comment

jan-ivar commented Apr 13, 2021

domenic commented Apr 13, 2021

yutakahirano commented Apr 28, 2021

jan-ivar commented Apr 28, 2021

jan-ivar commented Apr 28, 2021

yutakahirano May 6, 2021

Choose a reason for hiding this comment

yutakahirano May 6, 2021

Choose a reason for hiding this comment

yutakahirano commented May 6, 2021

jan-ivar commented May 11, 2021 • edited Loading

jan-ivar commented May 11, 2021

jan-ivar commented May 11, 2021 • edited Loading

jan-ivar commented May 12, 2021 • edited Loading

domenic commented May 12, 2021

jan-ivar commented May 13, 2021

jan-ivar commented May 14, 2021

yutakahirano commented May 19, 2021

jan-ivar commented May 25, 2021

jan-ivar commented Apr 13, 2021 •

edited by pr-preview bot

Loading

aboba left a comment •

edited

Loading

jan-ivar commented May 11, 2021 •

edited

Loading

jan-ivar commented May 11, 2021 •

edited

Loading

jan-ivar commented May 12, 2021 •

edited

Loading