AbortSignal consumers can potentialy emit abort event on it

[AMorgaut]

As many developers coming to the AbortController/AbortSignal API, I got a bit concerned by its "complexity", meaning... "Not a big deal but... Couldn't the signal & controller classes be merged?"

The main raised reason was about separation of concerns, forbidding a consumer to be able to call abort() on the signal

see:

• Abort controller design #438 (comment)
• Aborting fetch web-platform-tests/wpt#6484 (comment)

But looking at the API specification, I don't see anything preventing the consumer from calling signal.dispatchEvent(new Event('abort'));

I don't see much control on that topic in polyfills neither

Am I missing something ?

[annevk]

A consumer could do that and depending on who looks at the event (and how, e.g., isTrusted will be false) they might end up confused. (Note though that specifications do not use the event as a trigger.)

[AMorgaut]

Note though that specifications do not use the event as a trigger

That's also something that scares me
We may end up in a situation where the 'abort' event listeners are called while the aborted property might still be false

[annevk]

If someone gets a handle on your signal object they can also make the aborted property lie, right? There's a lot of potentially surprising cases due to the way JavaScript and the web platform in general work, but this doesn't seem like anything out of the ordinary.

But maybe I'm misunderstanding the threat model.

[AMorgaut]

If someone gets a handle on your signal object they can also make the aborted property lie, right?

That part is secured by the spec. aborted is a readonly property

If we really want the Controller/Signal distinction to make sense, what could potentially be done, while AbortSignal extends EventEmitter, is to tell that its extended dispatchEvent(event) method would fail (throw?) when called, at least if the provided event type is abort

Regarding polyfills, it's still easy with Symbols to let the AbortSignal expose access to the super.dispatchEvent(event) more privately to the AbortController

Out of that topic, the EventEmitter dependency prevents this API from following the same path than Promises to the ECMA spec and then to core JS engines shared with servers, react-native, ... ;-(

[annevk]

signal = new AbortController().signal;
Object.defineProperty(signal, "aborted", { value: 1 });
console.log(signal.aborted); // 1

[AMorgaut]

To me such code example, if not fixed, is only removing any interest to separated the controller from the signal.

If the API is meant to provide a real "protection" that some frameworks, libraries, vendor featured API might want, it should probably be done completely.

(Don't get me wrong @domenic I really appreciate the huge work done on CancelablePromise, and then AbortController)

There's a lot of potentially surprising cases due to the way JavaScript and the web platform in general work, but this doesn't seem like anything out of the ordinary.

I agree, but I also think that there is a global effort to improve the web platform against existing leaks & inconsistencies. I'm thinking of things like sandboxes, ES Object.freeze()/Object.seal() & getter properties, upcoming private fields...

controller = new AbortController();
signal = controller.signal;
Object.seal(signal);
Object.defineProperty(signal, "aborted", { value: 1 });

VM165:3 Uncaught TypeError: Cannot define property aborted, object is not extensible
    at Function.defineProperty (<anonymous>)
    at <anonymous>:3:8


console.log(signal.aborted); // false
controller.abort();
console.log(signal.aborted); // true

So nothing prevent from enforcing this API isolation if we want a "safe" signal

[annevk]

I still don't really understand the threat model or what you think this API is meant to provide. Its design follows established precedent of many web platform classes. If you cannot depend on this class, there are many other cases you cannot depend on either, so I'm not sure why we're discussing this in isolation.

[Jamesernator]

Apologies for the length.

the threat model

While there might be some security use cases the bigger problem is not security threats but rather the developer hazards it presents.

Implicit Contracts

The root of problems like introduced with AbortSignal is in a family I'm going to call implicit contracts.

Developers tend to have a good idea of what an object can and should do by the interface it provides.

Example

Let's take Promise as an example. One of the most basic invariants of promises is that if I have a promise I can call .then on it and when the promise is resolved my callback will be called with the resolve value and that the value returned is a new Promise that I can chain further.

However nothing is enforcing that this assumption work, there's no technical reason Promise.prototype.then couldn't be deleted or replaced with something completely different.

But people don't do delete Promise.prototype.then or change it for the most part. Why? Because developers accept that there's an implicit contract on promises that guarantees these methods will be available.

But where does this implicit contract come from? Well it's from a number of sources:

• Documentation on Promises
• The specification
• Past experience with pains of mutating globals
  • Or trust of educators warning against mutating globals

These sources combine to give developers an good idea of what the implicit contract for a given programming feature is and what is and isn't expected behavior.

The problem with AbortController/AbortSignal

The main problem with AbortController/AbortSignal is that because AbortSignal is an EventTarget through documentation and specification developers will treat that as it's implicit contract.

However I consider this problematic as in the majority of cases .dispatchEvent on AbortSignal is not what most developers will want. As such for a lot of developers the implicit contract will shift towards assuming that the abort-event will be fired exactly once because this is the overwhelmingly more likely use.

But because AbortSignal is clearly defined as an EventTarget and in popular documentation some developers will reasonably assume that they can use .dispatchEvent on it to cancel early without realizing others might've been relying on a different implicit contract.

Browsers don't even use this behavior

This is more frustrating because the more-common behavior that most people will probably use is exactly what the specification uses internally via special steps: https://dom.spec.whatwg.org/#abortsignal-add . This is specifically to sidestep the potential hazard that developers might fire 'abort' on AbortSignal.

In order to simulate the browser's behavior a developer would need to do:

function handler(event) {
  if (event.isTrusted) {
    // cancellation steps

    signal.removeEventListener(handler);
  }
}

if (!signal.aborted) {
  signal.addEventListener('abort', handler);
}

This seems unreasonable for the common-case to avoid the uncommon-but-possible case and most developers won't bother leading to potential bugs when it is fired twice unexpectedly.

So why bother having it?

If both the majority use case and specification internals expect that the abort handler is only fired once then why allow otherwise?

While I don't think this is necessarily a big deal as .dispatchEvent on an AbortSignal is likely to be fairly rare. I do think it is problematic as it's indicative a of a problem I've noticed with a lot of web specifications.

Similar mistakes again and again

I think in general these problems are indicative of a larger problem with web specification developments than just AbortController and AbortSignal. I think it's indicative of a mindset of favoring consistency with some vague "web way" of doing things rather than favoring well thought out designs that don't add complexity for little gain.

For example AbortController/AbortSignal behave like this because it's consistent with EventTarget, but why was consistent with EventTarget necessary to begin with?

Being consistent with EventTarget doesn't offer clear advantage to developers in the majority of use cases and adds hazards instead for the majority of cases, even specifications like fetch have to avoid those hazards.

I've observed decisions like this again and again over a wide number of web specifications and I don't understand why well-understood problems keep being re-introduced to the web.

My personal experience

I don't have so much of a problem with these sort've things in my current job but in a past job the design of web things definitely had a large amount of problems due to implicit contracts.

In my previous job dealing with deep mutation and cross-cutting effects was a huge problem as it caused a lot of bugs and made things very difficult to reason about. While the majority of the problems were with arrays and objects being mutated deep in the call stack, I did experience problems with .dispatchEvent specifically firing events at unexpected times.

And what led to these problems? Well other developers there assumed a different implicit contract about things in the system than myself or others.

And that's the problem, if developers have different assumptions about implicit contracts of objects it makes it hard to work together as assumptions will be violated.

At my new job this is overwhelmingly less of a problem due to a better test suite and TypeScript which allows encoding contracts more obviously. For example in TypeScript if something takes readonly string[] then I can be confident passing it a mutable array and that it won't edit it.

If you read only one part:

Poor decisions in web specification development have real impacts on developers ability to do their work. Some problems can be solved by tools like TypeScript or test suites however this still puts burden on developers to use those tools correctly.

When developing web specifications please do not make things that have wider implicit contracts than necessary to solve the problem, if the implicit contract is too wide pitfalls and footguns WILL arise and cause developers problems as I have experienced myself.

[annevk]

Browsers don't even use this behavior

Browsers don't use .then() all the time either, iirc, so I'm not sure why the comparison with promises doesn't hold.

And reusing existing patterns is primarily done to avoid introducing new categories of bugs and design issues. If there's a better way to do something, it'll get popular and eventually adopted (see promises), but thus far that hasn't happened for events.

[Jamesernator]

Browsers don't use .then() all the time either,

I think it's problematic with other features from ECMA262 that there's hidden features. I strongly wish both hosts (browsers, +others e.g. Node) would use the same interface to things which they are started too (I'll elaborate on this at the bottom).

but thus far that hasn't happened for events.

But this presupposes the solution for cancellation needed to be events. The primary reason AbortController was introduced was so that there'd be a way to cancel fetches, this doesn't require events.

This is something I've noticed and find unfortunate with web standards generally and something I think the TC39 overwhemingly does well.

I see there to be almost a fork with 2 different "Javascripts", the WHATWG and W3C where everything is exposed via WebIDL with separate specs that depend heavily on each other to function. And the TC39 version where everything is self-contained and offers clear extension points but avoids having lots of cross-dependencies.

There's also a significantly different processes, with the TC39 there's a clear process with stages that are extremely thorough for a feature to reach completion, and the WHATWG and W3C where design is less thorough but gets delivered a lot more quickly.

However I do find the consequence of this kinda annoying, for the most part since the stage-system for ECMA2562 was introduced I think the features produced are highly polished, very future proof and widely applicable to their use cases. However results do tend to happen a lot slower (at times bordering on a snail's pace) and it can a lot of time for simple changes to come to fruition.

On the flip side I've noticed the WHATWG and W3C move significantly faster but at a noticeable drop in quality of solutions that aren't as widely applicable or contain fewer anti-features and footguns.

Relating this to AbortController/AbortSignal I am aware one of the reasons it was introduced is that the TC39 was moving too slowly on development of Cancellation and fetch needed something sooner.

However because of the fairly fast nature of WHATWG's development existing features were used leading to a situation where we have something that is less widely applicable† and less polished* than what would've been produced if this were considered as a fundamental primitive that could be used widely throughout all Javascript code, not just browsers.

This has knock on effects, because in adopting AbortController you yourself would then oppose developing something new that is widely applicable.

† Because anyone who wants to use AbortController as a cancellation now primitive has to not only implement AbortController but also DOM Events and DOMExceptions from WebIDL. For a fundamental primitive of cancellation bringing "DOMException" seems almost silly in environments that don't require a DOM.

* Compared to something like Cancellation in C# which is widely supported and ties into all parts of the system well.

---

To just give you an idea of how ridiculous I find it when saying just use AbortController as the primitive for cancellation, this is the IDL required to implement a consistent interface:

IDL Interfaces required

interface AbortController {
  constructor();

  [SameObject] readonly attribute AbortSignal signal;

  void abort();
};

[Exposed=(Window,Worker)]
interface AbortSignal : EventTarget {
  readonly attribute boolean aborted;

  attribute EventHandler onabort;
};

[Exposed=(Window,Worker,AudioWorklet)]
interface EventTarget {
  constructor();

  void addEventListener(DOMString type
, EventListener? callback
, optional (AddEventListenerOptions or boolean) options
 = {});
  void removeEventListener(DOMString type
, EventListener? callback
, optional (EventListenerOptions or boolean) options
 = {});
  boolean dispatchEvent(Event event
);
};

callback interface EventListener {
  void handleEvent
(Event event
);
};

dictionary EventListenerOptions {
  boolean capture = false;
};

dictionary AddEventListenerOptions : EventListenerOptions {
  boolean passive = false;
  boolean once = false;
};

[Exposed=(Window,Worker,AudioWorklet)]
interface Event {
  constructor(DOMString type
, optional EventInit eventInitDict
 = {});

  readonly attribute DOMString type;
  readonly attribute EventTarget? target;
  readonly attribute EventTarget? srcElement; // historical
  readonly attribute EventTarget? currentTarget;
  sequence<EventTarget> composedPath();

  const unsigned short NONE = 0;
  const unsigned short CAPTURING_PHASE = 1;
  const unsigned short AT_TARGET = 2;
  const unsigned short BUBBLING_PHASE = 3;
  readonly attribute unsigned short eventPhase;

  void stopPropagation();
           attribute boolean cancelBubble; // historical alias of .stopPropagation
  void stopImmediatePropagation();

  readonly attribute boolean bubbles;
  readonly attribute boolean cancelable;
           attribute boolean returnValue;  // historical
  void preventDefault();
  readonly attribute boolean defaultPrevented;
  readonly attribute boolean composed;

  [Unforgeable] readonly attribute boolean isTrusted;
  readonly attribute DOMHighResTimeStamp timeStamp;

  void initEvent(DOMString type
, optional boolean bubbles
 = false, optional boolean cancelable
 = false); // historical
};

dictionary EventInit {
  boolean bubbles
 = false;
  boolean cancelable
 = false;
  boolean composed
 = false;
};

[Exposed=(Window,Worker),
 Serializable]
interface DOMException { // but see below note about ECMAScript binding
  constructor(optional DOMString message
 = "", optional DOMString name
 = "Error");
  readonly attribute DOMString name;
  readonly attribute DOMString message;
  readonly attribute unsigned short code;

  const unsigned short INDEX_SIZE_ERR = 1;
  const unsigned short DOMSTRING_SIZE_ERR = 2;
  const unsigned short HIERARCHY_REQUEST_ERR = 3;
  const unsigned short WRONG_DOCUMENT_ERR = 4;
  const unsigned short INVALID_CHARACTER_ERR = 5;
  const unsigned short NO_DATA_ALLOWED_ERR = 6;
  const unsigned short NO_MODIFICATION_ALLOWED_ERR = 7;
  const unsigned short NOT_FOUND_ERR = 8;
  const unsigned short NOT_SUPPORTED_ERR = 9;
  const unsigned short INUSE_ATTRIBUTE_ERR = 10;
  const unsigned short INVALID_STATE_ERR = 11;
  const unsigned short SYNTAX_ERR = 12;
  const unsigned short INVALID_MODIFICATION_ERR = 13;
  const unsigned short NAMESPACE_ERR = 14;
  const unsigned short INVALID_ACCESS_ERR = 15;
  const unsigned short VALIDATION_ERR = 16;
  const unsigned short TYPE_MISMATCH_ERR = 17;
  const unsigned short SECURITY_ERR = 18;
  const unsigned short NETWORK_ERR = 19;
  const unsigned short ABORT_ERR = 20;
  const unsigned short URL_MISMATCH_ERR = 21;
  const unsigned short QUOTA_EXCEEDED_ERR = 22;
  const unsigned short TIMEOUT_ERR = 23;
  const unsigned short INVALID_NODE_TYPE_ERR = 24;
  const unsigned short DATA_CLONE_ERR = 25;
};

This is a crazy amount of complexity an environment needs to add just to add a "primitive" for cancellation.

Especially when all the use cases for cancellation can be done within a fairly tiny interface:

Simplied AbortController

class AbortSignal {
  #getAbortState;
  #addHandler;
  #removeHandler;

  constructor({ getAbortState, addHandler, removeHandler }) {
    this.#addHandler = addHandler;
    this.#removeHandler = removeHandler; 
    this.#getAbortState = getAbortState;
  }

  get aborted() {
    return this.#getAbortState();
  }

  register(abortHandler) {
    this.#addHandler(abortHandler);
  }

  unregister(abortHandler) {
    this.#removeHandler(abortHandler);
  }
}

class AbortController {
  #aborted = false;
  #handlers = new Set();

  constructor() {
    this.signal = new AbortSignal({
      getAbortState: () => this.#aborted,
      addHandler: (abortHandler) => {
        if (!this.#aborted) {
          this.#handlers.add(abortHandler)
        }
      },
      removeHandler: (abortHandler) => this.#handlers.delete(abortHandler),
    });
  }

  abort() {
    if (this.#aborted) return;
    for (const handler of this.#handlers) {
      try {
        handler();
      } catch (err) {
        // Perhaps through an AggregateError?
      }
    }
  }
}

[annevk]

I guess I don't find it a lot of overhead as I suspect most environments need events. And whether to use events and how was something that was carefully considered during the design of this feature, which took quite a while. Also, anything that needs cancelation will use AbortController going forward.

[Jamesernator]

I guess I don't find it a lot of overhead as I suspect most environments need events.

Node has an event system too already, I know that they're generally not very interested in adding a second similar event system if they can avoid it.

I have noticed a sort've "holier than thou" attitude quite a lot in the WHATWG towards Node and it's quite frustrating given that in a lot of cases a simpler design would help a lot for simpler interoperability.

[annevk]

I've put some effort into finding solutions that work for both Node's event system and the one defined in the DOM Standard, but nothing has gained much traction so far (most recently EventEmitter which I think has been abandoned). Anyway, I definitely support more exploration of that to see how that can be bridged.