Aborting fetch (and possibly streams), part 2

NEW
Assigned to

Status

()

Core
DOM
2 months ago
20 hours ago

People

(Reporter: annevk, Assigned: baku)

Tracking

({dev-doc-needed})

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(6 attachments, 4 obsolete attachments)

(Reporter)

Description

2 months ago
We have two objects defined by DOM (AbortController and AbortSignal):

  https://github.com/whatwg/dom/pull/437

That DOM PR is considered done. The reason it hasn't landed yet is because the Fetch PR https://github.com/whatwg/fetch/pull/523 is not finalized and the Streams PR https://github.com/whatwg/streams/pull/744 doesn't have enough implementer commitments yet (we don't implement writable streams or piping thus far, but we may as well express support I think since we'll do it eventually).

As far as the Fetch PR goes, the API is decided on and is pretty similar to what we have I think, we just need to figure out how we want to write down the requirements in the standards.

Comment 1

2 months ago
Are there any WPT tests written yet?
Keywords: dev-doc-needed
(Assignee)

Comment 2

25 days ago
Created attachment 8891473 [details] [diff] [review]
part 1 - Moving to dom/abort

AbortController/AbortSignal are not part of fetch. We need to have it in a separate folder.
Attachment #8891473 - Flags: review?(bkelly)
(Assignee)

Comment 3

25 days ago
Created attachment 8891475 [details] [diff] [review]
part 2 - renaming

FetchController -> AbortController
FetchSignal -> AbortSignal
Attachment #8891475 - Flags: review?(bkelly)
(Assignee)

Comment 4

25 days ago
Created attachment 8891476 [details] [diff] [review]
part 3 - Removing the following algorithm
Attachment #8891476 - Flags: review?(bkelly)
(Assignee)

Comment 5

25 days ago
Created attachment 8891477 [details] [diff] [review]
part 4 - having a separate pref for Fetch + Abort API
Attachment #8891477 - Flags: review?(bkelly)
(Reporter)

Comment 6

23 days ago
Tests for the DOM part:

  https://github.com/w3c/web-platform-tests/pull/5960

Fetch part:

  https://github.com/w3c/web-platform-tests/pull/6484

(Streams is out-of-scope for this bug at this point.)

Comment 7

12 days ago
I wanted to run these patches against the tests now that they are available.  Unfortunately it seems something is missing because they don't compile:

4:12.17 /srv/mozilla-central/obj-x86_64-pc-linux-gnu-optdebug/dom/bindings/RequestBinding.cpp:4:10: fatal error: 'AbortObserver.h' file not found
 4:12.18 #include "AbortObserver.h"


Forget to hg add a file?

Anyway, dropping flags here until you have a chance to run them against the WPT tests.

Updated

12 days ago
Attachment #8891473 - Flags: review?(bkelly)

Updated

12 days ago
Attachment #8891475 - Flags: review?(bkelly)

Updated

12 days ago
Attachment #8891476 - Flags: review?(bkelly)

Updated

12 days ago
Attachment #8891477 - Flags: review?(bkelly)
(Assignee)

Comment 8

21 hours ago
Created attachment 8899728 [details] [diff] [review]
part 1 - Moving to dom/abort
Attachment #8891473 - Attachment is obsolete: true
Attachment #8899728 - Flags: review?(bkelly)
(Assignee)

Comment 9

20 hours ago
Created attachment 8899729 [details] [diff] [review]
part 2 - renaming
Attachment #8891475 - Attachment is obsolete: true
Attachment #8899729 - Flags: review?(bkelly)
(Assignee)

Comment 10

20 hours ago
Created attachment 8899730 [details] [diff] [review]
part 3 - Removing the following algorithm
Attachment #8891476 - Attachment is obsolete: true
Attachment #8899730 - Flags: review?(bkelly)
(Assignee)

Comment 11

20 hours ago
Created attachment 8899732 [details] [diff] [review]
part 4 - having a separate pref for Fetch + Abort API
Attachment #8899732 - Flags: review?(bkelly)
(Assignee)

Updated

20 hours ago
Attachment #8891477 - Attachment is obsolete: true
(Assignee)

Comment 12

20 hours ago
Created attachment 8899733 [details] [diff] [review]
part 5 - RequestInit.signal must be optional
Attachment #8899733 - Flags: review?(bkelly)
(Assignee)

Comment 13

20 hours ago
Created attachment 8899735 [details] [diff] [review]
part 6 - WPTs

Many tests pass. The missing ones are failing because we don't implement request.signal yet and because we don't abort the blob()/arrayBuffer()/... but only the networking part.

I think we can implement these 2 issues as follow up, maybe disabling abort+fetch in nightly, and just keepting AbortController/AbortSignal as separate api enabled. We have 2 prefs for 2 reasons.

annevk, what is blocking the AbortController/Signal part to land in the fetch API spec?
Flags: needinfo?(annevk)
Attachment #8899735 - Flags: review?(bkelly)
(Reporter)

Comment 14

20 hours ago
Some additional review from Domenic. It's basically done. You can find the PR here:

  https://github.com/whatwg/fetch/pull/523

And a preview of that PR applied to the specification here:

  https://fetch.spec.whatwg.org/branch-snapshots/cancelation/

Hope that helps.
Flags: needinfo?(annevk)
You need to log in before you can comment on or make changes to this bug.