Consider always rejecting postTask() if abort signal received before callback is complete

[brendankenny]

There are a few different abort behaviors possible when an abort occurs after a schedulder.postTask() callback starts but before it returns, depending on how the callback's execution is divided over tasks and microtasks. This previously came up in #60.

I'm wondering if the abort behavior should instead depend only on the fact that the abort signal was received between the start and end of the callback, regardless of the content of callback. At least as a JS developer, this would better match my intuition for handing off a promise chain to be slotted into an API's promise chain and then run.

Running in Chrome or Firefox (with dom.enable_web_task_scheduling enabled):

const controller = new TaskController();

scheduler.postTask(
  async () => {
    controller.abort();
    await new Promise(r => setTimeout(r, 0));
  }, {signal: controller.signal}
); // AbortError.

the postTask promise will be rejected. Switching the statement order in the callback (which matches the WPT subject of #60), though:

const controller = new TaskController();

scheduler.postTask(
  async () => {
    await new Promise(r => setTimeout(r, 0));
    controller.abort();
  }, {signal: controller.signal}
); // Fulfilled.

and the promise will end up fulfilled.

(these abort the tasks from within the callbacks, but that's just to keep the sequencing simple; the abort could come from a concurrent task)

From #60, there is an argument that this is reasonable behavior from joining the callback's promise chain. Adapting the AbortSignal example from the DOM spec, it would be something like this in JS:

function currentPostTaskish(callback, {signal}) {
  return new Promise((resolve, reject) => {
    signal.throwIfAborted();

    signal.addEventListener('abort', () => {
      reject(signal.reason);
    });

    setTimeout(() => {
      try {
        const result = callback();
        resolve(result);
      } catch (e) {
        reject(e);
      }
    });
  });
}

When run with the // Fulfilled example above:

• callback() is run synchronously up to await new Promise(...) and a Promise is returned
• the resolve() within currentPostTaskish is called with the pending promise from callback()
• the later reject(signal.reason) from the abort event has no effect
• currentPostTaskish returns a Promise depending only on result, so will end up fulfilled

For the // AbortError example, however, the signal has a chance to run reject(signal.reason) before resolve(result) runs, so the overall Promise is rejected.

Two issues I found with this behavior:

• First, it appears that not all async code acts identically. Swapping the Promise-setTimeout for an awaited constant (so it's only triggering a microtask), suddenly the scheduler.postTask() call rejects again:

  const controller = new TaskController();
  scheduler.postTask(
    async () => {
      await 5;
      controller.abort();
    }, {signal: controller.signal}
  ); // AbortError.

  Notably this is not what you get from the currentPostTaskish stand-in above or the scheduler-polyfill (they both fulfill, not reject), and not what you'd expect from async/await/Promise chain code in general based on the above reasoning about postTask promise handling (an await is an await for callback to return as a Promise, even if only a microtask). I'm guessing it's something about the abort signal propogation being a real task at the browser level vs the event handling in currentPostTaskish, but I don't really know. Behavior is identical in Chrome and Firefox (with dom.enable_web_task_scheduling enabled).

• Second, it can cause the promise chain to fork when there's an abort signal and an exception is thrown in the callback:

  const controller = new TaskController();
  scheduler.postTask(
    async () => {
      controller.abort();
      throw Error('thrown in callback'); // Unhandled rejection.
    }, {signal: controller.signal}
  ); // AbortError.

  The scheduler.postTask() rejects with an AbortError, but there's also an unhandled rejection for the Error thrown in callback.

  This also shows up in cancelled scheduler.postTask() callbacks that contain a scheduler.yield() that inherit the AbortSignal (Chrome only for now).

  const controller = new TaskController();
  scheduler.postTask(
    async () => {
      controller.abort();
      await scheduler.yield(); // Unhandled AbortError.
    }, {signal: controller.signal}
  ); // AbortError.

  The scheduler.postTask() again rejects with an AbortError, but now the unhandled rejection is from the same AbortError instance.

Finally, since code could be handed an arbitrary function to run in a scheduler.postTask() (so it won't be known ahead of time which behavior above will be triggered by a mid-task abort signal), it seems better semantically that the signal is interpreted as either triggering rejection up until callback starts execution or up until callback has finished:

• the abort came during callback execution, so callback will continue to execute to its end. Therefore the scheduler.postTask() promise should be fulfilled to match that the callback finished executing without error
• As a JS developer, I understand my promise chain isn't going to be interrupted in the middle unless I add hooks to interrupt it, e.g. signal.throwIfAborted() if I need more break points. However, the task was aborted and regardless of what ended up executed, the returned promise's resolution should reflect that fact so subsequent code can react accordingly

I personally find the second approach to be the more compelling, especially because it means subsequent code can also be cancelled without plumbing the AbortSignal everywhere (which is part of the point of the async chain).

I didn't find much guidance on abort semantics out there, but this also seems to be at least "encouraged" by the spec:

APIs that rely upon AbortController are encouraged to respond to abort() by rejecting any unsettled promise with the AbortSignal's abort reason.

To accomplish this, postTask would wait until the return value of callback() is resolved before resolving or rejecting the returned Promise. The JS equivalent would be something like

function proposedPostTaskish(callback, {signal}) {
  return new Promise((resolve, reject) => {
    signal.throwIfAborted();

    signal.addEventListener('abort', () => {
      reject(signal.reason);
    });

    setTimeout(() => {
      new Promise(innerResolve => innerResolve(callback()))
        .then(resolve, reject);
    });
  });
}

[mmocny]

Super interesting, and excellent summary.

As a JS developer I expect that an aborted signal is just a suggestion to abort in the future, provider that: work can actually be saved by doing so. I don't necessarily expect a fully completed task/resolved promise, to get switched from resolved to rejected because of a very late abort.

(Aside) ...Using a different example that I had personally experienced before: a framework-level component re-rendering update, which is async and supports abort()... if I try to abort() it due to yet another re-render being posted, I want it to throw any in progress work away, but I might still want to accept the update if its already fully completed and scheduled.

However, given the inconsistencies and complexity you point out in the examples, I think your suggestion #2 seems like a better alternative to me. I think. Its complicated :P

[shaseley]

This is a great write-up, thanks.

One thing to note is that when browsers invoke a callback (e.g. the scheduler.postTask(callback)), there's a microtask checkpoint after invocation. So when we read the value returned by the function, it happens after running microtasks. That's why aborting after await 5 in your one example still rejects: we haven't received the result and resolved the promise on the browser side yet, so the abort still has an effect (abort races with promise resolution).

Maybe the following is reasonable summary?

The promise returned by postTask():

1. is always rejected if the signal is aborted before that task starts.
2. is always rejected if the signal is aborted during the task running the callback. This includes any microtasks that run in that task.
3. might be rejected if the callback returns a pending promise and the signal is aborted after this but before the promise is resolved:
   1. yes if passing the signal to another signal-accepting API after it was aborted (APIs typically throwIfAborted()).
   2. yes if the signal is aborted and causes a (chained) promise rejection, e.g. awaiting a fetch() that gets rejected.
   3. yes if manually checking throwIfAborted() before the async function ends
   4. no otherwise (unless non-signal related, e.g. throwing an error)

But having a simple invariant that "abort --> the promise is rejected unless the async task (function) has finished" is compelling. And to Michal's point, this could save work if it's in a promise chain. Furthermore, introducing scheduler.yield() points with inheritance is a nice extension: the postTask() promise would be rejected anyway, but the yield points provide an earlier place to abort (vs. inserting a yield point causing the rejection).

So I think I support this, but I'll have to think about/analyze the compat risk.