-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Correctly reject in-progress body methods with AbortError
Prior to this change, response body methods such as response.body.arrayBuffer() would reject with a TypeError if the fetch was aborted. This change makes them correctly reject with an AbortError instead. This implements stage #3 of the "Body Cancellation" section of the design doc: https://docs.google.com/document/d/1OuoCG2uiijbAwbCw9jaS7tHEO0LBO_4gMNio1ox0qlY/edit#heading=h.fvc7d7q07oio Bug: 817687 Change-Id: Ifde322d9c22485a8ba9d14fd4ffca65c4fb4745a Reviewed-on: https://chromium-review.googlesource.com/954765 Commit-Queue: Adam Rice <ricea@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/heads/master@{#544335}
- Loading branch information
1 parent
7cb5db5
commit f2ae764
Showing
5 changed files
with
107 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
if (self.importScripts) { | ||
importScripts('/resources/testharness.js'); | ||
importScripts('../resources/test-helpers.js'); | ||
importScripts('/common/utils.js'); | ||
} | ||
|
||
// We perform the same tests on put, add, addAll. Parameterise the tests to | ||
// reduce repetition. | ||
const methodsToTest = { | ||
put: async (cache, request) => { | ||
const response = await fetch(request); | ||
return cache.put(request, response); | ||
}, | ||
add: async (cache, request) => cache.add(request), | ||
addAll: async (cache, request) => cache.addAll([request]), | ||
}; | ||
|
||
for (const method in methodsToTest) { | ||
const perform = methodsToTest[method]; | ||
|
||
cache_test(async (cache, test) => { | ||
const controller = new AbortController(); | ||
const signal = controller.signal; | ||
controller.abort(); | ||
const request = new Request('../resources/simple.txt', { signal }); | ||
return promise_rejects(test, 'AbortError', perform(cache, request), | ||
`${method} should reject`); | ||
}, `${method}() on an already-aborted request should reject with AbortError`); | ||
|
||
cache_test(async (cache, test) => { | ||
const controller = new AbortController(); | ||
const signal = controller.signal; | ||
const request = new Request('../resources/simple.txt', { signal }); | ||
const promise = perform(cache, request); | ||
controller.abort(); | ||
return promise_rejects(test, 'AbortError', promise, | ||
`${method} should reject`); | ||
}, `${method}() synchronously followed by abort should reject with ` + | ||
`AbortError`); | ||
|
||
cache_test(async (cache, test) => { | ||
const controller = new AbortController(); | ||
const signal = controller.signal; | ||
const stateKey = token(); | ||
const abortKey = token(); | ||
const request = new Request( | ||
`../../../fetch/api/resources/infinite-slow-response.py?stateKey=${stateKey}&abortKey=${abortKey}`, | ||
{ signal }); | ||
|
||
const promise = perform(cache, request); | ||
|
||
// Wait for the server to start sending the response body. | ||
let opened = false; | ||
do { | ||
// Normally only one fetch to 'stash-take' is needed, but the fetches | ||
// will be served in reverse order sometimes | ||
// (i.e., 'stash-take' gets served before 'infinite-slow-response'). | ||
|
||
const response = | ||
await fetch(`../../../fetch/api/resources/stash-take.py?key=${stateKey}`); | ||
const body = await response.json(); | ||
if (body === 'open') opened = true; | ||
} while (!opened); | ||
|
||
// Sadly the above loop cannot guarantee that the browser has started | ||
// processing the response body. This delay is needed to make the test | ||
// failures non-flaky in Chrome version 66. My deepest apologies. | ||
await new Promise(resolve => setTimeout(resolve, 250)); | ||
|
||
controller.abort(); | ||
|
||
await promise_rejects(test, 'AbortError', promise, | ||
`${method} should reject`); | ||
|
||
// infinite-slow-response.py doesn't know when to stop. | ||
return fetch(`../../../fetch/api/resources/stash-put.py?key=${abortKey}`); | ||
}, `${method}() followed by abort after headers received should reject ` + | ||
`with AbortError`); | ||
} | ||
|
||
done(); |
9 changes: 9 additions & 0 deletions
9
service-workers/cache-storage/serviceworker/cache-abort.https.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<!DOCTYPE html> | ||
<title>Cache Storage: Abort</title> | ||
<link rel="help" href="https://fetch.spec.whatwg.org/#request-signal"> | ||
<script src="/resources/testharness.js"></script> | ||
<script src="/resources/testharnessreport.js"></script> | ||
<script src="../../service-worker/resources/test-helpers.sub.js"></script> | ||
<script> | ||
service_worker_test('../script-tests/cache-abort.js'); | ||
</script> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<!DOCTYPE html> | ||
<title>Cache Storage: Abort</title> | ||
<link rel="help" href="https://fetch.spec.whatwg.org/#request-signal"> | ||
<script src="/resources/testharness.js"></script> | ||
<script src="/resources/testharnessreport.js"></script> | ||
<script src="../resources/test-helpers.js"></script> | ||
<script src="/common/utils.js"></script> | ||
<script src="../script-tests/cache-abort.js"></script> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<!DOCTYPE html> | ||
<title>>Cache Storage: Abort</title> | ||
<link rel="help" href="https://fetch.spec.whatwg.org/#request-signal"> | ||
<script src="/resources/testharness.js"></script> | ||
<script src="/resources/testharnessreport.js"></script> | ||
<script> | ||
fetch_tests_from_worker(new Worker('../script-tests/cache-abort.js')); | ||
</script> |