AbortController.prototype.timeout and AbortController.prototype.cancel

[jasnell]

As a complement to the new AbortSignal.timeout(), there are a couple more cases that may be worthwhile exploring, specifically with AbortController:

• abortController.timeout(delay[, reason])

  • Sets an internal timer on the AbortController to trigger the AbortSignal with reason after delay milliseconds.
  • If there is already an active internal timer, when timeout() is called, it is reset.
  • If abortController.abort() is directly called while there is an active internal timer, the signal is immediately triggered and the internal timer is cleared.
  • This is useful for implementing an ongoing "idle timeout" mechanism (e.g. the timer is reset whenever there is new activity but the signal is triggered when activity is not frequent enough).

• abortController.cancel()

  • Signals that the AbortController is no longer expected to trigger the AbortSignal at all, allowing the AbortSignal to clear it's 'abort' algorithms and indicating that it is expected to never trigger. This can be used as a signal that consuming code can safely ignore the signal after it becomes abandoned.
  • If called while there is an active internal timeout from abortController.timeout(), the internal timer is cleared.

Example:

const idleAborter = new AbortController();
// Controller is expected to trigger after 10 seconds
idleAborter .timeout(10_000);

const eventTarget = getSomeKindOfEventTarget({ signal: idleAborter .signal });

// Getting some kind of activity resets the internal timeout
eventTarget.addEventListener('activity', () => idleAborter.timeout(10_000));

// Signal that paying attention to the signal is no longer necessary and clears the timeout.
eventTarget.addEventListener('end', () => idleAborter.cancel());

[annevk]

TaskController uses setPriority(). Perhaps that means we should use setTimeout()?

If you can invoke that method multiple times, do we need cancel() or should passing 0 or null be sufficient to clear it?

[annevk]

cc @smaug---- @benjamingr @jakearchibald @MattiasBuelens @andreubotella

[benjamingr]

I think abortController.cancel would be too confusing since people would expect it to alias abort. I think these two use cases are interesting but are better enabled by working on new AbortController(...signalsToFollow) in terms of API

[MattiasBuelens]

I agree about the .cancel() name potentially being confusing. It also doesn't help that Streams already has readableStream.cancel() and writableStream.abort(). 😅 Perhaps abortController.close() is better? 🤔

The proposed .timeout() method seems quite complex, and I'm not sure if it's sufficiently general to be worth adding to the platform. I think it'd be better if we could first add some more "fundamental" utilities such as abortController.follow() (#920) and abortController.close(), on top of which you can build .timeout():

class TimeoutAbortController extends AbortController {
  #timer;

  timeout(delay) {
    if (this.#timer) {
      this.#timer.close(); // assuming this also cleans up the followed AbortSignal.timeout() signal
    }
    this.#timer = new AbortController();
    this.#timer.follow(AbortSignal.timeout(delay));
    this.follow(this.#timer.signal);
  }
}

[jasnell]

close() definitely works over cancel()

And I could get on board with follow with the exception that it's not much better/different than simply attaching an event listener to the signal.

class TimeoutAbortController extends AbortController {
  #timer;

  static #abort() { this.abort(this.#timer.reason); }

  timeout(delay) {
    this.#timer = AbortSignal.timeout(delay);
    this.#timer.addEventListener('abort', this.#abort, { once: true });
  }

  close() {
    this.#timer?.removeEventListener('abort', this.#abort);
    this.#timer = undefined;
    super.close();
  }
}

close() here is still useful, but the follow() feels superfluous.

[MattiasBuelens]

Fair point, follow() is probably overkill here. 😅

Nit-pick: you'll still want to remove the event listener from the old this.#timer at the start of timeout(delay), to handle this step:

If there is already an active internal timer, when timeout() is called, it is reset.

[jasnell]

@annevk:

If you can invoke that method multiple times, do we need cancel() or should passing 0 or null be sufficient to clear it?

We'd still want the close() (née cancel()) because it's actually doing more than clearing the timeout. It's signaling internally to its AbortSignal that it won't ever trigger it from that point forward, and that the AbortSignal can clear it's set of abort algorithms. Anything downstream that might follow the AbortSignal would see signal.closed = true and know that they don't actually need to follow the signal since it won't be triggered.

[jasnell]

I've opened a PR for abortController.close() and abortSignal.closed #1042

I'll table the proposal for abortController.timeout()

[domenic]

I think close()/closed still raises some tricky questions.

• What are the actual use cases? The original post does not mention any for cancel(), just for timeout().
• This is one of very few "manual resource management" operations in the web platform, and the first that is not tied to a large chunk of binary data. (Others I can think of are Blob/File, URL.revokeObjectURL(), and ImageBitmap.) Is this a reasonable precedent to set? Do we want to encourage a coding style that sprinkles close() calls throughout, like a C-style free()?
• If this is largely a primitive for implementing follow, we should consider instead doing follow (Consider adding AbortController.prototype.follow(signal) #920) directly.
• This would be the first bit of AbortSignal that is not implementable in "userland", i.e. it requires the ability to remove all listeners. Is that OK? Or should we perhaps expose the primitive in [Proposal] Add EventTarget.getEventListeners() #412 first?
  • On the other hand, exposing that primitive would go further than abortController.close() does, since it would be available to anyone who has a reference to the signal, not just the controller. We'd need some kind of thing where only the creator of the EventTarget has the ability to get or remove all listeners, to give this kind of separation.
  • On the third hand, this kind of worry about leaking event listeners is common for every EventTarget; there's nothing really special about AbortSignal.
• Is it OK that you can add abort listeners after closing? They won't be triggered by abort() (since abort() is a no-op after closing in the proposal), but they still exist.
• This further increases the division between spec "abort algorithms" and JavaScript abort listeners, since spec abort algorithms (including follow) have no effect on closed abort signals, but abort listeners still are present and can still be triggered. I and others have protested against this division before; although @annevk was not sympathetic, I hesitate to widen the gap.

[MattiasBuelens]

It's possible to not have close(), as long as all operations that add an abort algorithm also remove their abort algorithm once they've completed "normally".

Unfortunately, we're not even doing that in the specifications. See for example:

• EventTarget's add an event listener, step 5
• fetch() step 10
• Web Locks's request a lock, step 2
• Prioritized Task Scheduling's schedule a task to invoke a callback, step 6

The only spec I found that does this properly is Streams, where pipeTo() properly removes its abort algorithm in "finalize".

Maybe we should first fix how abort signals are used inside other specifications, and that might eliminate the need for a dedicated AbortController.close()? 🙂