Add Subscriber#active boolean (take 2)

[domfarolino]

This closes #76. This PR introduces the subscriber.active boolean attribute that can be used from within an Observable to determine whether a subscription is still active. I am still personally a tiny but unsure whether this is really necessary, so I'd like to use this space to describe the semantics and how it differs from other APIs, so we can come to a conclusion on this, since @benlesh feels much more strongly about this API.

On the surface it seems that subscriber.active === !subscriber.signal.aborted¹, however this is not quite true. The lifecycle of an Observable is the following:

1. Active subscription
2. subscriber.complete()/error() is called from script
3. The subscription is no longer "active", meaning the bodies of those methods cannot re-entrantly invoke themselves or any method on the Observer dictionary for that matter.
4. The actual corresponding Observer#complete()/error() body is run
5. The Subscriber#signal is aborted (via an internal AbortController)
6. subscriber.signal.aborted is true
7. All teardown callbacks (added by an API that probably looks like subscriber.addTeardown()) will run, as part of the abort algorithms associated with subscriber.signal

The subscriber.active attribute intends to expose precisely when a subscription no longer becomes active (just before the Observer#complete()/error() body is run), which is before the subscriber.signal is finally aborted. The only observable consequence I can think of for this is inside the complete()/error() methods, where subscriber.active is already false but subscriber.signal.aborted is not yet true (since teardowns haven't run yet).

I think that subscriber.active is a more sensible and ergonomic way to express the concept of an "inactive subscription", however I only hesitate to add it because I don't love the idea of having two different ways of getting this information with slightly different semantics. I'm curious if anyone would rely on subscriber.active in a way that !subscriber.signal.aborted couldn't be used for (thus causing broken code).

---

Here's an I can think of where subscriber.signal.aborted is insufficient for stopping an infinite loop:

let stopPushingValues = true;

const observable = new Observable(subscriber => {
  // The subscriber's `complete()` handler fires an event that
  // forces this observable to synchronously push a firehose of
  // values to the subscriber.
  document.addEventListener('stuff', e => {
    while (!subscriber.signal.aborted) {
      if (stopPushingValues === false) {
        subscriber.next(1);
      } else {
        // In this case, the subscriber has signaled something
        // to us that it wants us to complete the subscription.
        subscriber.complete();
      }
    }
  });
});

const ac = new AbortController();
observable.subscribe({
  next: v => {
    stopPushingValues = true;
  },
  complete: () => {
    document.dispatchEvent(new Event('stuff'));
  }
});

document.dispatchEvent(new Event('stuff'));

// The sequence of events is this:
//   1. Random code fires a 'stuff' event at document
//   2. This causes the Observable to start pushing values synchronously
//      to the `next()` handler. Eventually the `next()` handler decides that
//      doesn't want anymore values, so it signals to the Observable that it
//      wants the Observable to call `complete()`[^1] to end the subscription.
//   3. Observable detects `stopPushingValues` (i.e., that the subscriber wants
//      the Observable to stop), and calls `Observer#complete()`.
//   4. Observer#complete() triggers another 'stuff' event, which re-enters the
//      event-pushing code in the subscriber callback. This starts another
//      synchronous loop that tries to push values *again* to the subscriber. But
//      this time no values can be pushed, because once `complete()` is entered ever,
//      it becomes impossible to invoke `Observer#next()/complete()/error()` methods.
//      But the `while` loop never stops!
//
//      So here's how you could get yourself into this buggy situation. After you call
//      `complete()`, it's true that `subscriber.signal.aborted` will be true. As a
//      developer, you might rely on this, by using `while (!subscriber.signal.aborted)`
//      as a condition in the while loop, so that the while loop breaks *after* you call
//      `complete()`. However there's a very subtle timing thing here that you might not
//      realize. `subscriber.signal.aborted` is only true *after* the call to `complete()`
//      is done. So if `complete()` itself re-enters the while loop, it will never terminate
//      because `subscriber.signal.aborted` is never `true` *from within* the call to
//      `complete()`.

// [^1]: Note that this is the contrived part of this example. Normally when a
// subscriber wants to be "done", it just `abort()`s the signal that was passed into
// `subscribe()`, to unsubscribe. But that doesn't cause the issue in this example,
// so we had to craft a more contrived way to trigger it.

How important is it to protect against this kind of (highly contrived!) scenario? Or are there way less contrived scenarios where subscriber.signal.aborted is a footgun, and where subscriber.active would be critical.

Footnotes

1. Remember of course that subscriber.signal is aborted absolutely whenever a subscription is ended, not just when the subscriber unsubscribes / aborts the signal that it passed into subscribe(). ↩

[domfarolino]

I'd love to hear from some others in the community about consideration of this API! If anyone, like perhaps @tbondwilkinson, @domenic, or @bakkot has any opinions I'd love to hear them, since I'm personally a little bit torn between the nice semantics that this API introduces, but I'm not sure how to weigh it against the actual user need / confusion of having two similar ways of gleaning the same information.

[tbondwilkinson]

I do view the concept of an AbortSignal differently from whether the subscriber is resolved.

Imagine a Promise, where someone passed in an AbortSignal, where they want the Promise to reject if the AbortSignal does. The concept of whether the Promise is resolved is separate from whether the AbortSignal has rejected.

So I do think it's probably worth adding? I would expect people would almost universally pick checking isActive over checking the AbortSignal?

[domfarolino]

I think that makes a lot of sense, thanks. I agree with the Promise analogy. I think I'm happy enough to land this now that we've gotten at least a little feedback, but if others feel strongly I'm open to hearing more discussion!