Having thought about it while writing the above, my gut-preference is for method(s) on the request object.

It leaves the door open for cancellation tokens, which provide a better solution for service workers and response methods like response.json(). In the meantime, if developers want to cancel the response, they can use the already-available streaming methods rather than the helper functions like response.json().

jakearchibald on 4 Jan 2017

Re: service workers watching for changes to a fetch:

How do you feel about the event object itself having an addEventListener method?

stuartpb on 4 Jan 2017

When talking about the SW case, do you mean aborting the request sent to the network by the fetch handler due to external reasons?

delapuente on 4 Jan 2017

@stuartpb

How do you feel about event having an addEventListener?

This is for the service worker case right? Yeah, I had the same thought. I dunno if that's possible. But if we need to go down that route it could be some kind of observable thing instead.

jakearchibald on 4 Jan 2017

I think Observables want them, so they may come back to life

Nope. Not going to explore that route closely. I recommend we don't base this API on CancelToken.

benjamingr on 4 Jan 2017

👍2

@delapuente

When talking about the SW case, do you mean aborting the request sent to the network by the fetch handler due to external reasons?

Yeah. For example, I may react to a fetch for /article by fetching 3 different things, /article-start, /article-middle, /article-end. If the user hits the big "stop" button in their browser, or closes the tab, it'd be nice to know that happened and abort the related fetches.

jakearchibald on 4 Jan 2017

👍1

@benjamingr thanks, I'll update the OP.

jakearchibald on 4 Jan 2017

Having thought about it while writing the above, my gut-preference is for method(s) on the request object.

Personally, I'm in favor of that solution and it's also how it's typically implemented in other languages. I don't mind having programmers type a little more if they need abort functionality.

I'm also fine with the solution @jar-ivan suggested:

request(uri, {
  cancel: somePromise
})

Where somePromise fulfilling cancels the request.

benjamingr on 4 Jan 2017

👍9

Okay, so, one thing I think is kind of muddying this design a bit, in terms of solutions that work for SW fetch events and fetch()-initiated requests:

Service workers shouldn't have control over incoming __requests__.

They can do whatever they want with the response the SW produces for the request, of course. If the SW wishes to "abort" an incoming request, it can respond with an appropriate 5XX(?) code denoting "Request Terminated" (and it could also pass some out-of-band signal back to whatever originated the request that would make that entity abort the request).

And of course, a SW can control requests initiated the service worker, using fetch() - that's no different from the non-SW fetch() use case.

But, as far as the incoming request goes, while there's certainly sense in making it possible to monitor the request's status, I think that listening is as far as the abstraction should sensibly go. A request coming into a SW should be effectively read-only, beyond control - the solution should be not just something that can differ from fetch(), it must differ (though the solution for fetch() may take on a superset of the incoming-request-introspection).

Although Service Workers have a privileged position in the browser's plumbing, in terms of design I still mentally model them as, effectively, a local proxy server. You can write a proxy server that stops listening to a request, but you can't make a server that controls the UA making a request.

stuartpb on 4 Jan 2017

I'm also fine with the solution @jar-ivan suggested

This one for reference.

jan-ivar on 4 Jan 2017

👍1

I still dislike mutating the Request with state like cancelation, etc, but I'm willing to accept it at this time. I just wish we had an object that represents the on-going fetch activity. Request is more "a fetch that can be performed" and a Response is "a fetch that has been performed".

If we go with Request.cancel(), please include a Request.cancelled getter determining the current state as well.

Also, should we allow script to tell if a Request is completed and cancel() will effectively be a no-op?

Service workers shouldn't have control over incoming requests.

I'm not sure I understand this assertion. The entire ServiceWorker interception system is to allow it to gain some control over how that incoming request is processed.

wanderview on 4 Jan 2017

👍4

Also, should we allow script to tell if a Request is completed and cancel() will effectively be a no-op?

I'm +1 on both.

benjamingr on 4 Jan 2017

To TL;DR my last comment a bit, we need to recognize that there are four concerns in play here, which I believe will be best served if they're recognized orthogonally:

Monitoring an incoming request (in service workers)
Controlling an outgoing request
Monitoring an incoming response from an outgoing request
Controlling an outgoing response (from an incoming request to a service worker)

Two of these concerns are about monitoring something out of the current code's control, two of these things are about controlling something the current code initiated, two of them are about requests, and two are about responses - but these are four different cases, and an appropriate design for any of these concerns needs to recognize which elements of the case it shares with which of its siblings.

stuartpb on 4 Jan 2017

I'm personally against modifying the Request object, both because it fails to cancel the response (which will surprise people) and because it interacts badly with the various ways in which Requests are copied/cloned.

The other solutions are all trade-offs, but so far I like either the controller pattern or the cancelling promise (@jan-ivar) pattern. The latter has the advantage of working with service workers:

addEventListener('fetch', event => {
  event.respondWith(
    fetch('cat.jpg', {cancel: event.cancelled})
  );
});

taralx on 4 Jan 2017

I agree that modifying the Request object feels very wrong (why not go all the way back to the XHR api...). I kind of like the cancelling promise pattern. It's conceptually simple and easy to understand (well, not sure what I would intuitively expect when the cancel promise is rejected, but other than that...), while providing all the necessary functionality.
It would of course change the fetch API slightly (since now the second parameter can no longer be just a RequestInit dictionary, but instead would need to be some kind of FetchOptions dictionary), but that doesn't seem problematic.

mkruisselbrink on 4 Jan 2017

👍3

I think there's a strong argument for having something that represents the ongoing fetch:

Cancelation should affect the overall fetch, not the request.
Observing cancelation state and done state through getters.
The additional use cases centered around fetches (push, changing priority on the fly, observing progress).

Putting that on Request would get quite ugly. (Especially considering the Cache API and short lifetime of Request objects (fetch() creates a new one from the one it is passed).)

annevk on 5 Jan 2017

👍4

@benjamingr @jan-ivar I've updated the "token"s proposal in the OP to use the promise-based pattern you suggested. Cheers!

jakearchibald on 5 Jan 2017

👍1

@mkruisselbrink when you say "I kind of like the cancelling promise pattern", which pattern are you referring to?

jakearchibald on 5 Jan 2017

I agree with @annevk that having a representation of an ongoing fetch is the least hacky way to do this. However, I don't like the way you end up passing the controller out of the function that creates it:

// eww
let controller;
fetch(url, {
  control(c) {
    controller = c;
  }
});

This brings me back to "Add a method to the returned promise". We could make fetch() return a controller object that subclasses Promise. Its @@species would be Promise, so .then() would return a regular promise.

const controller = fetch(url);
const p = controller.then(response => …);
controller.abort();
p.abort === undefined;

Something like controller.observer could contain the "read-only" parts of the controller. This is where we could provide upload progress, changes to priority etc. You could safely pass this to other pieces of code allowing them to observe the fetch without being able to modify it.

const controller = fetch(url);
// Hand waving through the API a bit…
controller.observer.complete; // a promise that resolves, or rejects with a TypeError (network error) or AbortError (cancelled)
controller.observer.uploadProgress.subscribe(…);
controller.observer.priorityChange.subscribe(…);

This could also appear on the fetchEvent, solving the service worker case:

addEventListener('fetch', event => {
  const catFetch = fetch('cat.jpg');
  event.fetchObserver.complete.catch(err => {
    if (err.name == 'AbortError') catFetch.abort();
  });

  event.respondWith(catFetch);
});

You could even have a helper method to couple a fetch observer to another fetch. So changes to one fetch happen in other(s).

We could ship the controller first, and leave the observable until later, especially if we want to use TC39 observables.

Cons:

This is a pretty big change. One of the points of contention in the previous thread was allowing someone you gave the promise to, to affect the outcome of the promise (eg, aborting it). Whoever you pass the promise to can already lock & drain the response stream, so it isn't really immutable. If you're passing a fetch promise to multiple people and expecting them to all be able to independently handle the response… you're going to have a bad time.

If this becomes a blocker, the return type could switch on fetch(url, {controllable: true}), which is a little hacky, but IMO less hacky than the callback.

jakearchibald on 5 Jan 2017

👍5 ❤1

Passing the controller out of the function that creates it doesn't seem hacky at all to me - certainly no hackier than the way the new Promise constructor already works. It's certainly less hacky than changing the established return type of fetch().

stuartpb on 5 Jan 2017

If revealing constructors bother you so much, we could do something where you can also pre-construct the controller via new FetchController, which you then pass as control (kind of like a request token).

Of course, this makes it so you run into a whole thorny woods of potential mis-uses like attempting to use the same FetchController for multiple fetches. The revealing constructor makes it so that you're guaranteed to only have a 1:1 controller:fetch relationship, much like how resolve and reject are directly tied to their new Promise.

Of course, you could want to design for a world where one controller can control multiple Fetches (it's a use case you described for tokens above), but then you'd have to grapple with all the things that breaking that symmetrical assurance would ruin, like observer.

stuartpb on 5 Jan 2017

@stuartpb I rarely need to pass resolve/reject out of the promise constructor, since the promise-related work is done within the constructor function. That doesn't seem anywhere near as likely here.

we could do something where you can also pre-construct the controller

What are the pros of that vs returning a promise subclass from fetch(), as detailed above?

jakearchibald on 5 Jan 2017

I'm not certain that fetchEvent should have the same kind of observer as an outgoing fetch, since, as I noted earlier, those are fetches in opposite directions, privy to data of opposite natures (for a ServiceWorker, uploadProgress would represent data we're getting/seeing, but to a fetch caller, it represents data we're sending/producing). Maybe two sibling classes that inherit from a common parent.

stuartpb on 5 Jan 2017

If this becomes a blocker, the return type could switch on fetch(url, {controllable: true}), which is a little hacky, but IMO less hacky than the callback.

Sharing my thoughts. I think cancellable promises are not the right way. I would keep Promises as _values-in-time abstractions_ and I would not mess with fetch-specific details.

I always thought fetch API is missing an intermediate type to represent the ongoing operation so I'm more inclined to create this _control_ object once and for all.

To be honest, I really like the new FetchControl() option proposed by @stuartpb It's optional, enables progressive adding of functionality (like observers as @jakearchibald suggested), leverage separation of concerns and encourage dependency inversion.

In the future, it could offer the possibility of controlling multiple requests (to cancel all the fetches involved in the _start/middle/end_ example) and keeps regular fetch mechanisms lightweigh.

Following @wanderview suggestion, I would make .cancel() to return a promise which resolves if the fetch is actually cancelled or rejects with a reason (no op) if cancellation is not possible.

For the _big-stop-button_ case. I'm not sure if we should allow the requests to be continued. I'm happy with providing a way to notificate the SW about this action (or page closing) but perhaps we should stop all ongoing fetches since this is user's will.

In this case, maybe hardstop and allclosed could be functional events.

If I haven't convinced you, I'm more inclined for @stuartpb event.addEventListener() example.

Hope it helps.

delapuente on 5 Jan 2017

IMO aborting a fetch should signal a cancel stopping any upload of data from the request but also preventing the download of data from the response.

For me that's the most important aspect of this, that it immediately stops both upload and download so we are not wasting bandwidth and freeing up the connection. This is super important for devices on slow networks and where bandwidth is metered by usage.

If we only wanted to ignore or error the use of the response then that can be easily done in user land by wrapping the result in an observable. Thus we can do this today without any changes. Of course it would be nice to have a built-in standardized way to do this in the API, but nevertheless it can be done today as is.

So I would hope that it remains a goal of this feature to stop the data transfers of both uploads and downloads, that is the more important goal in my opinion.

jeffbski on 5 Jan 2017

👍5

@delapuente "cancelable promises" are not being suggested here. They've been abandoned. Or are you calling the FetchController promise subclass a "cancelable promise"?

In the subclass proposal, the returned value from fetch() becomes the intermediate type you're wanting.

const controller = fetch(url);
const p = controller.then(response => …);
controller.abort();
p.abort === undefined;

jakearchibald on 5 Jan 2017

@jakearchibald A controller that is also a promise, is magic. I think that's begging for trouble.

I can see SO filling up with questions like:

const controller = fetch(url).then(response => …);
controller.abort(); // Why is abort undefined???

Also, we should be forward-looking and use async functions in examples IMHO. Today I write:

let response = await fetch(...args);

Accessing the magic controller would force me to do this:

let controller = fetch(...args);
let response = await controller;

Which again is easy to get wrong:

let controller = await fetch(...args);
let response = await controller;
controller.abort(); // Why is abort undefined???

To await or not to await? Considering fetch is an asynchronous function, and a controller is a promise, I wouldn't blame anyone for getting it wrong.

jan-ivar on 5 Jan 2017

👍8 👎1

If we really need an object to represent the ongoing operation, this seems the most obvious:

let fetcher = new Fetcher(...args);
let response = await fetcher.fetch();

This separates what's synchronous from what's asynchronous. Same approach as MediaRecorder.

jan-ivar on 5 Jan 2017

👍6

Or are you calling the FetchController promise subclass a "cancelable promise"?

Yes, sorry. I knew cancellable promises were abandoned. It would be a _cancellable thenable_, but that's the kind of functionality I wouldn't mix: control and asynchronous response. I would rather keep them separated preventing also the kind of issues @jan-ivar is talking about.

delapuente on 5 Jan 2017

I'm not convinced that real code is going to drag out the controller all that often. I imagine myself writing this code:

fetch(url, {
  control(controller) {
    cancel_button.then(() => controller.abort());
  }
});

Which does not involve up-lifting the controller's scope.

taralx on 5 Jan 2017

Okay, I've written up a loose, preliminary spec that describes a way to move forward with this, with FetchController objects that contain FetchObserver objects, and an API where you can pre-construct them, or get them via revealing-constructor callbacks, or even mix and match: https://github.com/stuartpb/FetchController-spec

stuartpb on 13 Jan 2017

👍3

Since this topic is still a little contentious I was hoping to schedule a brief meeting to get to know everyone and talk through our options. I created #455 for that purpose which contains a link to arrange logistics. To be clear, that doesn't mean discussion has to stop happening here or that feedback not given in person won't be taken into account. It's just a brief focused chat and I suppose if it's successful we might try more of it.

annevk on 14 Jan 2017

👍1

stuffs about tokens and Observables

@jakearchibald FYI: as the _author_ of an observable library, I liked tokens. They cleaned up my code considerably. However, they are slightly less ergonomic for the users of the library _especially_ once they've been withdrawn from consideration at the TC39. If tokens were used with promises, fetch and async/await, they'd have been ubiquitous. That would mean that using them with observables would never be a big deal because users would usually have easy access to a token.

But since they're not coming for promises in JavaScript, the observable proposal is going to stay with its tried and true form involving subscriptions.

Interests in this issue from a consuming library

I'd like it if it were easy for me to detect/use cancellation on the result of a fetch.
It would be nice if fetch's returned promise cancellation semantic was congruent with other prior art like Bluebird.
I want interop with fetch to be as smooth as possible.

So in the case of RxJS, we have interop with promises all over the place. Pretty much any anything that accepts an observable will also accept a promise. I'd very much like it if I could detect any "special" promises that allowed some form of cancellation (like Bluebird and fetch). It would be ideal if I could find them both with one check.

The idea is that you'd be able to use fetch with RxJS directly like so:

Rx.Observable.interval(1000)
  .switchMap(() => fetch('some/data.json'))
  .subscribe(x => console.log(x))

This would enable RxJS (and possibly other libraries like Angular) to stop supporting XHR-wrapping Observable implementations as rely on fetch, at least in the long run as support improves. That would be a huge win in my opinion.

benlesh on 19 Jan 2017

👍4

I've written a quick demo of what starting/aborting a fetch would look like in preact & vanilla https://gist.github.com/jakearchibald/a87cab7682bebd9fd411c1b84940a7bb

I think @taralx is right, having the controller in a revealing function isn't such a big deal.

jakearchibald on 19 Jan 2017

👍1

From recent discussion & proposals, it looks like the controller and observer should be separate.

const controller = new FetchController();

fetch(url, {
  controller,
  observer(observer) {
    // ...
  }
});

In this model, a single controller could be used by multiple fetches. A controller could observe an observer:

controller.observe(observer);

…meaning it aborts & changes priority along with what it observes.

The observer would be exposed to the service worker:

addEventListener('fetch', event => {
  const controller = new FetchController();
  controller.observe(event.fetchObserver);

  event.respondWith(
    fetch(url, {controller})
  );
});

jakearchibald on 19 Jan 2017

I'm not at all sure what that API is or how it works... At least by glancing at it. Some might say that's a test of my ability to RTFM, but I think it would be best if the API was more obvious to first time readers of the code.

I'd caution against using "observer" as an API name unless it's the same as the observer API from the observable proposal.

benlesh on 20 Jan 2017

👍3

I think the ideal is that fetch would be something I'd never feel obliged to wrap in an observable. This controller API is something I'd definitely want/need to wrap.

I really hope you consider an API like Bluebird's or something libraries like Rx can probe for and handle cleanly without the need to wrap anything manually.

benlesh on 20 Jan 2017

👍4

The meeting is now set over in #455. Tim O'Ryan and anyone else who would like to join please contact me directly.

annevk on 21 Jan 2017

@blesh

I'd caution against using "observer" as an API name unless it's the same as the observer API from the observable proposal.

As far as I know, the fetch API doesn't have any dependencies on DOM APIs, and I believe that's intentional. So yeah, I was thinking actual observers, although that gives us a TC39 dependency which hasn't been great so far. Controlling can ship before observing, though.

jakearchibald on 24 Jan 2017

@blesh

I really hope you consider an API like Bluebird's or something libraries like Rx can probe for and handle cleanly without the need to wrap anything manually.

This is right at the heart of the disagreement.

const fetching = fetch(url);

If I give you fetching, should you be able to alter the in-progress request/response (abort, change priority). Or should I, the one who called fetch(), be the only one who can change the in-progress fetch?

When they make a movie based on this moment in web history, it'll be called "The one who called 'fetch'".

jakearchibald on 24 Jan 2017

😄1

Notes from the meeting today: https://github.com/whatwg/fetch/issues/455#issuecomment-274869169

I'm going to update the OP to reflect this. (i did)

jakearchibald on 24 Jan 2017

👍4

Is it absolutely unacceptable to change the return type of fetch(..)? I don't mean the "adding methods to the promise" stuff, I mean literally changing fetch(..) to return something that's not a promise. Yes, I know this will break code. I'm just asking if it's possible to consider that?

I don't suspect this will considered at this late point, but I figured I'd just ask.

I had suggested in that other "fetch cancelation" thread that fetch() return the fetch-controller, and that this fetch-controller object have a property on it for the promise that's currently the return of fetch(..), as well as the abort() method:

let { abort, pr } = fetch(..);

The ergonomics of using fetch() only for the promise just requires an extra .pr in there, like:

await fetch(..).pr

getify on 25 Jan 2017

Is it absolutely unacceptable to change the return type of fetch(..)?

Not at all, we can avoid breaking code and just return an object which is a promise that has a cancel method but no chaining at all (so the capability can be further separated). In my view, this is similar to your idea except the object returned also has a then.

The consensus in the meeting was that since controller is a very unobtrusive API - it will give people the capability to cancel fetch requests but does not really limit exploring other options (like tokens, or cancellable promises) in the future.

The consensus was basically to go ahead with the controller option and to observe how people interact with it - if people start wrapping fetch and adding .cancel on it - the spec can be amended to add .cancel in an ad-hoc way. A controller also means finer grained control than just a .cancel.

benjamingr on 25 Jan 2017

an object which is a promise that has a cancel method but no chaining at all...

... also has a then.

What does it mean for an object (promise?) to have a then(..) method but not allow chaining? Do you mean it would be a then(..) that returned undefined instead of another promise?

Sounds pretty troublesome to conceive of a "thenable" that doesn't actually conform to A+ semantics for thenables -- remember we sold ourselves to thenable duck typing.

getify on 25 Jan 2017

@getify it would return a standard promise, but that promise in fact would not have a .cancel so cancellation is on the original fetch:

var request = fetch("/foo");
var data = request.then(x => x.json());
request.cancel(); // this aborts the request
data.cancel(); // TypeError: data.cancel is not a function

Again, this is just a proposal for extension in the future - and mainly to demonstrate that we can keep the controller API and built on top of it in the future.

benjamingr on 25 Jan 2017

👎4 👍3

@getify this is covered in the meeting notes https://github.com/whatwg/fetch/issues/455#issuecomment-274869169

If subclassing/extending the return type of fetch() so it has a controller methods is unacceptable (I recall you were one of the people saying it was unacceptable), then I don't see how returning a thennable or an object with a promise property is acceptable, unless the original complaint was purely academic.

jakearchibald on 25 Jan 2017

👍1

Ahh, the subclassed-promise approach. I read about that earlier in this thread, and it doesn't seem to satisfy the original concerns (from the other thread) that people will share that promise (which has a cancel() on it) and thus share (perhaps accidentally) the cancelation capability in a way that will create surprising action-at-a-distance -- that is, that 3 different consumers of the same promise can be affected by only one of them deciding to cancel it.

Promises are already strongly established in developer habits, especially with respect to sharing promises directly and liberally, so I see this is a common and likely scenario.

If you have a controller with a promise+cancel on it, and people share the controller, this same action-a-distance can happen. But I think this is less likely to accidentally occur because this new controller object wouldn't be a promise itself and thus wouldn't have any precedent around liberal sharing -- it wouldn't automatically be recognizable as a thenable, wouldn't be usable with await, etc.

It seems like people would be more likely to only share the .pr from the controller rather than sharing the controller itself, thus reducing the chances of accidentally sharing the cancelation capability.

getify on 25 Jan 2017

3 different consumers of the same promise can be affected by only one of them deciding to cancel it.

Right, but three consumers of the same resolved value (a response) can influence each other. Only one can consume the stream. The promises can't be shared liberally without some thought.

Also, I don't see how this is an unacceptable breaking change, whereas breaking all current code using fetch is acceptable.

I really don't think we need to retread this ground here, it's all in the other thread.

jakearchibald on 25 Jan 2017

I don't see how this is an unacceptable breaking change, whereas breaking all current code using fetch is acceptable.

What I would find "more acceptable" is that existing code had to change so that developers had to decide intentionally if they want to share the cancel capability explicitly or not. Affirmatively deciding on that point is better for design/architecture of code than just having things silently and accidentally permeate. I do understand the pain of breaking existing code. But sometimes pain leads to better code.

Extending the existing promise that's already being shared around by a lot of code is "pit of failure" design IMO because it means that the cancelation capability is now being shared automatically, without any code changes. That means that of all the consumers of a promise from fetch(..), only one of them has to decide it wants to act extra in doing a cancelation, and all the other consumers of that promise are, without any code change, now broken/affected.

I really don't think we need to retread this ground here, it's all in the other thread.

OK, fine. I'll drop it. But in my reading of this thread, I didn't see anyone actually addressing the concerns I have here. That's why I spoke up.

getify on 25 Jan 2017

We have to remain backwards compatible and cannot require existing code to change. That's not an option unfortunately. I suggest raising a new issue at https://github.com/whatwg/meta/issues/new if you think that's worth debating further as this is not the place.

annevk on 25 Jan 2017

👍1

We also could introduce a second method name who's only difference is the return type:

let pr = fetch() 
let {pr, cancel} = fetchCancel()

getify on 25 Jan 2017

@getify

The entire reason we all like the controller approach so much is that it lets TC39 hash out cancellation (as a concept) in the language and decide on how APIs with cancellation should look like while providing a way to abort a fetch which is future compatible to API changes with cancellation.

It provides a simple extensible solution for a problem developers have with the current API.

The meeting consensus wasn't against or for different methods or subclassing promises (we were actually mostly against that) or .cancel or destructuring. It was to decide on a simple extensible API (controller) and consider adding these other things later - depending on how/if people wrap .fetch and how the language settles cancellation in the future.

benjamingr on 25 Jan 2017

👍4

There are a lot of things we could do. A lot of people have strongly-held opinions about controlling fetch that are directly incompatible. Whatever we decide, it won't be "the way" someone wants, and they may be unhappy about that. However, if we continue to ship nothing to solve this issue, everyone is unhappy.

After we ship a solution, if you're one of the unhappy ones, thanks for taking one for the team. 😄

jakearchibald on 25 Jan 2017

👍3

I really appreciate @annevk's coverage of different things about fetch that need representing:

The additional use cases centered around fetches (push, changing priority on the fly, observing progress).

I wasn't expecting cancellation discussion to have any bearing on these capabilities, & was piqued and excited to see observers here, and particularly the diversity of use cases for observers. In particular, detecting push is a very important issue to me, one tracked in #65. That this issue could be the basis for this super important capability was an unexpected & great thing to read. :+1:

rektide on 25 Jan 2017

The push case is really interesting as it would (I imagine) yield a request & response. Given that a response can be consumed, does it fit with the "observing" model?

The current proposal suggests that a fetch observer can exist in multiple processes at once (page & service worker). If each is given a response object, what happens if one consumes it? We may have to create a clone per process here. I don't think a single body can exist in multiple processes, currently we hand-off between the client & service worker via fetch() or respondWith().

Also, in general, what does it mean to consume a push response? Does that take it out of the push cache (whereever that is)?

jakearchibald on 25 Jan 2017

@jakearchibald my initial thoughts were that one would:

get notification of a PUSHed resource's url (& maybe headers) via observer.
create a fetch to consume that resource, which would pull from the registry. (This would still have to work for streaming data.)

Rather than the observer consuming the fetch, the observer is merely signalling that there is a PUSH frame whose stream identifier corresponds to the original Fetch's stream id. This is all just how I envisioned fulfilment of the original ask in #65 & it's predecessor, & I'm not sure if I'm on the mark or no, but here's the original's wording-

I'd be nice if there was an ~observer API that allowed an application to be notified when new fetch requests are added to the registry

rektide on 25 Jan 2017

re: naming FetchObserver in a way so as to not be entangled with TC39 Observables etc, what about calling it FetchReporter? (I was thinking of FetchEventSource to denote that the API is addEventListener, but I could see that being confusing for ServiceWorker where there's an actual source of events called fetch.)

stuartpb on 25 Jan 2017

👍1

3 different consumers of the same promise can be affected by only one of them deciding to cancel it.

This would only be true if someone composed all of the cancellation through in this manner. It rarely makes sense to compose cancelation through in this way when multicasting. Any cancellation of a multicast type (like a promise) would require refCounting. Which would live internally in that returned promise.

This would easily be the most ergonomic solution for everyone. It's also a solution that's detectable from a single passed value, which is useful for libraries like Rx and others.

benlesh on 25 Jan 2017

@blesh this kind of ref-counting isn't off the table forever, but it's not what we're pursuing now. (see the previous threads for discussion of this - it was my preferred solution if we were going to get proper concealable promises)

jakearchibald on 27 Jan 2017

@rektide

get notification of a PUSHed resource's url (& maybe headers) via observer.

create a fetch to consume that resource, which would pull from the registry. (This would still have to work for streaming data.)

I don't think using fetch() would work well here, as it'd be difficult to tell the difference between a push-cache hit or miss. However, I think the general idea is good…

observePushSomehow(async event => {
  console.log(event.request);
  const response = await claimResponse();
});

…where claimResponse() would reject if it'd already been claimed. This would remove it from the push-cache too. Unclaimed responses would remain in the push-cache.

Still unsure if this sits with the observer, though.

jakearchibald on 27 Jan 2017

Maybe the service worker shouldn't get a fetch observer. That could be over-complicating the design.

Eg:

addEventListener('fetch', event => {
  console.log(event.fetchObserver);
  event.respondWith(
    fetch(event.request, {
      observe(observer) {
        // …
      }
    })
  );
});

Would I get response progress events from both event.fetchObserver and observer? If I get a push, does that appear in both?

If I don't call respondWith, will event.fetchObserver continue to give me events?

If these questions become sticking points, the service worker could receive something limited, like a "fetch signaler", which doesn't offer response progress events, or h2-push events.

jakearchibald on 27 Jan 2017

(Note that I edited this comment for clarity several times after I originally posted it, so if you read it via email, you should probably read it again.)

Would I get response progress events from both event.fetchObserver and observer?

Yes, but you probably wouldn't be subscribing to both sets of events (or, if you are, you'll be handling them two separate ways) - you'd either be listening to the client's view of the fetch (the event.fetchObserver), or the ServiceWorker's view of the fetch (the one that's attached to the call to fetch). There is a distinction, ie. if I'm choosing to make a fetch within a fetch event handler and then modify the response that I'm piping back to the client (ie. the outside caller that initiated the event in the ServiceWorker).

As I noted above, I'm not against the idea of ServiceWorker events having a different kind of observer - as they are, again, fundamentally different kinds of observation - but I don't think it should be a merely limited version of the other kind of observer.

If I get a push, does that appear in both?

I think push would be its own ServiceWorker event, similar to fetch: in other words, it would look largely like a fetch (or, rather, the then callback of a fetch), but one initiated by the server.

That's based on my understanding of HTTP2 Server Push, where pushes aren't inherently tied to any request. If I'm misunderstanding something and they are, like if they're modeled as some kind of subresource of the main response stream, then this'd need some revisiting, but I just checked, and it looks like they're regarded as being effectively independent of any request, so having push-handling be a separate event on ServiceWorkers seems like it'd be the natural design here.

If I don't call respondWith, will event.fetchObserver continue to give me events?

Uh, if you don't call respondWith, the fetch isn't getting a response, right? In that case, I imagine it'd look the same way as when a server isn't providing a response to a fetch, ie. the only events you'll get will be of an abort or error nature.

stuartpb on 27 Jan 2017

Would I get response progress events from both event.fetchObserver and observer? If I get a push, does that appear in both?

Why is there an event.fetchObserver? I mean, when the request is sent through the SW, there is not fetch in progress yet except for the navigation preload request. Is that .fetchObserver associated to the navigation preload?

delapuente on 28 Jan 2017

Is that .fetchObserver associated to the navigation preload?

I'm not sure what you mean by "navigation preload", but the fetch that's being observed by event.fetchObserver (in the way I'm modeling it) is the "fetch" that whatever dispatched the request is seeing. So, for instance, when a script on the page calls fetch(), or the UA dispatches a request for style.css based on a <link rel="stylesheet"> in the page, from those two points of view, they have "initiated a fetch", even if there may not be any such fetch on the wire: they're dispatching a fetch to the ServiceWorker, who may then respond either with a fetch on the wire, or a fetch from cache (or a generative response from a function, or something like that).

stuartpb on 28 Jan 2017

Oh, got it. Thank you. I'll give this problem a thought.

By the way, I was referring to this navigation preload.

delapuente on 28 Jan 2017

@jakearchibald is the goal of fetch to be a primitive everyone will wrap? Or is the goal that it be more ergonomic enough that everyone will use it OOTB and not wrap it like they do with XHR2? Perhaps I'm just misunderstanding the goals here.

benlesh on 29 Jan 2017

@blesh that's an over-simplification of the problem, as you know. People wanting fetch to see totally different will wrap it. Different wrappings are incompatible. See https://github.com/whatwg/fetch/issues/447#issuecomment-275123525

jakearchibald on 29 Jan 2017

@blesh to clarify, it should be clear from https://github.com/whatwg/fetch/issues/447#issuecomment-273781662 that the usage of the fetch API without a wrapper is heavily guiding the design here.

There are lots of higher-level patterns that their champions believe to be "the one true way", and they're fundamentally incompatible, so we cannot be all of them out-of-the-box.

jakearchibald on 29 Jan 2017

A salient idea I just had on why the control argument to fetch should take a pre-constructed object in addition to taking revealing constructor callbacks: this would allow for the future introduction of extended FetchController objects, such as natively-implementation-based fetch-control behaviors.

For instance, as Anne noted in #448, a fetch that intends to propagate the status from an upstream fetch in a Service Worker would need to have that propagation performed via JS-thread mediation, introducing an unnecessary amount of message-passing overhead. If the observe argument is specced to accept externally-constructed objects, there could then be some alternative object specified down the line that signals that control is derived as such, allowing the propagation to take place without touching the JS thread, with something like new FetchFollowingController(evt.fetchReporter) (this also addresses the question of making it easier to follow an existing fetch's events, as raised in the meeting notes).

stuartpb on 1 Feb 2017

To more pithily convey the sort of thing what I'm describing would allow, riffing off of Jake's example code from earlier:

addEventListener('fetch', event => {
  event.respondWith(
    fetch(event.request, {
      /* the FetchFollowingController constructor here
         can be polyfilled, and can ALSO be implemented natively,
         in which case abort propagation can happen
         behind the scenes and not have to block on JS */
      control: new FetchFollowingController(event.fetchObserver)
    });
  );
});

stuartpb on 1 Feb 2017

@blesh to clarify, it should be clear from #447 (comment) that the usage of the fetch API without a wrapper is heavily guiding the design here.

@jakearchibald anyone that wants composition with cancellation will _definitely_ need to wrap fetch with this design. Since the future to the result is not linked to that which can abort the future in any way, there's no way to return both together from any function as a single object unless they're combined via some common wrapper into a more composable shape, refCounting is added, etc. For reactive programming, this will be a huge bummer. Perhaps there is a misunderstanding of how it could be implemented with ref counting? I don't know.

Outside of that, I think this design is very unergonomic and unlike anything I've seen in the ecosystem. I understand the controller is for cancellation, but at first blush I have no idea what a "fetchObserver" is (I presume it's a reference to a single subscriber to the fetch resulting promise? It's what it sounds like. and wouldn't you have more that one?) But that's anecdotal and totally subjective.

benlesh on 21 Feb 2017

I understand the controller is for cancellation, but at first blush I have no idea what a "fetchObserver" is

The controller is for making to changes, and the observer is for observing changes. So:

"I want to change the priority of this fetch" - method on the controller.
"Is the state of this fetch completed/abort/in progress?" - method on the observer.
"Has this fetch changed priority?" - listener on the observer.
"Has this fetch been aborted?" - listener on the observer.
"I want to abort this fetch" - method on the controller.
"I want to hear about upload progress" - listener on the observer.

The intent here is to provide means of observing fetches without being able to control them. Developers have asked for this distinction, and it's also needed within a service worker.

jakearchibald on 21 Feb 2017

👍2

Ok, here goes. Here's a proposal based on @stuartpb's work and other discussion. It's formed of three parts:

A controller - allows me to say what I want to happen.
A signal - expresses what should happen.
An observer - what's happening & what actually happened.

Fetch controller

[Constructor(), Exposed=(Window,Worker)]
interface FetchController {
  readonly attribute FetchSignal signal;

  void setPriority(octet priority);
  void abort();
};

dictionary RequestInit {
  // …
  FetchSignal signal;
}

[Exposed=(Window,Worker)]
interface FetchSignal : EventTarget {
  readonly attribute octet priority;
  readonly attribute boolean aborted;

  attribute EventHandler onabort;
  attribute EventHandler onprioritychange;
}

The fetch controller allows you to signal your intent to fetches.

const controller = new FetchController();
const signal = controller.signal;

fetch(url, {signal}).then(r => r.json()).catch(err => {
  if (err.name == 'AbortError') {
    console.log('The request/response was aborted');
  }
});

// sometime later…
controller.abort();

A signal can be assigned to many fetches, meaning you can control many fetches at once.

const controller = new FetchController();
const signal = controller.signal;

abortButton.addEventListener('click', () => {
  controller.abort();
}, {once: true});

startSpinner();

fetch('story.json', {signal}).then(async response => {
  const data = await response.json();
  // Fetch all the chapters
  const texts = data.chapterURLs.map(url =>
    fetch(url, {signal}).then(r => r.text())
  );

  // Add the chapters to the page as they arrive
  for await (const text of texts) {
    addToPage(text);
  }
}).catch(err => {
  if (err.name == 'AbortError') return;
  showErrorMessage();
}).finally(() => {
  stopSpinner();
});

Fetch will read the current state from the signal, meaning this fetch will reject with an AbortError without issuing a request.

const controller = new FetchController();
const signal = controller.signal;
controller.abort();

fetch(url, {signal});

Having the signal as a separate object means you can pass the signal around without also giving the ability to control.

Fetch observer

[Exposed=(Window,Worker)]
interface FetchObserver : EventTarget {
  readonly attribute octet priority;
  readonly attribute FetchState state;

  // Events
  attribute EventHandler onstatechange;
  attribute EventHandler onprioritychange;
  attribute EventHandler onrequestprogress;
  attribute EventHandler onresponseprogress;
};

dictionary RequestInit {
  // …
  ObserverCallback observe;
}

callback ObserverCallback void (FetchObserver observer);

enum FetchState {
  // Pending states
  "requesting", "responding",
  // Final states
  "aborted", "errored", "complete"
};

[Constructor(TODO), Exposed=(Window,Worker)]
interface FetchProgressEvent : ProgressEvent {
  // Maybe we don't need anything else. Just move ProgressEvent to the fetch spec.
}

This lets you observe an ongoing fetch.

fetch(url, {
  observe(observer) {
    observer.addEventListener('responseprogress', event => {
      if (!lengthComputable) return;
      progressEl.max = event.total;
      progressEl.value = event.loaded;
    });
  }
});

Within a service worker

partial interface FetchEvent {
  readonly attribute FetchSignal signal;
};

This allows you to react to signals from the client.

addEventListener('fetch', event => {
  event.respondWith(async function() {
    const texts = [url1, url2, url3].map(url => 
      fetch(url, {signal: event.signal}).then(r => r.text())
    );

    const fullText = (await Promise.all(texts)).join('');

    return Response(fullText);
  }());
});

By passing the signal to all fetches, they'll all abort if the page signals to abort.

fetchEvent.request will already be associated with event.signal, meaning:

// This will abort if event.fetchSignal signals to abort.
fetch(event.request);

// This does not follow event.fetchSignal.
fetch(event.request, {signal: undefined});

Signals elsewhere

I want to think a bit more about it, but it feels like caches.match and cache.match should also be able to take a fetch signal. Priority may not have an impact here, but the ability to abort is useful.

jakearchibald on 22 Feb 2017

👍3

Only about half-way through reading, but looks good so far. One thing I'm not seeing, not sure if I missed it: do signals have an addEventListener, so I can follow a signal even without it being attached to a fetch (ie. if I want to follow it for some kind of bespoke "virtual fetch"-like construction)?

stuartpb on 22 Feb 2017

Okay, I see it has a couple events in the form of onwhatever, I take it the addEventListener is implied.

stuartpb on 22 Feb 2017

@stuartpb apologies, I forgot to inherit from EventTarget there. Yes, assume anything with an EventHandler also has addEventListener.

jakearchibald on 22 Feb 2017

Why do the accessors return promises instead of values? What is the resolution behavior of those promises?

taralx on 22 Feb 2017

One thing that's not exactly clear here: what happens when I .follow multiple signals? Does the new one replace the old one? Does it follow both? What if future extensions to fetch control introduce conditions where two sets of signals can't control the same fetch (ie. say there's a "pause whenever" priority level that pauses the fetch when resources are tight, and there's a "consciously wake" event that only applies when the fetch is in that state)?

stuartpb on 22 Feb 2017

The signal uses promises because it might be in a different thread to the controller, eg in the service worker case. Because of this, we may as well make signals transferrable.

They're promises on the fetch observer because the data comes from the network thread.

jakearchibald on 22 Feb 2017

Observation will nearly always be on a different thread or in a different process from where network processing is actually happening. We can't provide sync access to that data.

wanderview on 22 Feb 2017

I still don't see how network state being on another thread means that the JS thread needs to hit the event loop before it can read it.

stuartpb on 22 Feb 2017

At what point does .follow check if the signals form a cycle - when the signal is followed, or whenever it propagates?

stuartpb on 22 Feb 2017

@stuartpb the browser may be adjusting priority as the user scrolls the page - finding out about that in the service worker would have to be async, no?

jakearchibald on 22 Feb 2017

@stuartpb when does a cycle get problematic? It would just restate the same value, no?

jakearchibald on 22 Feb 2017

the browser may be adjusting priority as the user scrolls the page - finding out about that in the service worker would have to be async, no?

Oh, CHANGES in priority have to be async, of course. Those are events.

The key point that I'm trying to make is that that doesn't mean the scripting thread needs a guarantee that it'll have seen the priority-change event before reading the changed priority. I mean, the network thread could also change priority while we're handling the event for the last change, in which case we're just slowing things down so we can have stale data.

stuartpb on 22 Feb 2017

when does a cycle get problematic? It would just restate the same value, no?

So signals don't propagate any state update that matches the last state update they had? That's an important point to state - I don't think I read it in your proposal. And I thought we were saying that reading a signal's state had to be async - is it making this check asynchronously before it makes the decision to propagate?

stuartpb on 22 Feb 2017

Like, yeah, if the changes are async, then it seems like it should be possible to land signals in an infinite loop of propagating back-and-forth priority changes.

stuartpb on 22 Feb 2017

Yeah, I don't think change events should fire unless a change has occurred.

I guess that means holding state internally that could be exposed synchronously, I'm just worried that it might create race conditions.

I'm hoping to guarantee that if you getState, then add a listener in the promise reaction, you haven't missed anything.

jakearchibald on 22 Feb 2017

I think the thing that's not adding up for me is that .follow seems to imply that the fetch being controlled will adopt the state of the signal, but there's also control that can put it out of sync with the signal it's following (including following a second signal). Like, I can't put my finger on it, but it feels like somewhere in this formulation is something relying on an assumption of consistency that's not guaranteed to be there.

stuartpb on 22 Feb 2017

Like... Signal C is following Signal B, and Signal B is following Signal A. Signal B changes to Priority 2. Signal C changes to Priority 3. Signal A changes to Priority 2. Signal C does not change to Priority 2, because the propagation gets blocked at Signal B, which already changed to Priority 2.

stuartpb on 22 Feb 2017

I know this is still under discussion, but gecko bug to start working on underlying bits:

https://bugzilla.mozilla.org/show_bug.cgi?id=1341738

wanderview on 22 Feb 2017

@stuartpb I forgot to reply to your earlier question. Yes, my intention was to allow a controller to follow multiple signals.

Code for your example, in case anyone finds it easier to follow (like me):

const controllerA = new FetchController();
const controllerB = new FetchController();
const controllerC = new FetchController();

controllerC.follow(controllerB.signal);
controllerB.follow(controllerA.signal);

controllerB.setPriority(2);
controllerC.setPriority(3);
controllerA.setPriority(2);

// controllerA priority: 2
// controllerB priority: 2
// controllerC priority: 3

Yeah, I can see what you're saying. I'm not sure how much of an issue it is in practice though. Hmm.

jakearchibald on 23 Feb 2017

Thinking about the sync/async thing again, I think @stuartpb's right. If the object has an event listener, we don't need async getters, as the value can be set as part of the listener.

All I want to ensure is that:

console.log(fetchObserver.state);

fetchObserver.addEventListener('statechange' => {
  console.log(fetchObserver.state);
});

…means a state isn't missed between the two logs. But if tasks are queued, it should just work.

@annevk does this make sense?

Update: @annevk confirmed this in IRC. I've updated the proposal.

jakearchibald on 23 Feb 2017

👍2

How useful is the separation of controller & signal? They could be rolled into one, simplifying the API. The only thing you'd lose is the ability to pass the signal around without the controller.

How this would look in a page:

const controller = new FetchController();

fetch(url, {controller});

And in a service worker:

addEventListener('fetch', event => {
  event.respondWith(async function () {

    const texts = [url1, url2, url3].map(url =>
      fetch(url, { controller: event.controller }).then(r => r.text())
    );

    const fullText = (await Promise.all(texts)).join('');

    return Response(fullText);
  }());
});

Would need to figure out how the controller worked in both the service worker & client. It could be a new controller created using an internal signal, or it could represent the same controller.

We can probably drop follow & unfollow in this model.

jakearchibald on 23 Feb 2017

If the object has an event listener, we don't need async getters, as the value can be set as part of the listener.

Yeah, that's one way I was thinking about it - this would be the most simple / straightforward approach, as it would give JS a guarantee of consistency (everything in the same sync block is going to get the same values for a fetch's priority). It would, however, be sacrificing some degree of realtime accuracy.

The other way to do it would have it be like performance and/or MutationObserver, where the value is always read directly from the live state (a la performance.now() or MutationObserver's takeRecords()), and when the change callback / event fires, it aggregates all changes that happened between the last instance of the callback and now (a la MutationObserver's observe()).

This is a bit of a complication to handle what essentially amounts to edge cases (and a queue-draining behavior like takeRecords() may not be desirable or necessary, versus a buffer / only-the-latest-state approach like how Performance does it), but it would be robust, and it would be equally consistent.

stuartpb on 23 Feb 2017

Thinking about the sync/async thing again, I think @stuartpb's right. If the object has an event listener, we don't need async getters, as the value can be set as part of the listener.

Actually, doing this has some implications for the implementation. With the async getter and no event listener we can actually avoid queueing runnables to the event loop. With a sync getter we must queue runnables for every change of these values whether an event listener is registered or not.

Edit: The implication being that making the getters sync reduces browser's ability to optimize.

wanderview on 23 Feb 2017

With the async getter and no event listener we can actually avoid queueing runnables to the event loop. With a sync getter we must queue runnables for every change of these values whether an event listener is registered or not.

You can still skip it if the fetch doesn't have an Observer attached at all, which I'd think would be more common than an Observer that's attached with no event listener.

stuartpb on 23 Feb 2017

You can still skip it if the fetch doesn't have an Observer attached at all.

Sure, but if someone is listening for priority changes (rare), we would still have to queue runnables for every state change (multiple per fetch).

The other way to do it would have it be like performance and/or MutationObserver, where the value is always read directly from the live state (a la performance.now() or MutationObserver's takeRecords()), and when the change callback / event fires, it aggregates all changes that happened between the last instance of the callback and now (a la MutationObserver's observe()).

Those APIs do not have to deal with cross-thread data AFAIK.

wanderview on 23 Feb 2017

Anyway, we can still do the sync getters. Just want to make sure people understand the implications.

wanderview on 23 Feb 2017

Those APIs do not have to deal with cross-thread data AFAIK.

Anyway, we can still do the sync getters. Just want to make sure people understand the implications.

Yeah, so there'd maybe have to be a, what, hundred-odd cycle hitch to wait for a mutex in the event that we're reading the state as a new state is being written? I can live with that.

stuartpb on 23 Feb 2017

Not saying we can't make them sync, but just be aware of the implications. Browsers are doing work to move more network operations off-main-thread and this would require dispatching many runnables back to the main thread. Maybe thats fine, but lets not ignore it.

wanderview on 23 Feb 2017

Also, individual state events would be nice - and (if I understand @wanderview correctly) they would let the implementation dispatch fewer runnables.

this would require dispatching many runnables back to the main thread.

Well, not necessarily. Like, one of the reasons I was describing aggregating all state change into one callback is that it would let many successive changes ride the same runnable. Also, we could have MutationObserver-style filters, so as to tell the network thread "you don't have to notify JS about anything but this one kind of change". (This would also be the same kind of filtering that'd be involved to implement individual-kind-of-state-arrival events, I think, no?)

stuartpb on 23 Feb 2017

@wanderview with the only (non-)alternative being lazy getters that block script execution, right? I can see how we might want a design that does less message passing.

annevk on 23 Feb 2017

@annevk I'd like to see attribute EventHandler onpush added to FetchObserver (or a PushObserver added, if something makes FetchObserver unsuitable).

@stuartp: I think there's some misunderstandings about HTTP/2 Push, based on your comment where you said:

That's based on my understanding of HTTP2 Server Push, where pushes aren't inherently tied to any request. If I'm misunderstanding something and they are, like if they're modeled as some kind of subresource of the main response stream, then this'd need some revisiting, but I just checked, and it looks like they're regarded as being effectively independent of any request, so having push-handling be a separate event on ServiceWorkers seems like it'd be the natural design here.

In contrast, section 8.2.1 Push Requests of RFC7540 says:

Pushed responses are always associated with an explicit request the client. The PUSH_PROMISE frames sent by the server are sent on that explicit request's stream. The PUSH_PROMISE frame also includes a promised stream identifier, chosen from the stream identifiers available to the server (see Section 5.1.1).

So, Push responses are definitely sent in association with a specific client request.

This is used, for example, in Generic Event Delivery using HTTP Push (aka webpush), where a client POSTs to a /subscribe endpoint, which stays open, and expects as long as that connection is open that it can get push responses sent to it associated with that specific outstanding POST. See 4. Subscribing for Push Messages for this example use case. By associating each push with a request, Webpush can allow for a single HTTP2 connection to signal and route deliveries of messages from many simultaneous /subscribe requests to the appropriate subscriber. If there were no association between request and push, Webpush would be unable to figure out which subscriber to send a pushed message to.

Would greatly appreciate making sure we don't accidentally block the possibility of progress on #51. #51 was opened by @martinthomson, author of Webpush Protocol.

rektide on 24 Feb 2017

@rektide the difference with push is that reading a response has side-effects. It has the usual stream-based effects, but also a listener will have to explicitly claim the pushed response, which pulls it out of the H2 cache, through the HTTP cache, and into the listener.

If we're happy with giving observers the ability to claim pushed resources, it can be a listener on the fetch observer.

jakearchibald on 24 Feb 2017

Note that my username is @stuartpb; @stuartp is Stuart Park, who was last seen on GitHub in 2011 and has yet to comment on this thread.

As for claiming pushes off a fetch... looking at a summary of the spec again (though I clearly haven't had a great batting average basing my suggestions on this so far), it looks like pushes only come in after headers have been received, which is when fetch() resolves, correct? So, wouldn't it make sense for a push handler to get attached to the response of a resolved fetch, like, say, via response.addEventListener("push"), where every event coming into that has properties that look something like the components of a fetch (ie. a response object to call .clone() or .arrayBuffer() on, and a Request-like object describing the resource being pushed)?

Then, if you want to, say, pass along the response from a fetch, but handle pushes, without passing them along, you'd do something like event.respondWith(response => (response.addEventListener("push", pushHandler), response)). To only selectively claim some pushes, you could have the push event handler have some kind of behavior where either the push event has a respondWith-like method that lets you put the push "back in the pipe" to the next push handler (ie, any push handler attached to the response in the code receiving the response you passed to respondWith), or (better, IMO) you could just have the handler not claim and/or return the push coming in, and it'd get passed back into the pipe naturally.

As for instantiating push responses to the fetch coming into the ServiceWorker... would it break anything to change the return type of event.respondWith from void to some kind of object that allows the ServiceWorker to instantiate pushes attached to the response (in addition to the pushes that would be coming in as part of the fetch response)?

stuartpb on 24 Feb 2017

So, wouldn't it make sense for a push handler to get attached to the response of a resolved fetch, like, say, via response.addEventListener("push")

Also, as something of an aside - and I know this ship, to a degree, has sailed, what with ServiceWorkers already being out in the wild and all - but I think this kind of event handler, where having one attached makes it the canonical insertion point for handling all the types of an event, replacing any existing browser plumbing (as opposed to the way .addEventListener has traditionally worked, with event.preventDefault() being something you'd have to opt into), should have been put under a different name than addEventListener, like addEventHandler, to denote that it's not merely listening to events, but actively controlling them.

Would it maybe be possible to introduce that as an alternative name for setting these kinds of listeners, and deprecating the use of addEventListener for that purpose, the way that pseudo-element selectors in CSS were originally spelled with a single colon (like :after), the same way as pseudo-classes (like :active), and were later redefined to use a double colon (ie ::after)?

Maybe these addEventHandler handlers could even, instead of just being the same kind of function attached via a different name, have changed semantics (which could potentially be reused for other kinds of events in the future) where their return value (either explicitly or implicitly when the handler returns void) is the event, which will get passed, with modifications, to any next event handlers in sequence, as alluded to with

you could just have the handler not claim and/or return the push coming in, and it'd get passed back into the pipe naturally.

?

stuartpb on 24 Feb 2017

That's not how service worker works. You can add an event without changing behaviour. You don't influence fetch unless you call respondWith, which is like preventDefault but provides an answer.

jakearchibald on 24 Feb 2017

Thanks Jake & Stuart, I'd frankly forgotten that need to "claim" a push request which ya'll did a good job discussing this time & last. My apologies for neglecting that important bit, and also apologies for getting your username wrong @stuartpb.

If we're happy with giving observers the ability to claim pushed resources, it can be a listener on the fetch observer.

I don't have strong feels for what the right API for handling push is. I more know that I really want _something_ that can tell me about PUSHes sent in association with a fetch, and am happy helping however I can to get to that end goal. This approach above sounds totally fine, sound, and sensible to me!

rektide on 24 Feb 2017

@jakearchibald if we simplify the API and drop signals, will we drop the .follow() method?

delapuente on 25 Feb 2017

Yep, see https://github.com/whatwg/fetch/issues/447#issuecomment-281974648

jakearchibald on 25 Feb 2017

it looks like pushes only come in after headers have been received, which is when fetch() resolves, correct?

I'm pretty sure that's not true. You can start pushing the moment you have an open connection so a push frames can arrive before a response header frame.

annevk on 25 Feb 2017

@annevk, that's not quite right, though your conclusion is spot on. A push is always associated with another request. The push can be started as soon as the first octets of the request arrive. I suppose that if you model a request/response pair as a connection, then it might look like that, but generally you have some part of a request inbound before the push can be triggered.

That does mean that pushes can appear before you have a response, definitely. You have to have that so that you know not to make another request.

martinthomson on 27 Feb 2017

Does the observing pattern being discussed here allow for the following use case?

Some applications that use service workers are online-first, but they might still want to handle flaky/poor internet connections using offline capabilities.

Currently, you can accomplish this using streams (like this: https://jakearchibald.com/2016/streams-ftw/). The idea is to call fetch, and once you get the response back start reading the response body. If you don't get enough of the data within a certain time period, you decide the internet connection isn't good enough and respond from cache instead or redirect to a url that serves a cached response.

It would be really nice if this spec accounted for this use case, as using streams feels too low-level and somewhat hacky.

The main requirements to make this possible are:

having well-defined progress events that allow you to see what % of the data you've received (this seems to be proposed already in Jake's comment
somewhat granular progress events and/or having the ability to specify how granular your progress events are. For example, if you get progress events every 5% of the way then you can detect a poor connection much faster/more accurately than if you only get progress events every 25% of the way.

aaronsn on 20 Mar 2017

@aaronsn

I wouldn't expect progress events to be throttled more than stream reading, and if they are it'd only be because they're happening very fast, eg many per display frame.

Are you aware of a progress events implementation (in XHR) that is problematic for this use case?

jakearchibald on 21 Mar 2017

@jakearchibald I actually didn't know about progress events in XHR. I think as long as this did something similar it would be fine. Just wanted to make sure this use case was accounted for.

aaronsn on 21 Mar 2017

I just landed FetchController, FetchSignal and FetchObserver in mozilla-inbound, disabled by pref.
The implementation follows comment 28, but without the 'priority' support.

We need to define the type of event to dispatch for abort and statechange. Currently I'm using a simple 'Event' object. Maybe this is enough.

bakulf on 22 Mar 2017

🎉6 👍2

I don't know why I wrote 'comment 28', I meant" https://github.com/whatwg/fetch/issues/447#issuecomment-281731850

bakulf on 22 Mar 2017

Note, we haven't written any WPT tests yet. I filed a bug to do that before we enable this pref by default on nightly:

https://bugzilla.mozilla.org/show_bug.cgi?id=1349587

wanderview on 22 Mar 2017

👍1

@bakulf @wanderview That's exciting! Just making sure we don't pref on by default before it's in the spec. Is there an "intent to implement"?

jan-ivar on 23 Mar 2017

That's exciting! Just making sure we don't pref on by default before it's in the spec. Is there an "intent to implement"?

Yea, we won't let it ride the trains to release channels until the spec is finalized. We may enable on nightly-only once we have more confidence its working as intended. For example, I found this today:

https://bugzilla.mozilla.org/show_bug.cgi?id=1349950

I don't think we have an Intent-to-Implement message, but we will send an Intent-to-Ship before moving forward further.

My hope is we can get some kind of agreement to move forward on spec/impl at the upcoming service worker meeting. I know @annevk won't be there, but it will be interesting to hear from the other browser vendors. If they agree and @annevk doesn't have objections perhaps we can move forward.

wanderview on 23 Mar 2017

FWIW, I have no particular issues with the high-level design, I think the main challenge is going to be working through all the details, making sure we didn't forget anything, and get all the tests written.

annevk on 23 Mar 2017

I guess I should also note that we haven't implemented any of the priority or progress bits from Jake's proposal. We focused just on cancellation and FetchState for now.

wanderview on 23 Mar 2017

F2F:

Drop "follow" - @stuartpb is right about the edge cases
Come up with base classes so it can be used for "abort" only
If the fetch is initiated on the same thread as the controller, should we ensure aborting before the fetch promise resolves always rejects the promise with an AbortError, even if the response completes? It may involve removing things from the microtask queue. Worried that pretending an upload was aborted, when it actually completed, is a bit misleading.

jakearchibald on 4 Apr 2017

If the fetch is initiated on the same thread as the controller, should we ensure aborting before the fetch promise resolves always rejects the promise with an AbortError, even if the response completes? It may involve removing things from the microtask queue. Worried that pretending an upload was aborted, when it actually completed, is a bit misleading.

Yeah, I thought this was resolved with the decision that abort() would be async, resolving with a boolean of whether the request was actually aborted or if it had already ended by the time the abort was evaluated.

stuartpb on 4 Apr 2017

Come up with base classes so it can be used for "abort" only

What's the rationale for this?

stuartpb on 4 Apr 2017

Drop "follow" - @stuartpb is right about the edge cases

That's kind of a bummer - I actually rather liked the "opaque object that lets control be abrogated to mirror an upstream request" pattern, and I feel like a simpler implementation of this (ie. one where you can only follow one signal, which works like you're putting your fetch in a "pool" with any other fetches following the signal, and you can't independently control the fetch unless you "unfollow" the signal first) would be kinda useful, without all the edge cases that marred the graph-traversal approach.

stuartpb on 4 Apr 2017

That said, I do agree that follow/signal is going to involve some dedicated discussion, it's something that could be introduced later, and shouldn't be a blocker for the rest of the spec (because the fact that XMLHttpRequest is still the only way to make abortable requests in 2017 is a downright crisis), so pulling it out seems like absolutely the right move, at least for now.

stuartpb on 4 Apr 2017

To be clear, signal still seems very useful. The value of the "follow" method just isn't very clear. At least the way I understand what follow does, you can trivially easily just add your own event listeners to the signal that call methods of your controller, getting the exact same behaviour follow/unfollow would give you.

mkruisselbrink on 4 Apr 2017

One of the benefits of a signal object over just the Controller and Observer design that I originally pitched is that it lets you pass a value that only lets requests follow your request's changes - ie. downstream code wouldn't be able to hitch a ride on your fetch's progress events, or any of the other details that might be exposed via FetchObserver that wouldn't be needed to follow it.

Another advantage would be that it'd let all this control happen outside of the JavaScript thread, rather than having to rendezvous with a callback every time a request needs to be aborted in response to another request being aborted.

In any case, it's definitely polyfillable (or prollyfillable or ponyfillable or whatever the kids are calling it these days) using the other concepts, following a clearer spec to come at some point down the line, after the basics of fetch observation and control have been surfaced to define this kind of functionality.

stuartpb on 4 Apr 2017

Yes, having a signal object is definitely something that we agreed on being a good idea at the F2F. The comment about follow was purely about the follow/unfollow methods on the controller object, as those don't seem to add anything.

mkruisselbrink on 4 Apr 2017

Well, as I said in paragraph 2 there, having a follow method, on top of being more convenient for end developers (allows just calling a method instead of having to specify your own closure/callback that might not cover everything that follow may be extended to incorporate in following), allows for control to be propagated without having to loop the scripting thread in on the decision. Mostly a performance win.

stuartpb on 4 Apr 2017

Ah sorry, yeah, that makes sense. Although as long as Controller has synchronous getters for the priority and state attributes, you'll still need to somewhat loop the scripting thread in. Although yeah, you can probably get away with somewhat fudging when exactly you update those attributes.

mkruisselbrink on 4 Apr 2017

Yeah, I thought this was resolved with the decision that abort() would be async, resolving with a boolean of whether the request was actually aborted or if it had already ended by the time the abort was evaluated.

I think it would be helpful for implementations to agree on some points where abort has consistent behavior. The current FF prototype has this weirdness currently:

https://bugzilla.mozilla.org/show_bug.cgi?id=1349950

So this code:

const url = '/';
const controller = new FetchController();
const signal = controller.signal;
const observe = (observer) => {
  observer.addEventListener('statechange', event => {
    console.log('state changed to ' + observer.state);
    if (observer.state === 'responding') {
      console.log('Calling abort!');
      controller.abort();
    }
  });
}

fetch(url, {signal, observe}).then(r => {
  console.log('fetch() resolved with a Response');
  return r.text();
}).then(text => console.log(text)).catch(err => {
  if (err.name == 'AbortError') {
    console.log('The request/response was aborted');
  } else {
    console.log('Unexpected error: ' + err.name);
  }
});

Outputs this on jsbin for me:

    "state changed to responding"
    "Calling abort!"
    "state changed to aborted"
    "state changed to complete"
    "fetch() resolved with a Response"
    ""

Note that the state changes to "aborted", but the final Response and .text() output resolve. This seems wrong? I would expect that if .abort() is ever called before state changes to "completed" then the promises should reject with AbortError.

wanderview on 6 Apr 2017

Of course, then FF prototype also does this if you abort after Response is resolved:

const url = '/';
const controller = new FetchController();
const signal = controller.signal;
const observe = (observer) => {
  observer.addEventListener('statechange', event => {
    console.log('state changed to ' + observer.state);
    if (observer.state === 'responding') {
      // don't abort here this time
    }
  });
}

fetch(url, {signal, observe}).then(r => {
  console.log('fetch() resolved with a Response');
  console.log('Calling abort!');
  controller.abort();
  return r.text();
}).then(text => console.log(text)).catch(err => {
  if (err.name == 'AbortError') {
    console.log('The request/response was aborted');
  } else {
    console.log('Unexpected error: ' + err.name);
  }
});

You get:

"state changed to responding"
"state changed to complete"
"fetch() resolved with a Response"
"Calling abort!"
"The request/response was aborted"

It seems likely something is clearly inconsistent and wrong here, but its not clear to me from the sketch what the correct defined behavior should be. I think to progress on our prototype we need to define what happens at each state when you call abort.

wanderview on 6 Apr 2017

The correct behavior should be that abort() returns its own promise, which will resolve with true if and only if the fetch was terminable. If the fetch is completed - even if it's yet to report its completion (ie. it's only completed on the network thread) - the abort() promise should resolve with false (and should resolve only after this other state has been returned to JS).

In any case, the fetch should never be able to go from any terminal state (completed, aborted, or failed) to any other - a completed fetch will never abort or fail, and an aborted fetch will never complete.

See also https://github.com/whatwg/fetch/issues/448#issuecomment-275274933, on why abort() can't/shouldn't just be synchronous.

stuartpb on 7 Apr 2017

Oh, we're talking about what should happen after a fetch resolves with a response... the state shouldn't be changing to complete, there. That's the issue I'm seeing - completion must be returned once the response has terminated, not when it's been received.

stuartpb on 7 Apr 2017

I believe the state is supposed to change to responding before the fetch resolves, and then only change to completed when whatever body-consuming promise like res.text() resolves.

Aside: what happened to having two separate states for "responding with headers" and "responding with body", which demarcate the pre- and post-resolution sides of the fetch response object? I'm pretty sure the proposal I wrote had that.

EDIT: I just took another look, and while my proposal had response as an event representing the completion of headers (ie. one that would coincide with the resolution of an initial fetch call), it actually didn't have any state properties beyond FetchObserver.finished. Probably meant to add that in a future revision 😅

stuartpb on 7 Apr 2017

I think what'd make the most sense here would be to fork https://github.com/github/fetch and use it as a reference implementation of the desired behavior (or at least as close to such a thing as is possible via what's already possible with XHR).

stuartpb on 7 Apr 2017

Bikeshed: Has there been any F2F on having a statechange event versus having separate events for each kind of state change (a la https://github.com/stuartpb/FetchController-spec#fetchobserver-events)? I'd really prefer both (ie. dispatching statechange events followed by state-specific events), especially as I see myself more frequently wanting the form factor of the latter than the former (eg. listening for a responding event rather than listening to every statechange event and manually filtering within to see if the state was changed to responding, which has been one of the most annoying things about working with XHR's readystatechange event).

stuartpb on 7 Apr 2017

@wanderview The observer should reflect what actually happened, so it should never go from "aborted" to "complete", which are both end states.

jakearchibald on 9 Apr 2017

@stuartpb

The correct behavior should be that abort() returns its own promise, which will resolve with true if and only if the fetch was terminable

I don't think this works since a signal can be given to multiple fetches. What if one of eight fetches is successfully aborted? I think it's simplest to say "if you want to know what happened, observe it".

Aside: what happened to having two separate states for "responding with headers" and "responding with body"

I didn't think too hard about the states, and I'm more than happy to dig into them again. My aim was to allow states for opaque responses, and making the distinction between "responding with headers" and "responding with body" would be exposing new timing data, which we shouldn't do.

I'm not against adding additional states for non-opaque responses - @annevk do you think that's useful?

Bikeshed: Has there been any F2F on having a statechange event versus having separate events for each kind of state change (a la https://github.com/stuartpb/FetchController-spec#fetchobserver-events)? I'd really prefer both

Would it be be better to add responding and complete promises to the observer? Where complete would potentially reject with a network/abort error?

jakearchibald on 9 Apr 2017

I think we already expose the difference for opaque responses. Due to fetch() vs the load event. That's what got exploited and we discussed in #355.

Where is the latest proposal hosted?

annevk on 9 Apr 2017

My aim was to allow states for opaque responses, and making the distinction between "responding with headers" and "responding with body" would be exposing new timing data, which we shouldn't do.

Doesn't XHR already go through a HEADERS_RECEIVED state, which is what I patterned this distinction off of?

stuartpb on 9 Apr 2017

@jakearchibald

I don't think this works since a signal can be given to multiple fetches. What if one of eight fetches is successfully aborted?

I'd notch that as a flaw in the signal model, then (another reason I feel signals/following should be held back as a higher-level abstraction, to be defined later above per-fetch control / observation).

stuartpb on 9 Apr 2017

@annevk

I think we already expose the difference for opaque responses. Due to fetch() vs the load event.

I think that's different to what @stuartpb is proposing. fetch resolves once headers have been received, but I think @stuartpb wants "responding with headers" to mean something after making the request but before headers have been received.

Although, does XHR already sorta expose this via upload progress events?

Where is the latest proposal hosted?

Still the comment at https://github.com/whatwg/fetch/issues/447#issuecomment-281731850. I'll start getting a PR together based on @mikewest's work.

jakearchibald on 9 Apr 2017

@stuartpb

Doesn't XHR already go through a HEADERS_RECEIVED state, which is what I patterned this distinction off of?

Huh, maybe I'm misunderstanding what you're asking for. I thought "responding with headers" would be between sending the response and receiving headers.

I'd notch that as a flaw in the signal model

I think it's the major benefit of the signal model 😢. If you want to know the outcome, that's what the observer is for.

jakearchibald on 9 Apr 2017

I think it's the major benefit of the signal model 😢. If you want to know the outcome, that's what the observer is for.

Yes, but putting this on the resolution of an abort() promise is required to ensure the atomicity of this outcome. Even with mutexed synchronous getters, you can't tell if abort() beat the fetch's natural completion between when you call it and when it reaches the network thread using queries to an Observer, before or after.

stuartpb on 9 Apr 2017

And it's not enough to see if the state has gone from being in-progress to aborted, because the caller needs to know whether or not it was their call to abort() that aborted the fetch. You could have three abort()s called on the same fetch, but only one will be the one that causes the fetch to abort. The other two would be superfluous, and could likely need to handle cleanup differently.

stuartpb on 9 Apr 2017

@stuartpb

the caller needs to know whether or not it was their call to abort() that aborted the fetch

Oh, so you'd expect controller.abort() to resolves with false if the fetch was already aborted?

jakearchibald on 9 Apr 2017

Oh, so you'd expect controller.abort() to resolves with false if the fetch was already aborted?

Yes. That's how I interpreted this description from the January 24th meeting notes:

Aborting an already-complete request shouldn't produce an error, it should be a no-op, but there should be some way to confirm that something was actually aborted. E.g. some deletion APIs return a boolean to signal if anything was actually deleted.

Much as how requesting the deletion of something that doesn't exist (whether it existed previously or not) means that nothing was actually deleted in response to the request (it was effectively a no-op), calling abort() on a fetch that's already aborted would state "abort not performed", returning false to signify the non-operation.

stuartpb on 9 Apr 2017

I'm not really sold on the benefits of doing this on the controller when we'll have an observer. I don't think it really matters who aborted the fetch, it's more about the result.

jakearchibald on 9 Apr 2017

Huh, maybe I'm misunderstanding what you're asking for. I thought "responding with headers" would be between sending the response and receiving headers.

OK, so we can avoid confusion over things like whether "receiving headers" means the process of receiving headers or the point at which we've finished receiving headers, here's the complete list, off the top of my head, of every state, and transition step, a fetch goes through (in the traditional normally-completing HTTP request/response model, ignoring how premature termination or HTTP/2 may complicate these):

State Null: Nothing has been sent, no connection has been opened.
Step 1: A connection has been opened to the server, putting us into State A.
Step 2: The client begins sending their request to the server, entering State B.
Step 3: The client finishes sending the path, method, and headers of their request to the server, and begins sending the body of their request (if any) in State C. (If the request has no body, proceed directly to Step 4 / State D.)
Step 4: The client finishes their request (headers and optional body), and begins waiting in State D for an acknowledgement from the server.
Step 5: The client has received an acknowledgement of its request from the server, putting the fetch into State E as it begins receiving the response.
Step 6: The client has received the status and headers of the request, putting the fetch in State F as it continues to receive the body content (if any) of the response.
Step 7: The client finishes receiving the response from the server altogether, putting the fetch into State G (completion).

I'm talking about differentiating State E and State F, here, where State E is "responding with headers" and State F is "responding with body". I guess XHR's readyState 1 combines State A through State E, so this could potentially reveal meaningful timing information for Step 5 relative to Step 6 (which is when XHR fires readystatechange to readyState 2) - however, exposing a combined state for the entirety of a response being received (combining State E and State F) would also expose Step 5 (though not exposing Step 6, which would be exposed under XHR).

I mean, maybe I'm misinterpreting XHR's readyState values - I readily admit that I don't see much distinction in readyState 2 versus readyState 3 (they'd both fall under Step 6 in the model I described above - maybe States C and F above should be split on this distinction between finishing headers and beginning the body?) - but this seems like a sensible level of granularity to describe fetches under.

stuartpb on 9 Apr 2017

And, if I'm not entirely off the mark (which I may well be - I'm going just off a reading of the spec rather than any firsthand tests / experience), XHR Progress Events already expose Step 5 in the form of the loadstart event, so, yeah, there'd be nothing new here.

stuartpb on 9 Apr 2017

re: readyState 3, the assumption that I made when I wrote the events in my proposal was that the transition to readyState 3 would correspond to the first progress event after reaching readyState 2, which is why I didn't include a specific state for it (it's just "the first progress event within the post-headers state", and XHR making that a separate state was just one of those weird vestigial details from XHR's original implementation-oriented interface).

stuartpb on 9 Apr 2017

@wanderview The observer should reflect what actually happened, so it should never go from "aborted" to "complete", which are both end states.

I need to look at the PR you wrote this morning, but can the observer state be "completed" before the fetch() promise resolves? If the answer is yes, then the following is true?

const controller = new FetchController();

fetch(url, { controller }).then(response => {
  // maybe a no-op if already in completed state
  controller.abort();
  return response.text();
}).then(text => {
  // We get here if the body is small enough that its already completed before
  // the fetch promise resolves with the Response.
});

wanderview on 9 Apr 2017

@stuartpb your states are flawed. You can get a response before you have even transmitted all request headers, even in H/1.

annevk on 9 Apr 2017

@wanderview Yes (based on my understanding of abort() without speaking for Jake's PR et al) - in that case, it would go from "loading" (or whatever) to "complete", and never reach "aborted", because the fetch completed before the abort call reached it.

stuartpb on 9 Apr 2017

And differentiating between E and F we shouldn't do, because redirects need to be opaque.

annevk on 9 Apr 2017

@wanderview The PR doesn't cover any of this yet. Yeah, need to have a look at other event+promise APIs and look at the ordering.

With the above (btw it's the signal that gets passed to fetch), I think it'd abort in time, since .text() would check the signal's state at the start.

jakearchibald on 9 Apr 2017

@annevk

And differentiating between E and F we shouldn't do, because redirects need to be opaque.

Does XHR's HEADERS_RECEIVED not already differentiate between E and F?

Also, isn't opacity only applicable for a subset of fetches (for which State E could be merged into State F, or vice versa)?

stuartpb on 9 Apr 2017

@stuartpb no, you reach HEADERS_RECEIVED after all redirects are followed and the headers of the final response are fully parsed.

annevk on 9 Apr 2017

you reach HEADERS_RECEIVED after all redirects are followed and the headers of the final response are fully parsed.

Well, then that'd be the semantics for Step 6 (State E -> F) (assuming fetch is operating in "follow" redirect mode, which I guess isn't currently exposed to scripting, but I could see being so in the future, even if just eg. as part of an extension to node-fetch).

stuartpb on 9 Apr 2017

I'm not sure what you mean. (Note that I also pointed out that a single state system doesn't work since it's incompatible with HTTP.)

annevk on 9 Apr 2017

I'm saying that in this state system (and we can say that it'd be only be reflecting the state of the response, ie Step 2 / State B are followed immediately by Step 5 / State E - the request state'd probably be tracked separately, possibly under an upload object like how XHR's progress events are factored), the "receiving headers" state would encapsulate all redirect responses, to keep redirects properly opaque, and Step 6 would only trigger after the first non-redirect response's headers had finished (a la the semantics of XHR's HEADERS_RECEIVED readyState).

stuartpb on 9 Apr 2017

I'm not really sold on the benefits of doing this on the controller when we'll have an observer. I don't think it really matters who aborted the fetch, it's more about the result.

Another reason I'm uneasy about the signal-that-can-potentially-control-multiple-otherwise-independent-fetches model, elaborating a bit on what I alluded to here: if the network thread is truly independent of the scripting thread, there could be hitches on that side, meaning that callbacks could fire in JS where some of the fetches attached to the signal have been aborted, and others are still pending evaluation of the signal - and that doesn't feel like it's going to play well with a lot of the other assumptions that are getting introduced into the mix here (like a promise choosing not to resolve based on a read of the signal?)

Again, I can't lay down a formal proof of what's wrong with this picture, but something in my gut is telling me it's not adding up, that there's at least one scenario where the arrows cross into a void.

stuartpb on 9 Apr 2017

With the above (btw it's the signal that gets passed to fetch), I think it'd abort in time, since .text() would check the signal's state at the start.

Ok, thanks. That's getting at my question a bit more. Because its possible the entire body is in memory and available by the time we resolve the fetch() promise with the Response. At the extreme, we can try to reason about how this works with empty string body or single character body.

wanderview on 10 Apr 2017

I think getting proposed WPT tests might be the most helpful here. Its easier to look at the cases and say "yes, that makes sense" or "oh my, really".

wanderview on 10 Apr 2017

It seems likely something is clearly inconsistent and wrong here, but its not clear to me from the sketch what the correct defined behavior should be.

It may help to rephrase the API as being about abort requests, not about forceful aborts. It's a request to the person who you give the signal to to stop doing unnecessary work. It's not a demand that the API terminate with an AbortError. You only terminate with an AbortError if you actually decided to respond to that abort request and abort the underlying process (here, a fetch).

So in some of the tricky cases being discussed here, where there is no work left to abort, then in that case you should just ignore the abort request---there's nothing to abort.

domenic on 11 Apr 2017

I guess maybe I'm having problems reasoning here because we're really talking about aborting two things:

The network request and the resulting transfer of the headers+body.
The consumption of the body content.

If (1) has completed, should the FetchController.abort() method also abort the streamed consumption of the body? My gut reaction playing with the API was that it should.

wanderview on 11 Apr 2017

Yes, I think it's indeed important to separate those two, and answer that question. I believe the plan is that it should abort the consumption, but the details on exactly how are not clear. E.g. if the browser has "consumed" a zero-byte response (or just a small response) already, can you still abort() it? Probably not, I would think, but we should define that in detail.

domenic on 12 Apr 2017

+1 at what @domenic says here:

It may help to rephrase the API as being about abort requests, not about forceful aborts.

@wanderview in my opinion, abort() should try to abort everything.

IMHO, just keep abort() idempotent so if it resolves to anything and it first resolved with true, keep successive calls resolving with true.

delapuente on 12 Apr 2017

We've started speccing and discussing this:

jakearchibald on 12 Apr 2017

🎉6

Yes, I think it's indeed important to separate those two, and answer that question. I believe the plan is that it should abort the consumption, but the details on exactly how are not clear. E.g. if the browser has "consumed" a zero-byte response (or just a small response) already, can you still abort() it? Probably not, I would think, but we should define that in detail.

What do you mean by "if the browser has consumed" a small response? In this snippet:

const controller = new FetchController();

fetch(url, { controller.signal }).then(response => {
  controller.abort();
  return response.text();
}).then(text => {
  // do we get here?
}).catch(e => {
  // or here?
});

The browser will likely have buffered the entire body text for small or empty bodies when the fetch promise resolves. So in my distinction from https://github.com/whatwg/fetch/issues/447#issuecomment-293263806 this has completed (1).

I would not say that (2) has happened yet. The body hasn't been consumed by either the browser or script yet.

wanderview on 12 Apr 2017

I guess I don't see why (2) hasn't happened yet. Is "buffered" different from "consumed"?

I still don't see what there would be to abort, if the body is completely buffered/consumed already.

domenic on 12 Apr 2017

I guess I don't see why (2) hasn't happened yet. Is "buffered" different from "consumed"?

Its the definition of the disturbed body in the fetch spec:

https://fetch.spec.whatwg.org/#concept-readablestream-disturbed

No one has read from the ReadableStream, called .text() yet, etc. Its not consumed at that point.

wanderview on 12 Apr 2017

Sorry to keep picking at this, but I thought of another reason to make FetchController.abort() force things like Response.text() to reject even if the full body is buffered:

It would let the browser throw away the buffered data immediately. Today we either have to wait for the Response to be GC'd or for script to call a consuming API like Response.text(). Both of these have some amount of cost in either time kept alive or wrapping the data to expose to content.

wanderview on 12 Apr 2017

👍6

That's a pretty good reason. I'm convinced.

domenic on 12 Apr 2017

Which I guess leads me to some other questions:

Is the signal propagated on Response.clone()? Or should you be able to cancel one side of the clone and not the other (maybe thats doable via streams API)?

Should we allow a CancellationSignal to be passed in the Response constructor so script can cancel synthetic Responses?

wanderview on 12 Apr 2017

Sorry to keep picking at this, but I thought of another reason to make FetchController.abort() force things like Response.text() to reject even if the full body is buffered:

FWIW: RxJS has a similar behavior with unsubscribed/disposed ReplaySubjects. It cleans up memory and throws if someone tries to resubscribe. Similar reasoning.

benlesh on 12 Apr 2017

👍2

@jakearchibald you mentioned the f2f meeting coming up, and some possible discussion points.

Seems like it might also be an opportune time to discuss what Push might look like. Some pertinent links to help job memories- you Jake had some early ideas late January. @martinthomson provided some clarification on how Push works at the end of another later thread on Push.

Is it worth trying to limit the problem to exclude ServiceWorkers in the beginning? The very minimal additions I'd see to your idl would be just adding a push type to FetchObserver-

[Exposed=(Window,Worker)]
interface FetchObserver : EventTarget {
  // ...
  attribute EventHandler onpush;
};

The onpush listener needs to either get a Response I'd imagine, or some new PushResponse that is structurally similar.

These two pieces could make a useful start for getting notices of any server pushed resources. I do think longer term it would be necessary & desireable to allow ServiceWorkers to also push additional resources in response to a request, but forgoing that and starting with the basics doesn't seem to me at this position like a danger, like it would be limiting to ServiceWorker's latter approach.

rektide on 14 Apr 2017

@rektide those were the conclusions from those attending the F2F. There are no meetings planned, but we do have a plan for aborting a Fetch now and it's outlined in https://github.com/whatwg/fetch/issues/447#issuecomment-293515060. I suspect we'll open a separate issue to work through FetchObserver.

annevk on 15 Apr 2017

The more I think about it, I think a push event should only be given a Request. If it wants the response, it should fetch. That way it goes through all the usual checks like CORS and CSP.

We may want a fetch option to fetch from the push cache only.

jakearchibald on 15 Apr 2017

@wanderview

Is the signal propagated on Response.clone()?

Yeah, I don't think you'd be able to cancel a single clone (aside from response.body.cancel(reason).

jakearchibald on 15 Apr 2017

Hey, what's the status on this?

benjamingr on 10 May 2017

@benjamingr Google I/O, followed by me being away in June. We expect to move forward in July, unless there's volunteers. Possibly DOM might have abort signals sooner for Streams.

annevk on 10 May 2017

Thanks for the update! July sounds good.

benjamingr on 10 May 2017

How likely will this feature be implemented?

maxbaluev on 22 May 2017

How likely will this feature be implemented?

Gecko has it prototyped. Just waiting for spec and WPT tests to finalize before working on it more.

wanderview on 22 May 2017

The relevant pull requests are https://github.com/whatwg/dom/pull/437 and #523, though the status is still as I mentioned above. (If these pull requests are not to your liking that would be a good thing to air now though, don't wait for July.)

annevk on 22 May 2017

If we already have

var timeoutID = setTimeout(function(){}, 10);
clearTimeout(timeoutID)

Why not simply do

var promiseObjectAsID = fetch(url);
promiseObjectAsID.then(function(){}, function(){});
cancelFetch(promiseObjectAsID)

Is it because we don't want another global variable like cancelFetch?

qfan on 6 Jun 2017

@qfan It'd perpetuate the common misconception that a promise is some kind of control surface. If you pass me a promise, you generally don't expect me to be able to cancel your operation.

It also doesn't play very well with await.

jan-ivar on 6 Jun 2017

👍1

BTW, this is a great hack to get abort for fetch today: you run fetch inside a web worker and you terminate the web worker when you want to abort a fetch.

mitar on 6 Jun 2017

😄8 👍4

@jan-ivar I have the feeling that the discussion of "Aborting a fetch" topic is always about something much more generic, such as cancelling a promise in general, or adding an aborting signal in general for everything in JavaScript.

But I think fetch is not at all general. It is so special that it needs low level work in JavaScript engine or binary library loaded.

All one needs to cancel a fetch are 2 things:

A reference to identify which fetch it is.
A function to cancel it.

Call it a controller with a method or whatever, they are essentially an identifier and an action.

I personally don't care whether the reference is a controller (object), numeric ID (int), promise(object) or a Web Worker (object). If I have to use a PID as control surface I'll use it.

Now thanks to the great hack @mitar mentioned, we have both 1 and 2.

qfan on 6 Jun 2017

@mitar I like it. Paves the way for some kind of polyfill. Shame it won't work in contexts that don't allow workers.

jakearchibald on 7 Jun 2017

@jakearchibald I have some proof on concept in this repo https://github.com/morantron/poor-mans-cancelable-fetch, that could be turned into a FetchController polyfill.

Shame it won't work in contexts that don't allow workers.

At least in a browser context, worker support is pretty much everywhere. The _hack_ allows you to cancel a browser's native fetch request. So for this approach to work you need to have both fetch + worker, which yields all browsers except IE11, and Opera Mini. In those cases, you can abort an underlying XHR request if using a polyfill.

Outside of the browser context, I guess there is an underlying layer that allows you to abort network requests? :thinking:

Do you think it could be a separate polyfill or should it be included in this same fetch polyfill?

I'd be happy to contribute :raised_hands:

Morantron on 27 Jun 2017

@Morantron

At least in a browser context, worker support is pretty much everywhere

Unless you're in a worker already, or a service worker, which I what I was meaning (sorry, I should have been specific).

jakearchibald on 27 Jun 2017

It is possible to cancel a Fetch request using a Fetch stream reader.

The following example is a quick draft that appears to work well in browsers that support Fetch Streams (the request is aborted after a few bytes of transfer).

var fetch = (function() {
    var fetch = window.fetch;
    var cancelableFetch = function() {
        var reader;
        var cancelled = false;
        var request = fetch.apply(this, arguments).then(function(response) {

            reader = response.body.getReader();
            if (cancelled) {
                // abort stream
                reader.cancel();
                return null;
            }

            var body = '';
            var decoder = new TextDecoder();

            function readStream() {
                return reader.read().then(function(result) {
                    if (cancelled) {
                        return null;
                    }
                    body += decoder.decode(result.value || new Uint8Array, {
                        stream: !result.done
                    });
                    if (cancelled) {
                        return null;
                    }
                    if (result.done) {
                        return body;
                    }

                    return readStream();
                })
            }

            return readStream();
        });
        var cancel = function() {
            if (cancelled) {
                return;
            }
            cancelled = true;
            console.warn('Fetch aborted');
            if (!reader) {
                return;
            }
            // abort stream
            reader.cancel();
        };
        return {
            then: function(resolve) { return request.then(resolve); },
            cancel: cancel
        };
    };
    return cancelableFetch;
})();

// reqular fetch request
var request = fetch('https://ajax.googleapis.com/ajax/libs/angularjs/1.6.4/angular.min.js');

// regular processing of result
request.then(function(body) {
    console.log("Fetch complete:", (typeof body === 'string') ? body.length : body);
}).catch(function(err) {
    console.log(err.message);
});

// cancel request
request.cancel();

Demo: https://jsfiddle.net/4awe08ob/

https://jakearchibald.com/2015/thats-so-fetch/#streams

https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream#Browser_Compatibility

Polyfill (untested): https://github.com/jonnyreeves/fetch-readablestream

optimalisatie on 5 Jul 2017

👍1

@optimalisatie The concept of cancelling via the response body has been suggested before, at https://github.com/whatwg/fetch/issues/27#issuecomment-86981339

That is however not the same as canceling a request. If the server takes a while to respond (or time out), then there is no way to cancel the request. The suggestion is a good fallback for a polyfill to approximate the desired functionality, but by no means a satisfactory way to cancel a fetch request.

Rob--W on 6 Jul 2017

👍1

@Rob--W I didn't notice it yet, however, it does appear to be slightly different. The example that I provided does abort the request. In Chrome, just a few bytes will be transfered and the request is aborted. Per haps the reader interface performs some other action to abort the request.

fetch-abort

optimalisatie on 6 Jul 2017

I'd like to add a real-world use case to this thread. We want to use fetch aborting to rate limit our applications, in order to more evenly distribute our traffic to our upstream cloud providers. Currently this would be done inside the application code, by buffering and throttling the creation of outgoing requests.

However, our users do often run multiple application instances (same domain) and we would prefer to be able to do the rate limiting via a single domain level Service Worker intercepting all of the requests, rather than having each application instance do it individually in a vacuum with no knowledge of the rates occurring in sibling application instances.

Items of interest to us:

intercept the fetch before the request stream initiates (no traffic to upstream provider is critical)
truly abort some requests, we only need the most recent request to go through in a given window of time

dnalbach on 10 Jul 2017

@dnalbach the proposal we're implementing caters for that. Abort signals from the page are passed to the service worker, but the service worker can create its own that abort the request independently.

jakearchibald on 11 Jul 2017

👍2

@Rob--W my apologies for the misunderstanding, it is correct that the request was not truly cancelled in the reader.cancel() solution. I investigated it a bit further and discovered a potential solution for some applications using window.stop(). It will cancel Fetch requests but it has the same effect as pressing the stop button in the browser.

fetch-cancel

optimalisatie on 16 Jul 2017

@optimalisatie window.stop() is not a reliable way to cancel requets . Furthermore, when it does work, it does not abort a specific request, but *all requests.

* window.stop() does not abort fetch in Firefox - https://bugzilla.mozilla.org/show_bug.cgi?id=1165237

Rob--W on 16 Jul 2017

@Rob--W you are correct, I wanted to share it because I didn't find a reference to it in the topic. It may be beneficial in some situations.

One more potential solution may be Shadow DOM. It's only supported in Chrome, Opera and Safari but it may enable targeted cancellation of Fetch requests without the latency issue from WebWorkers (~40ms startup latency, making them unsuitable for some applications).

https://developer.mozilla.org/en-US/docs/Web/Web_Components/Shadow_DOM

optimalisatie on 16 Jul 2017

The following solution enables cancellation of Fetch requests in all browsers, including IE. It would also enable targeted cancellation of any other javascript execution and promises.

https://github.com/optimalisatie/exec.js

Firefox:

Chrome:

// exec.js (644 bytes)
// https://github.com/optimalisatie/exec.js

// fetch request
var runner = new exec(function(postMessage) {
    fetch('https://ajax.googleapis.com/ajax/libs/angularjs/1.6.4/angular.min.js')
        .then(function(response) {
            response.text().then(function(text) {
                postMessage(text);
            });
        }).catch(function(err) {
            console.log(err.message);
        });
},function(data) {
    console.log('fetch result', data);
});

// cancel request after +/- 170ms (fine tune to test cancellation in Firefox)
setTimeout(function() {
    runner.stop();
}, 170);

It also works for setInterval/setTimeout.

optimalisatie on 16 Jul 2017

❤2 😕2 🎉2 👍2

The following solution works in all browsers that support fetch.

Include exec.js + exec-fetch.js (621 + 221 bytes) in the HTML document.

<script src="exec.min.js"></script>
<script src="exec-fetch.min.js"></script>

The native fetch API is now enhanced with a .abort() method. This is not a Polyfill.

// normal fetch request
var request = fetch('https://ajax.googleapis.com/ajax/libs/angularjs/1.6.4/angular.min.js')
    .then(function(response) {
        response.text().then(function(text) {
            console.log('response', text.length);
        });
    }).catch(function(err) {
        console.log('err', err.message);
    });

// abort request after 10ms
setTimeout(function() {
    request.abort();
}, 10);

optimalisatie on 18 Jul 2017

😕3 👍1

The following also works with fetch:

async function getResource(onCancel) {
  var controller = new FetchController(), 
  signal = controller.signal;
  onCancel(() => controller.abort());
  var result = await fetch("./someResource", {signal}).then(req => req.text());
  return process(result);
}

// outside
var cancel;
var resourcePromise = getResource(fn => cancel = fn);
onExternalEventThatCausesCancellation(() => {
  cancel(); // calls .abort on the fetch controller.
});

// Or:
var resource = await getResource(onExternalEventThatCausesCancellation);

benjamingr on 19 Jul 2017

😕1

@optimalisatie Nice work! Can it be fashioned into a spec polyfill? I prefer the spec API. E.g.:

async function foo(signal) {
  try {
    let response = await fetch('https://foo.com/some.resource.js', {signal});
    console.log('response ' + (await response.text()).length);
  } catch(err) {
    console.log(err);
  }
}

let control = new FetchController();
foo(control.signal);
setTimeout(() => control.abort(), 10); // abort foo after 10ms

jan-ivar on 19 Jul 2017

Fiddle that works in Firefox 55+ w/pref.

jan-ivar on 19 Jul 2017

@benjamingr

@jan-ivar
yes, that is possible.

exec-fetch.js simply adds the .abort() method to the original fetch API that works in all browsers.

optimalisatie on 19 Jul 2017

@optimalisatie this issue is not for discussing browser support, this issue is about discussing the API.

There _is_ an API for aborting a fetch (which you can polyfill), this issue can be closed unless we want it to be used for references and tracking implementation status.

Your trick with running a window and stopping it is nice - but I'm not sure why it's relevant to the API design discussion here. You might want to PR polyfills (although they can already support this API based on the XHR one).

I think this issue can be closed - the API discussion was had, everything works - and we need to make sure all browsers have issues tracking this.

benjamingr on 19 Jul 2017

Wouldn't the iframe approach have issues with CSP settings?

Morantron on 19 Jul 2017

@benjamingr
I understand. Regarding an XHR Polyfill, the difference is the amount of code required and performance. The iframe based solution offers no performance disadvantage and it may enable performing fetch requests in a separate thread in future versions of Chrome. It works with the original Fetch API with no serialization/conversion overhead.

@Morantron
To enable exec.js with Content-Security-Policy, 'nonce-execjs' needs to be added script-src.

optimalisatie on 19 Jul 2017

Iframes can't run in a different thread if you have synchronous access to their content window.

jakearchibald on 19 Jul 2017

@jakearchibald
We are investigating it. It appears to be possible but the current version of exec.js (optimized for speed) may not be the correct way to achieve multi-threading.

Update

After further testing, multi-threading with much better performance than WebWorkers is possible, however, as it seems it can only be achieved by starting Google Chrome 55+ with the flag --process-per-site.

Subframes are currently rendered in the same process as their parent page. Although cross-site subframes do not have script access to their parents and could safely be rendered in a separate process, Chromium does not yet render them in their own processes. Similar to the first caveat, this means that pages from different sites may be rendered in the same process. This will likely change in future versions of Chromium.

https://www.chromium.org/developers/design-documents/process-models
https://www.chromium.org/developers/design-documents/oop-iframes

We've tested with Chrome 61.0.3159.5 (unstable) so it appears that multi-threading will not become available to subframes soon.

In Chrome 61 WebWorkers are still very slow with a startup latency of ~100ms on a 2016 Core M7 laptop.

optimalisatie on 19 Jul 2017

Unfortunately this thread seems to have become some kind of weird advertisement for a third-party library, so I am unsubscribing. Feel free to @-mention me if you need something relevant.

@benjamingr we should not close this issue until it is fixed (i.e. the corresponding spec change has landed.)

domenic on 19 Jul 2017

😕3 😄3

That's referring to the window proxy, not the content window.

It's possible for origin-sandboxed iframes to end up on another thread. Not sure if anyone implements them that way though.

jakearchibald on 19 Jul 2017

@domenic oh, I am sorry - I thought the spec was updated because MDN already is at https://developer.mozilla.org/en-US/docs/Web/API/FetchController and https://developer.mozilla.org/en-US/docs/Web/API/FetchController/abort - I would volunteer writing the spec but I'm 100% sure it'll take the people here more time if I do than if I don't :)

Do we have an ETA on updating the spec @jakearchibald ?

benjamingr on 19 Jul 2017

@benjamingr It's in progress.

AbortController has landed in the DOM spec https://dom.spec.whatwg.org/#aborting-ongoing-activities
The fetch PR is at https://github.com/whatwg/fetch/pull/523
The tests PR is at https://github.com/w3c/web-platform-tests/pull/6484

jakearchibald on 19 Jul 2017

🎉11 👍7

My apologies @domenic I just intended to share a solution for abortable Fetch that works in all browsers. I understand that this topic is concerned with the official spec.

@jakearchibald thank you for the feedback!

optimalisatie on 19 Jul 2017

what's new?

Yakoot on 22 Aug 2017

See https://github.com/whatwg/fetch/issues/447#issuecomment-316378856.

This is already being implemented in Mozilla & Edge. Chrome will start soon.

More info on how it works: https://github.com/w3c/web-platform-tests/pull/6484#issuecomment-315775251

jakearchibald on 22 Aug 2017

🎉8

As per the TC39 cancellation proposal issue, would it be possible to decouple this spec from EventTargets so that it could be brought into JS directly?

RangerMauve on 12 Sep 2017

@RangerMauve that kind of request would be better suited for https://github.com/whatwg/dom where AbortController et al are defined. I don't immediately see how we'd go about that though or get the same kind of functionality. In particular with an eye on the future where subclasses will want to dispatch other events as well.

annevk on 13 Sep 2017

👍2

Thank you @annevk, I wasn't sure where to post to get people's eyes on this. :D

RangerMauve on 13 Sep 2017

👍1

🎉

benjamingr on 20 Sep 2017

🎉22

I know I'm late for the party and missed something, but why the attribute name was chosen to be just signal and not, for example, abortSignal? If at any future point there will be a need to provide other signals to fetch, how is this change planned to be made without breaking compatibility? I saw somewhere that the type AbortSignal may be changed to a FetchSignal, how would this look like? Thank you.

sompylasar on 20 Sep 2017

@sompylasar The signal object will handle the propagation of _all_ fetch control signals, not just aborting - the idea is that you'd want to keep a set of fetches in general lock-step.

stuartpb on 20 Sep 2017

👍2

Yep, if we introduce FetchController its .signal will be a FetchSignal which inherits from AbortSignal.

jakearchibald on 21 Sep 2017

@jakearchibald

Chrome will start soon.

Guys from Google say it already work, but it still doesn't as of Chrome 63.

wzup on 23 Feb 2018

@wzup the article says it works in Edge and Firefox, and points to https://bugs.chromium.org/p/chromium/issues/detail?id=750599 for Chrome, which isn't marked fixed yet...

annevk on 23 Feb 2018

It looks like a change for this was landed on Feb 5, so based on the Chromium release schedule, I am guessing that it would show up in Chrome 66. Just a guess.

joeyparrish on 23 Feb 2018

it's on chrome canary (66)

Bnaya on 23 Feb 2018

@joeyparrish That change added the AbortController and AbortSignal APIs, but it is not yet integrated with fetch. From the commit message:

One fetch() wpt test that previously failed due to AbortController being undefined now times out because the "signal" properties in fetch options doesn't do anything yet. This will be fixed once it is implemented.

If you want, you can already write your own functions that can be aborted through an AbortSignal. However, Chrome's fetch doesn't yet know about AbortSignals, so you can't yet abort a fetch with it. (Hopefully that changes soon?)

MattiasBuelens on 24 Feb 2018

Ah, okay. Now I get it. Thanks for clarifying.

joeyparrish on 24 Feb 2018

That change added the AbortController and AbortSignal APIs, but it is not yet integrated with fetch.

@MattiasBuelens Which means we can't use the existence of the AbortController constructor as a mean to check whether abortion is supported?

Mouvedia on 10 Jun 2018

@Mouvedia No, you can't. However, you can check if Request.prototype.signal exists, like in yetch. 😉

MattiasBuelens on 11 Jun 2018

👍3

I couldn't find a reference to abortions automatically triggered by the browser in the specification.

What happens to ongoing requests when you navigate away or close a tab?
Should the abort event (of the signal) fire?
Do I have to rely on onbeforeunload?

Mouvedia on 18 Dec 2018

That's #153 and there's various issues against whatwg/html too. Someone needs to do a lot of research across browsers to get that nailed down more exactly.

annevk on 21 Dec 2018

Fetch: Aborting a fetch: The Next Generation

How cancellation feels

The fetch controller

The fetch observer

Controlling multiple fetches at once

Within a service worker

Most helpful comment

All 240 comments

Interests in this issue from a consuming library

Fetch controller

Fetch observer

Within a service worker

Signals elsewhere

Update

Related issues