Proposal: fetch with multiple AbortSignals

[jovdb]

I have some 'Higher Order functions' that can create me a fetch function with some extra behavior on it.

Some of them can abort the request.
Here a simplified example with a timeout that can abort:

const fetch = pipe(
  window.fetch,
  withTimeout({ timeoutInMs: 100 }), // 1 fetch can take max 100ms
  withRetry({ retryCount: 3, delayInMs: 100 }), // Retry 3x with a delay of 100ms
  withTimeout({ timeoutInMs: 400 }), // all fetches (with retry) can take max 400ms
)

In a real application, the second withTimeout could also be a withCancel function.

The first withTimeout will create an AbortController instance and set the signal property on the requestInit argument of fetch,
The second withTimeout will also create an AbortController and set the signal to the requestInit argument. This gives a problem because there already is a signal.

I could create for this sample one AbortController outside the withTimeout's and pass it to both withTimeout's as argument, but in a real application the enhancing of a fetch function can be done on different layers in the application. This causes other problems:

• I must know if a parent layer already added abort behavior, if so I must pass the same abortController instance to withTimeout.
• I must known which abortController instance is used for this fetch and have acces to it.

and this makes the code more dirty in my opinion.

What would be great is a way to pass multiple abort signals to a fetch function, some idea's:

• let fetch also accept an array of abortSignals
• provide a way to combine/merge two AbortSignal's into one AbortSignal

Here the example on CodePen

[annevk]

Given that AbortSignal has an event and can be constructed via AbortController, you should be able to create your own and do whatever, no?

[domenic]

I'm unclear on exactly the connection to the OP's problem, but the OP's suggestion at the bottom of their post of

provide a way to combine/merge two AbortSignal's into one AbortSignal

is definitely something we considered. I believe we just put it off to get v1 out the door. An AbortSignal.any([signal1, signal2, ...]) that aborts if any of the inputs are aborted makes sense to me. (Name bikeshedding possible.)

[jovdb]

@annevk I think you are right, I will try it on another way...
I would like to create an abortable chain of actions.
I think I will need to create my own 'aborter' object that can have a hierarchical structure where a parent will abort its children.
At the leafs of the tree where a fetch is, it should create an AbortController and pass the signal to the fetch.
I will rethink it and will post my workaround if I can get it to work,
Thanks

[jakearchibald]

fwiw

function anySignal(signals) {
  const controller = new AbortController();

  function onAbort() {
    controller.abort();

    // Cleanup
    for (const signal of signals) {
      signal.removeEventListener('abort', onAbort);
    }
  }

  for (const signal of signals) {
    if (signal.aborted) {
      onAbort();
      break;
    }
    signal.addEventListener('abort', onAbort);
  }
  
  return controller.signal;
}

[jovdb]

Useful function!
Small improvement, if one of the signals is already aborted, I would prefer to return an (or the) aborted signal instead of undefined.

if (signal.aborted) {
  onAbort();
  return signal;
}

[jakearchibald]

My bad, I should have used break, not return. I've updated the snippet.

[jovdb]

FYI: I added my experiments with abortable fetch on https://github.com/jovdb/fetch-hof
and in the end, I didn't need the multiple AbortSignals on fetch.

I have used the CancelToken proposal in my experiment.
I pass around this generic CancelToken from function to function, where each function can subscribe to the CancelToken and pass a new CancelToken to it children (if needed: like withTimeout)

The withFetchAbort higher order function will create an AbortController and pass the AbortSignal to fetch, it subscribes to the CancelToken. If the CancelToken is cancelled, it will call abortController.abort() to cancel the fetch.

[annevk]

@benjamingr this issue might be of interest to you. I'm folding it into whatwg/dom#920 and hope that's acceptable to everyone.

[gamliela]

Great comment from @jakearchibald.

Some proposed changes -

1. Propagate reason from the aborted signal to the wrapper signal;
2. Don't attempt unsubscribe if subscription hasn't been done yet;
3. Typescript added.

function anySignal(signals: AbortSignal[]) {
    const controller = new AbortController();
    const unsubscribe: (() => void)[] = [];

    function onAbort(signal: AbortSignal) {
        controller.abort(signal.reason);
        unsubscribe.forEach((f) => f());
    }

    for (const signal of signals) {
        if (signal.aborted) {
            onAbort(signal);
            break;
        }
        const handler = onAbort.bind(undefined, signal);
        signal.addEventListener('abort', handler);
        unsubscribe.push(() => signal.removeEventListener('abort', handler));
    }

    return controller.signal;
}

[jakearchibald]

fwiw, it can be simplified further:

function anySignal(signals: AbortSignal[]): AbortSignal {
  const controller = new AbortController();

  for (const signal of signals) {
    if (signal.aborted) return signal;

    signal.addEventListener("abort", () => controller.abort(signal.reason), {
      signal: controller.signal,
    });
  }

  return controller.signal;
}

…now that signals can be used to remove event listeners.

[gamliela]

Nice! Yeah I just learned about it yesterday. It’s not supported by node.js yet but is supported by Jsdom so should be good for testing browser code. Re. implementation, there is a memory leak if returning early, it’s important to abort the controller too.

…

________________________________ From: Jake Archibald ***@***.***> Sent: 11 February 2023 12:55 AM To: whatwg/fetch ***@***.***> Cc: Alon Gamliel ***@***.***>; Comment ***@***.***> Subject: Re: [whatwg/fetch] Proposal: fetch with multiple AbortSignals (#905) fwiw, it can be simplified further: function anySignal(signals: AbortSignal[]) { const controller = new AbortController(); for (const signal of signals) { if (signal.aborted) return signal; signal.addEventListener("abort", () => controller.abort(signal.reason), { signal: controller.signal, }); } return controller.signal; } …now that signals can be used to remove event listeners. — Reply to this email directly, view it on GitHub<#905 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AB7TOENBK3FI6QQ6EF5TGETWWYUERANCNFSM4HMG6GOA>. You are receiving this because you commented.Message ID: ***@***.***>

[benjamingr]

It’s not supported by node.js yet but is supported by Jsdom so should be good for testing browser code.

I'm pretty sure it is.

[jakearchibald]

@gamliela I don't think there's a memory leak there. Maybe this explains it https://jakearchibald.com/2020/events-and-gc/

And Node does support signals.

[gamliela]

@jakearchibald My comment about node.js was based on MDN docs - they must be outdated then.

I think the AbortController stays in memory until it gets aborted. In case of early return this may happen much after the return (or never happen), hence the memory leak.

I spent some time on this and created the example below. Run test1() in memory profiler with and without the commented line below. Check when the 80MB are deallocated in each case. WIth this line, it get deallocated immediatelly. Without this line, it get deallocated only after 10 seconds.

function anySignal(signals) {
  const controller = new AbortController();
  controller.data = Array(20000000).fill(1);

  for (const signal of signals) {
    if (signal.aborted) {
      //controller.abort(); // <-------- this
      return signal;
    }

    signal.addEventListener("abort", () => controller.abort(signal.reason), {
      signal: controller.signal,
    });
  }

  return controller.signal;
}

function test1() {
  const signal1 = AbortSignal.timeout(10000);
  const signal2 = AbortSignal.abort();
  const signal = anySignal([signal1, signal2]);
  signal1.addEventListener('abort', () => console.log('done!'));
}

[jakearchibald]

I think the AbortController stays in memory until it gets aborted

That isn't true. It can be GC'd like everything else in JS.

[gamliela]

I was talking in the context of the code, not about AbortController's in geneal. Of course they can be GC'd. Have a look at the code.

[benjamingr]

We have a utility for this in node called util.aborted that doesn’t leak the callback

[pauldraper]

@jakearchibald there is a memory leak if a not-first signal has already been aborted.

The fix is to add controller.abort(signal.reason) to the early exit.

function anySignal(signals: Iterable<AbortSignal>): AbortSignal {
  const controller = new AbortController();

  for (const signal of signals) {
    if (signal.aborted) {
      controller.abort(signal.reason);
      return signal;
    }

    signal.addEventListener("abort", () => controller.abort(signal.reason), {
      signal: controller.signal,
    });
  }

  return controller.signal;
}

[slavafomin]

fwiw, it can be simplified further:

function anySignal(signals: AbortSignal[]): AbortSignal {
  const controller = new AbortController();

  for (const signal of signals) {
    if (signal.aborted) return signal;

    signal.addEventListener("abort", () => controller.abort(signal.reason), {
      signal: controller.signal,
    });
  }

  return controller.signal;
}

…now that signals can be used to remove event listeners.

@jakearchibald I'm afraid there is a bug in your implementation. When you do: if (signal.aborted) return signal; you are not unsubscribing listeners added in previous loop iterations, so they continue to listen for events. As @pauldraper has mentioned, we need to additionally call controller.abort() in this case to unsubscribe prior listeners.

So the final code should look like this:

function anySignal(signals: AbortSignal[]): AbortSignal {
  const controller = new AbortController();

  for (const signal of signals) {
    if (signal.aborted) {
      controller.abort();
      return signal;
    }

    signal.addEventListener("abort", () => controller.abort(signal.reason), {
      signal: controller.signal,
    });
 }

  return controller.signal;
}

By the way, there is no need to pass signal.reason as @pauldraper did, because this signal is not returned from the function in the first place due to the early exit and is solely used to trigger listeners deregistration.

---

Here's my version of the function with a single return statement, optional rest arguments and comments:

/**
 * Returns an abort signal that is getting aborted when
 * at least one of the specified abort signals is aborted.
 *
 * Requires at least node.js 18.
 */
export function anySignal(
  ...args: (AbortSignal[] | [AbortSignal[]])

): AbortSignal {

  // Allowing signals to be passed either as array
  // of signals or as multiple arguments.
  const signals = <AbortSignal[]> (
    (args.length === 1 && Array.isArray(args[0]))
      ? args[0]
      : args
  );

  const controller = new AbortController();

  for (const signal of signals) {

    if (signal.aborted) {
      // Exiting early if one of the signals
      // is already aborted.
      controller.abort(signal.reason);
      break;
    }

    // Listening for signals and removing the listeners
    // when at least one symbol is aborted.
    signal.addEventListener('abort',
      () => controller.abort(signal.reason),
      { signal: controller.signal }
    );

  }

  return controller.signal;

}

---

@jakearchibald could you please update your example to fix this? I'm afraid people from Google search could accidentally copy it with the memory leak in it.

[fregante]

It looks like this is now possible with the new AbortSignal.any() API, available in Chrome 116+ (no Firefox/Safari yet)

Example:

fetch(url, {
	signal: AbortSignal.any([
		userCancelSignal,
		AbortSignal.timeout(10000),
	]),
});

• https://chromestatus.com/feature/5202879349522432
• https://bugs.webkit.org/show_bug.cgi?id=256176

In the meanwhile I also published a helper to combine them cross-browser:

• https://github.com/fregante/abort-utils

[suhaotian]

Try this module https://github.com/jacobheun/any-signal