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()? 🙂

[jasnell]

For an example use case from Cloudflare Workers...

First, some background:

The fetch Request object can receive an AbortSignal as part of it's initialization. It also has it's own AbortSignal that the spec calls "this's signal". On initialization of the Request object, "this's signal" is set to follow the signal passed on init only if the init signal is provided. If the init signal is not provided, then there is nothing that will ever trigger this's signal.

When the Request object is cloned, the clones signal is set to follow this's signal, which may not ever actually be triggered by anything.

In the Workers implementation of fetch, we are able to use the fact that we know that certain AbortSignals will never actually be aborted as an indicator that we are able to ignore it safely. In such cases, we are simply able to skip adding the additional logic that handles the cancelation. We keep track of whether an AbortSignal can actually be triggered by anything using an internal flag that is essentially equivalent to the proposed closed flag (it's currently named something else but it doesn't matter).

The tl;dr is -- It's helpful to know if I actually need to pay attention to the AbortSignal. If there's nothing that can trigger it, then I can ignore it. Given the note in the spec: "... an API observing the AbortSignal object can choose to ignore them.", then "closed" is effectively a strong hint that it can do so.

Whether that's enough of a justification is up for debate :-)

@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".

It's not entirely that simple. Many uses simply can't remove their abort algorithms if they no longer have a handle to the algorithm itself. For instance, passing in an anonymous handler signal.addEventListener('abort', () => { ... }), or adding those in a different scope.

@domenic:

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.

I think so. Having the closed attribute set is enough of a signal. If someone ignores it and adds handlers anyway, then that's on them.

[MattiasBuelens]

On initialization of the Request object, "this's signal" is set to follow the signal passed on init only if the init signal is provided. If the init signal is not provided, then there is nothing that will ever trigger this's signal.

It sounds like it might be easier to allow "this's signal" to be undefined if no init.signal was provided. I'm not entirely sure why every request must have a signal, most other specs perform an "if signal is present" or "if signal is not null" check first. Maybe we should change this in Fetch?

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".

It's not entirely that simple. Many uses simply can't remove their abort algorithms if they no longer have a handle to the algorithm itself. For instance, passing in an anonymous handler signal.addEventListener('abort', () => { ... }), or adding those in a different scope.

Indeed, you'd have to store a reference to the added abort algorithm somewhere, probably in an internal slot. But that's also the case for "regular" event listeners, and developers already know how to do that.

[jasnell]

It sounds like it might be easier to allow "this's signal" to be undefined if no init.signal was provided. I'm not entirely sure why every request must have a signal, most other specs perform an "if signal is present" or "if signal is not null" check first. Maybe we should change this in Fetch?

That would work in theory but given that it's currently always expected, changing that to undefined in certain cases would be a breaking change that I don't know if we can get away with.

[MattiasBuelens]

Riiiiight, a request's signal is exposed as Request.prototype.signal. Never mind... 😅

[Jamesernator]

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()?

• 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?

• 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.

I opened another issue a while ago to allow attaching trusted-event only listeners which would essentially allow for the lifetime of listeners to be tied to the controller, NOT to the event-target itself.

i.e. We could do:

let controller = new AbortController();
controller.signal.addEventListener(
    "abort",
    () => { console.log("Aborted!"); },
    { trustedOnly: true },
);

and if the controller performs .abort() then it can immediately collect all listeners with trustedOnly: true as the only way to create a trusted event is via the controller. Similarly if controller is collected then all such trustedOnly: true listeners can be collected as well.

Late listeners similarly could just be immediately discarded as they will never be callable if the controller has been collected or already aborted.

The opt-in of the flag is the main thing I don't like about the idea, but it could be possible for certain events to have trustedOnly: true be default.