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

XMLHttpRequest: response stream errors #27778

Merged
merged 2 commits into from Mar 2, 2021
Merged

Conversation

@annevk
Copy link
Member

@annevk annevk commented Feb 25, 2021

For whatwg/xhr#314.

It seems Chrome does not throw exceptions here at all...

@annevk annevk requested a review from yutakahirano Feb 25, 2021
@github-actions github-actions bot temporarily deployed to wpt-preview-27778 Feb 25, 2021 Inactive
@yutakahirano
Copy link
Contributor

@yutakahirano yutakahirano commented Feb 26, 2021

https://crbug.com/602051 Service worker doesn't intercept sync XHR requests.

cc: @domfarolino

We can write a test with https://github.com/web-platform-tests/wpt/blob/master/fetch/api/resources/bad-chunk-encoding.py alternatively.

Copy link
Contributor

@yutakahirano yutakahirano left a comment

LGTM

@annevk
Copy link
Member Author

@annevk annevk commented Feb 26, 2021

@yutakahirano thanks for identifying that! I was wondering if there were non-ReadableStream ways to trigger this condition. That will make testing less cumbersome going forward and also reduces my worry about this being not handled well across callers (although most specification callers of fetch do not seem to account for it at the moment).

@wpt-pr-bot wpt-pr-bot added the xhr label Feb 26, 2021
@wpt-pr-bot wpt-pr-bot requested review from caitp, jdm, Manishearth and wisniewskit Feb 26, 2021
annevk added a commit to whatwg/xhr that referenced this pull request Feb 26, 2021
And also clean up the way we manage XMLHttpRequest's response, which allows for simplifying "handle response end-of-body" and "handle errors".

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

Fixes #314.
@annevk annevk mentioned this pull request Feb 26, 2021
3 of 3 tasks complete
@annevk annevk merged commit 57ceb7e into master Mar 2, 2021
21 checks passed
21 checks passed
@github-actions
update-pr-preview
Details
@github-actions
update-pr-preview
Details
@azure-pipelines
Azure Pipelines Build #20210226.31 succeeded
Details
@azure-pipelines
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
@azure-pipelines
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
@azure-pipelines
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
@azure-pipelines
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
@azure-pipelines
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
@community-tc-integration
download-firefox-nightly Community-TC (pull_request)
Details
@community-tc-integration
lint Community-TC (pull_request)
Details
@community-tc-integration
sink-task Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-results Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-results-without-changes Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-stability Community-TC (pull_request)
Details
@community-tc-integration
wpt-decision-task Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-results Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-results-without-changes Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-stability Community-TC (pull_request)
Details
@wpt-fyi
wpt.fyi - chrome[experimental] Chrome results
Details
@wpt-fyi
wpt.fyi - firefox[experimental] Firefox results
Details
@wpt-fyi
wpt.fyi - safari[experimental] Safari results
Details
@annevk annevk deleted the annevk/sync-xhr-stream-errors branch Mar 2, 2021
annevk added a commit to whatwg/xhr that referenced this pull request Mar 3, 2021
And also clean up the way we manage XMLHttpRequest's response, which allows for simplifying "handle response end-of-body" and "handle errors". (Also ends up removing the remaining dependency on Streams.)

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

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

Successfully merging this pull request may close these issues.

None yet

4 participants