Abort controller design

[annevk]

#437 is the latest instance of generic design of the abort controller. (#434 is a previous iteration that also has discussion.)

whatwg/fetch#523 is a specific implementation of the abort controller for the Fetch API.

We need to decide if the generic abort signal needs to be serializable. @domenic argued in favor. Such a design would require a private MessageChannel. We might have to abstract MessageChannel or create some technical debt for someone to cleanup later.

For the Fetch API we will need to expose a signal to the service worker that signals "abort" whenever the "main thread" signal signals "abort" or the "main thread" fetch that caused the service worker to dispatch a fetch event is terminated with reason "abort" (I don't think we want to trigger that signal in the service worker for other reasons).

I believe these are the main outstanding things to sort through. Apologies to everyone about the various repositories this ends up involving. Open to suggestions on how to organize things better.

[annevk]

AbortSignal gains an associated incoming port (a MessagePort object) and outgoing ports (a set of MessagePort objects).

AbortSignal object serialization steps:

1. If value's aborted flag is set, then set serialized.[[Aborted]] to true and return. (Or should we simply throw? There's not much point to this.)
2. Let channel be a new MessageChannel object.
3. Append channel's port1 to value's outgoing ports.
4. Append the following algorithm to value's abort algorithms: (Aside: we should change abort algorithms invocation to pass a signal argument.)
   1. For each port in signal's outgoing ports: invoke port's postMessage() with "aborted". (Aside: hopefully we can do better than this.)
5. Set serialized.[[IncomingPort]] to ! StructuredSerializeWithTransfer(channel's port2, channel's port2).

AbortSignal object deserialization steps:

1. If serialized.[[Aborted]] is present and true, then set value's aborted flag and return.
2. Set value's incoming port to ! StructuredDeserializeWithTransfer(serialized.[[IncomingPort]], the current Realm).
3. Append an event listener to value's incoming port listening for "message" events that runs these steps with the event e:
   1. Assert: e.data is "aborted".
   2. Run signal abort with value.
4. Enable value's incoming port.

I think this works, but it needs to be double checked.

cc @domenic

[domenic]

Overall seems pretty reasonable standalone. I think it'd help to see this in action though by defining the transfer steps for Request that use it.

(Or should we simply throw? There's not much point to this.)

I wouldn't; it's useful to sometimes pass things around without having to do a state-check and a separate code path.

(Aside: hopefully we can do better than this.)

Yeah... this and the event listener on the other side really cry out for a better spec-level abstraction for these things. Maybe the port message queue is what should be used here?

It's fine for now though.

Set serialized.[[IncomingPort]] to ! StructuredSerializeWithTransfer(channel's port2, channel's port2).

Second arg should be a list.

[annevk]

I'm not sure I follow, currently a Request object cannot be serialized or deserialized. I thought we needed this for Streams.

[annevk]

Where I invoke StructuredDeserializeWithTransfer I should also get the [[Deserialized]] slot from the return value.

[domenic]

I thought we needed this for Streams.

I think I may have said that in the past, but I was confused; streams use these only for piping operations, but don't store signals themselves internally. So if you transfer a stream there's no AbortSignal that would go along with it whose transferring/cloning we would need to define.

I'm not sure I follow, currently a Request object cannot be serialized or deserialized.

Not via structured serialize... but we do something very similar, and hacky, in the fetch spec now, to get it from the service worker to the main thread. (Both Request and Response, I guess.) And the idea would be that, if we send a Request to a service worker, it needs to bring along its AbortSignal, so the service worker can know about any abort requests.

I think the current hacky serialization should ideally be based on structured serialize? Not as sure about that...

[annevk]

I think the current hacky serialization should ideally be based on structured serialize?

Currently we just pass the underlying request/response across threads, which is somewhat similar to passing JavaScript records across, although streams made that less ideal.

I guess we should flush this out more when we start working on defining abort in Fetch in much more detail.

[domenic]

Currently we just pass the underlying request/response across threads, which is somewhat similar to passing JavaScript records across, although streams made that less ideal.

Right. To be clear the steps around 10.3.1.2.5 in https://w3c.github.io/ServiceWorker/#fetch-event-respondwith are what I'm referring to. They are very similar to what transferring a stream will eventually look like (at least for byte streams), and so we could probably shorten them greatly by factoring out "transfer a stream". And then probably "transfer a stream" will also be used to allow structured transfer of the stream.

---

However I realized that in order to make fetch() abortable we're going to need to store the abort signal in the stream after all, if we want the abort signal to control both the initial headers request and the body. So we will need to define transferring/cloning of AbortSignals in order to define transferring of streams.

[jakearchibald]

@annevk

For the Fetch API we will need to expose a signal to the service worker that signals "abort" whenever the "main thread" signal signals "abort" or the "main thread" fetch that caused the service worker to dispatch a fetch event is terminated with reason "abort" (I don't think we want to trigger that signal in the service worker for other reasons).

This won't include fetches that are terminated as part of terminating the fetch group right? They're terminated with "fatal". Feels like the service worker should hear about this, so the developer can abort related fetches.

[jakearchibald]

I wonder if we can do something to aid extensibility here, so a subclass could do something like:

A FetchSignal has an associated priority, which is a number. It is serializable.

Then, to update the priority of a fetch signal to newPriority:

1. If signal's priority equals newPriority, return.
2. Set signal's priority to newPriority.
3. Fire an event named 'prioritychange' at signal.
4. Broadcast to cloned listeners to run "update the priority of a signal" with newPriority.

Some rough scribblings around how this could work:

Then AbortSignal's serialize steps could be:

1. Let channel be a new MessageChannel object.
2. Append channel's port1 to value's outgoing ports.
3. For each serializable item:
   1. If the item is a flag and is set, set serialized.[[(the item's name)]] to true.
   2. Else if the item is a number, set serialized.[[(the item's name)]] to the number.
4. Set serialized.[[IncomingPort]] to ! StructuredSerializeWithTransfer(channel's port2, channel's port2).

To deserialize AbortSignal:

1. Set value's incoming port to ! StructuredDeserializeWithTransfer(serialized.[[IncomingPort]], the current Realm).
2. Remove serialized.[[IncomingPort]].
3. For each key, serializedValue in serialized:
   1. If serializedValue is a boolean, set the flag on value named key
   2. Else set the property (these aren't properties are they? What are they?) on value named key to serializedValue.
4. Append an event listener to value's incoming port listening for "message" events that runs these steps with the event e:
   1. Run the algorithm named e.data.algorithm, with arugments e.data.arguments.
5. Enable value's incoming port.

To broadcast to cloned listeners a given algorithm name and array of arguments:

1. Let data be a new object.
2. Set data.algorithm to the given algorithm name.
3. Set data.arguments to the array of arguments.
4. For each port of signal's outgoing ports:
   1. Post a message to the port with data

[domenic]

It's kind of unprecedented to automatically make subclasses serializable, but I agree we want to find some way to make it easy.

[annevk]

Do we need to solve that for v1?

[jakearchibald]

I think v1 can be same-thread, so no serialisation needed.

[Jxck]

signal seems have a generic role to send SIGNAL not only abort for me.
currently I have no idea what kind of SIGNAL should signal sends,
but if new kind signal need to send in the future, Abort prefix for class will expected to conflict.

so, Abort prefix for Controller/Signal classes are really right ?
more general name like TaskController/TaskSignal are seems fits for me.

[jakearchibald]

I think we can go from AbortSignal extends EventTarget to AbortSignal extends Signal extends EventTarget later if need be.

[Jxck]

how about Controller ?

[jakearchibald]

Same

[Jxck]

basically signal will came from controller, so lots of case we need not to care about class of Signal.
but, controller are instantiate explicitly by developer.

const controller = new AbortController(); // explicit
const signal = controller.signal; // inplicit

controller.abort() // only for abort usage

AbortController and abort() is only for Abort.
more generic API's seems extensible for me..

controller = new TaskController();
signal = controller.signal

signal.addEventListener('any', ...)
controller.dispatchSignal('any', ...)

'abort' ing task is one use-case of 'abort' instead of 'any'.

[annevk]

@Jxck all the use cases we have thus far are either abort or abort + something else. If at some point we need something that is not abort, but does fit the same kind of pattern, we can generalize quite easily and have SignalController and Signal or some such and just make AbortController and AbortSignal inherit from those.

[Jxck]

ok thanks.

one more thing.
discuss about Aborting/Canceling Promise are too long to read through for me.
could you give me a pointer for why Controller and Signal are separated ?
(why we can't new/argument to fetch/call abort() a single Class ?)

[jakearchibald]

@Jxck web-platform-tests/wpt#6484 (comment) may help. Developers wanted to separate the ability to abort from the ability to react to a signal to abort.

It also simplifies the design & avoids race conditions. Imagine a signal that can also change priority. If you've posted that to a worker, you now have two threads that can change the priority.

[Jxck]

thanks @jakearchibald .

I found signal can use multiple.
so this codes works right ?

const controller = new AbortController()
const signal = controller.signal

Promise.race([
  fetch(url, {signal}),
  fetch(mirror1, {signal}), 
  fetch(mirror2, {signal}),
  fetch(mirror3, {signal}),
]).then((res) => {
  // first fetched response
  console.log(res)
  // stop rest of fetch
  // abort after resolve race(), so AbortErrors are unhandled
  controller.abort()
}).catch((err) => {
  if (err.name == 'AbortError') {
    // abort before race hasn't resolved
    return
  }
  // fail to race
  console.error(err)
})

[jakearchibald]

@Jxck I'm not sure what does exactly that you think. Promise.race() rejects as soon as one of the promises rejects, so if one of your mirrors is down, it may result in rejection before one of your mirrors responds. You may want promiseAny() from https://jakearchibald.com/2014/offline-cookbook/#cache-network-race.

Also, note that you're also aborting res's body stream when you call controller.abort().

[Jxck]

I meant we can stop rest of task when first task finished and Promise.race() resolved.
this makes race() more useful for me (of course I should care about reject you mentioned).

[annevk]

I'm going to close this since AbortController shipped. If there's still a desire for a transferable/serializable version at some point we can dig up this issue for thoughts.

[nuxodin]

Why not made a "FetchController" or "RequestController" - And so allow to handle "progress"-events too?
controller.addEventListener('progress',function(){...})

And does it really makes sense not to use the controller directly, but an additional "signal object"?

const controller = new RequestController;
fetch(url, {controller});

I'm not deep in the subject, so you can answer briefly.

[jakearchibald]

@nuxodin the plan is to have a fetch observer for this:

fetch(url, {
  observe(observer) {
    // observer will fire events & have properties related to the fetch.
  }
});