Re: [whatwg/dom] AbortSignal consumers can potentialy emit abort event on it (#784)

> 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](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal) 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:

```js
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.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/784#issuecomment-569866661

Received on Tuesday, 31 December 2019 05:26:56 UTC