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

Fetch/XHR/Beacon: test locked/disturbed ReadableStream #12639

Merged
merged 3 commits into from
Aug 24, 2018

Conversation

annevk
Copy link
Member

@annevk annevk commented Aug 23, 2018

See whatwg/fetch#801 for context.

Copy link
Member

@igrigorik igrigorik left a comment

Choose a reason for hiding this comment

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

LGTM from this end for the sendBeacon portion. Although, I'd love to get @yutakahirano sanity check too.

@@ -0,0 +1,23 @@
function assert_beacon(stream) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Beacon with streams is never going to work because it's keepalive + ReadableStream which is rejected by another reason.

@annevk
Copy link
Member Author

annevk commented Aug 24, 2018

@yutakahirano good point, fixed.


function assert_request(input, init) {
assert_throws(new TypeError(), () => new Request(input, init), "new Request()");
assert_throws(new TypeError(), () => fetch(input, init), "fetch()");
Copy link
Contributor

Choose a reason for hiding this comment

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

await missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

Because fetch never throws. Maybe we should make all tests asynchronous.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh my, thanks for spotting that.

@annevk
Copy link
Member Author

annevk commented Aug 24, 2018

I think I fixed that too.

@annevk annevk merged commit aecdb30 into master Aug 24, 2018
@annevk annevk deleted the annevk/locked-or-disturbed-stream branch August 24, 2018 07:34
annevk added a commit to whatwg/fetch that referenced this pull request Aug 28, 2018
This also introduces "safely extract" which mainly serves to make it explicit which extraction operations cannot throw.

Tests: web-platform-tests/wpt#12639.

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

Successfully merging this pull request may close these issues.

4 participants