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

Set stored error from |reason| in abort(reason) #903

Merged
merged 2 commits into from
Mar 9, 2018

Conversation

ricea
Copy link
Collaborator

@ricea ricea commented Mar 8, 2018

Previously, calling abort(reason) on a WritableStream would set the
stored error to a TypeError, meaning that future operations on that
WritableStream would reject with a TypeError. This resulted in losing
the original reason when an error happened in a pipe. Instead, retain
the abort reason in the stored error slot.

Similarly, aborting the writable side of a TransformStream would set the
stored error on the readable side to a TypeError. Make it use the reason
as well to reflect the new behaviour of the writable side.

Closes #896.


Preview | Diff

ricea added a commit to ricea/web-platform-tests that referenced this pull request Mar 8, 2018
This tests the changes in whatwg/streams#903.

Replace expectations that a TypeError will be stored after abort() is
called with with expectations that the reason passed to abort() will be
stored instead.

Also add a test of the stored error on the readable side of a
TransformStream after abort() has been called on the writable size. This
was not explicitly tested.

Also fix a bug in the general.js test: it didn't wrap a call to
assert_unreached properly, so a failure would have shown up as an
unhandled rejection rather than being reported properly.
Previously, calling abort(reason) on a WritableStream would set the
stored error to a TypeError, meaning that future operations on that
WritableStream would reject with a TypeError. This resulted in losing
the original reason when an error happened in a pipe. Instead, retain
the abort reason in the stored error slot.

Similarly, aborting the writable side of a TransformStream would set the
stored error on the readable side to a TypeError. Make it use the reason
as well to reflect the new behaviour of the writable side.

Closes whatwg#896.
@ricea
Copy link
Collaborator Author

ricea commented Mar 8, 2018

Tests are in web-platform-tests/wpt#9926.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clean spec change, but then a bunch of test changes. Thanks for doing the work :)

ricea added a commit to web-platform-tests/wpt that referenced this pull request Mar 9, 2018
This tests the changes in whatwg/streams#903.

Replace expectations that a TypeError will be stored after abort() is
called with with expectations that the reason passed to abort() will be
stored instead.

Also add a test of the stored error on the readable side of a
TransformStream after abort() has been called on the writable size. This
was not explicitly tested.

Also fix a bug in the general.js test: it didn't wrap a call to
assert_unreached properly, so a failure would have shown up as an
unhandled rejection rather than being reported properly.
@domenic domenic merged commit dd20dd0 into whatwg:master Mar 9, 2018
@ricea ricea deleted the abort-no-censorship branch March 9, 2018 14:14
aarongable pushed a commit to chromium/chromium that referenced this pull request Mar 16, 2018
Update WritableStream to match the standard version
whatwg/streams@7b8dffe.

* Significant user-visible changes:

Method lookups are now cached at construction time. Changing
underlyingSink.write after construction will no longer do anything. This
is whatwg/streams#860.

When abort(reason) is called, the stored error will now be set to
|reason| instead of a TypeError. This is
whatwg/streams#903.

* Significant internal changes:

CreateWritableStream() operation is now exported. TransformStream will
be modified to use this in a follow-up CL. This is
whatwg/streams#857.

New common operations ValidateAndNormalizeQueuingStrategy,
MakeSizeAlgorithmFromSizeFunction, CreateAlgorithmFromUnderlyingMethod
and CreateAlgorithmFromUnderlyingMethodPassingController are
implemented.

Remove PromiseCallOrNoop0 and PromiseCallOrNoop2 as they are no longer
used.

Update external/wpt/streams test expectations for the massive reduction
in failures. Also chromium simple-queue tests expected abort() to result
in a TypeError, and needed updating. Also remove the expectation that
external/wpt/streams/readable-streams general.html will Timeout, since
none of the other tests using general.js are marked Timeout.

Bug: 820246, 820387, 626703
Change-Id: Id1364921c37b03cdf3c89e201079292b13b9214c
Reviewed-on: https://chromium-review.googlesource.com/961566
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543659}
aarongable pushed a commit to chromium/chromium that referenced this pull request Mar 22, 2018
Update TransformStream to match the standard version
whatwg/streams@7b8dffe.

Significant changes:

Method lookups are now cached at construction time. Changing
underlyingSink.write after construction will no longer do anything. This
is whatwg/streams#860.

When the readable side is cancelled, the stored error on the writable side will
be set to the |reason| that was passed to cancel(), rather than a TypeError.
This is whatwg/streams#903.

Bug: 780689, 820387

Change-Id: I33c59541eea1bfc5f96722bce750ba4fa7ff96ee
Reviewed-on: https://chromium-review.googlesource.com/964112
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Commit-Queue: Adam Rice <ricea@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545038}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Apr 1, 2018
…son passthrough, a=testonly

Automatic update from web-platform-testsStreams: Modify tests to expect abort reason passthrough (#9926)

This tests the changes in whatwg/streams#903.

Replace expectations that a TypeError will be stored after abort() is
called with with expectations that the reason passed to abort() will be
stored instead.

Also add a test of the stored error on the readable side of a
TransformStream after abort() has been called on the writable size. This
was not explicitly tested.

Also fix a bug in the general.js test: it didn't wrap a call to
assert_unreached properly, so a failure would have shown up as an
unhandled rejection rather than being reported properly.

wpt-commits: 81468a0b88d771af9dfa0feb5de99367e32d4b6e
wpt-pr: 9926
wpt-commits: 81468a0b88d771af9dfa0feb5de99367e32d4b6e
wpt-pr: 9926
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…son passthrough, a=testonly

Automatic update from web-platform-testsStreams: Modify tests to expect abort reason passthrough (#9926)

This tests the changes in whatwg/streams#903.

Replace expectations that a TypeError will be stored after abort() is
called with with expectations that the reason passed to abort() will be
stored instead.

Also add a test of the stored error on the readable side of a
TransformStream after abort() has been called on the writable size. This
was not explicitly tested.

Also fix a bug in the general.js test: it didn't wrap a call to
assert_unreached properly, so a failure would have shown up as an
unhandled rejection rather than being reported properly.

wpt-commits: 81468a0b88d771af9dfa0feb5de99367e32d4b6e
wpt-pr: 9926
wpt-commits: 81468a0b88d771af9dfa0feb5de99367e32d4b6e
wpt-pr: 9926

UltraBlame original commit: 7ec8a0f2b4173c7bbd730cbfed9dae6ae1c8eed6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
…son passthrough, a=testonly

Automatic update from web-platform-testsStreams: Modify tests to expect abort reason passthrough (#9926)

This tests the changes in whatwg/streams#903.

Replace expectations that a TypeError will be stored after abort() is
called with with expectations that the reason passed to abort() will be
stored instead.

Also add a test of the stored error on the readable side of a
TransformStream after abort() has been called on the writable size. This
was not explicitly tested.

Also fix a bug in the general.js test: it didn't wrap a call to
assert_unreached properly, so a failure would have shown up as an
unhandled rejection rather than being reported properly.

wpt-commits: 81468a0b88d771af9dfa0feb5de99367e32d4b6e
wpt-pr: 9926
wpt-commits: 81468a0b88d771af9dfa0feb5de99367e32d4b6e
wpt-pr: 9926

UltraBlame original commit: 7ec8a0f2b4173c7bbd730cbfed9dae6ae1c8eed6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
…son passthrough, a=testonly

Automatic update from web-platform-testsStreams: Modify tests to expect abort reason passthrough (#9926)

This tests the changes in whatwg/streams#903.

Replace expectations that a TypeError will be stored after abort() is
called with with expectations that the reason passed to abort() will be
stored instead.

Also add a test of the stored error on the readable side of a
TransformStream after abort() has been called on the writable size. This
was not explicitly tested.

Also fix a bug in the general.js test: it didn't wrap a call to
assert_unreached properly, so a failure would have shown up as an
unhandled rejection rather than being reported properly.

wpt-commits: 81468a0b88d771af9dfa0feb5de99367e32d4b6e
wpt-pr: 9926
wpt-commits: 81468a0b88d771af9dfa0feb5de99367e32d4b6e
wpt-pr: 9926

UltraBlame original commit: 7ec8a0f2b4173c7bbd730cbfed9dae6ae1c8eed6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants