Add a utility for knowing an AbortSignal was aborted

[benjamingr]

Hey,

Speaking to library authors in the ecosystem it appears that this pattern of code is very common (also in our code):

if (signal.aborted) {
  // cleanup
} else {
  const listener = (err) => {
    // cleanup  
  };
  signal.addEventListener('abort', listener);
  resource.on('done', () => signal.removeEventListener('abort', listener);
}

It would be very useful to be able to write this code in a more ergonomic way, talking to @getify about this in the CAF repo a utility was suggested:

const { once } = require('events');

// returns a promise for when the signal was aborted
async function aborted(signal, resource = null) {
  if (signal.aborted) return; // early return on aborted signal
  // the kWeak bit not implemented yet is so that the event listener doesn't leak
  await once(signal, "abort", { [kWeak]: resource });
}

Which would let you do

await aborted(signal);
// Or
// or some other resource, when the request gets GCd the listener gets removed automatically
await aborted(signal, request);

Any opinions on this? (Personally I am in favour) If we add such an API under what module would it live?

cc @jasnell @addaleax ?

[benjamingr]

(I am happy to implement - though I haven't landing the weak event handlers PR yet because I am very hesitant about it)

[getify]

The common usage pattern I anticipate for this util is:

// ..
await Promise.race([
   promise1,
   promise2,
   aborted(signal)
]);

FWIW, in CAF, the promise I create for an abort signal actually rejects when the token is aborted... this makes detection of the abort a little easier (like with try..catch around the await). I think it would be more useful if aborted(..) made a promise that rejects rather than resolves. If not, I just have to create some more machinery around it to not create breaking changes in CAF.

In any case, I would envision shipping some sort of polyfill for this util along with CAF, because CAF is designed to work in both Node and the browser. So interop with the browser env is important here. Maybe it's a good idea to float such a util with WHATWG (or whoever) to see what their interest level is?

[benjamingr]

@getify I think we'll use it a ton in core as well, we currently have a ton of code that looks like:

if (signal.aborted) {
  // cleanup
} else {
  const listener = (err) => {
    // cleanup  
  };
  signal.addEventListener('abort', listener);
  resource.on('done', () => signal.removeEventListener('abort', listener);
}

That would become:

aborted(signal, resource).then(() => /* cleanup */);

Other than being significantly shorter the selling point for me here is that the code is significantly safer as well - since you don't have to remember to remove the listener to the signal.

Maybe it's a good idea to float such a util with WHATWG (or whoever) to see what their interest level is?

Sure, I'll open an issue.

[getify]

I'm also wondering if something like what was done with util.promisify(..) could be done here? That utility looks for a symbol on the function that has its already-provided promisified implementation, and transparently returns it if so. It only creates a new function if that symbol key isn't provided.

Could signal have something like that on it, either the promise itself, or a function to vend the promise, and then the aborted(..) util looks for that, and if it finds it, just returns, and if not, it attaches the event?

This approach would make it significantly easier for me to migrate CAF to using this util without introducing a major breaking change.

[benjamingr]

@getify thanks, opened whatwg/dom#946 I think we can ship aborted experimentally (if James/Anna/other Node people agree it's interesting to explore) regardless of the outcome of the standards effort.

I feel strongly that if we want to pursue a standards contribution rather than a Node.js utility we should not ship a experimental utility in the meantime but I am very interested what Jake/Domenic/Anne have to say.

Could signal have something like that on it, either the promise itself, or a function to vend the promise, and then the aborted(..) util looks for that, and if it finds it, just returns, and if not, it attaches the event?

I think it's worth exploring. I think one of the cool bits is that if it's a utility and we don't actually have to call addEventListener we can just bind the lifetime of the listener to the lifetime of the resource which would negate the need to actually remoe the listener.

Can you write a quick code example of what you mean here so I make sure we're on the same page?

[getify]

Can you write a quick code example of what you mean here so I make sure we're on the same page?

I was sorta envisioning logic like this:

async function aborted(signal) {
   if (signal[Symbol.abortPromise]) {
      return signal[Symbol.abortPromise];
   }

   // ..
}

In CAF, then, I could continue to create the promise myself, and instead of (or in addition to) attaching it as a .pr property, I could just set it on that symbol location.

Then, CAF-using code would just look like:

Promise.race([
   // whatever
   aborted(signal)
])

...instead of needing the special casing like:

Promise.race([
   // whatever

   signal.pr || aborted(signal)
])

The promise I vend in CAF could reject instead of fulfill, so that CAF's logic and use-cases are still preserved. But then Node could create a fulfilling-promise by default, if that's more desired.

---

BTW, the reason CAF needs to vend its own promise is not just that reject semantic... but also because CAF relies on being able to pass along a specific reason to that promise rejection, so that it's easy to detect in catch(..) handlers that the cancellation occured.

[jasnell]

Definitely think some utility code would be helpful. I'd actually rather not optimize it for Promise.race() given how many times I see that pattern abuse in really fundamental ways.

Let's back up and review the requirements for a bit... what exactly is needed...

1. Determine if a signal is provided.
2. Determine if a signal has already been aborted (early bail out)
3. Attach an abort listener that cancels the expected action
4. Run the expected action
5. If, after the action is completed, the signal has been triggered, perform cleanup
6. Otherwise, remove the abort listener and continue...

Given that, a wrapper such as the following would work... (haven't tested and in a hurry typing this so may be errors but you should get the idea)

async function cancelable(asyncFn, cancelFn, signal) {
  return funcction (...args) {
    if (signal.aborted)
	  throw new AbortError();
	
	signal.addEventListener('abort', cancelFn, { once: true };
	try {
	  return await Reflect.call(this, args);
	} finally {
      signal.removeEventListener('abort', cancelFn);
    }
  }
}

// ....
const ac = new AbortController();
async function foo() {}
async function cancelFoo() {}

const fn = cancelable(foo, cancelFoo, ac.signal);

await fn(1, 2, 3);

As I said, this is rough but something like this should work to address the common pattern.

[getify]

I'd actually rather not optimize it for Promise.race()

If it's not exposed as a promise, it won't be of any use to CAF (or other libs of the same vein), which has a few years head start on this effort here in terms of how it's modeled async cancellation via a promise attached to the abort signal, and the heavy use of Promise.race(..) to stop waiting on a promise when a cancellation is triggered.

[debadree25]

Is there still interest in this? I would be interest in giving this a try

If we add such an API under what module would it live?

Also same question

[benjamingr]

Feel free to pick this up!