Do we need the `subscriber.closed` attribute?

[domfarolino]

In a review of some Observable constructor web platform tests, the Subscriber#closed attribute was brought up. I hadn't heard of this before, and while I can imagine this could be useful, a concrete practical use-case doesn't really jump out at me, so I'm filing this to spur some discussion about why this might be useful / if it is really needed.

Please see all of the discussion starting at https://github.com/web-platform-tests/wpt/pull/42219#discussion_r1341427510 and https://github.com/web-platform-tests/wpt/pull/42219#discussion_r1361243283.

[domfarolino]

https://github.com/web-platform-tests/wpt/pull/42219#discussion_r1361243283 has me just a little confused. It sounds like the AbortSignal passed into subscribe() is expected to be different than the AbortSignal that the Observable can pluck off of Subscriber#signal. That seems fine, but the purpose of this is to ensure that subscriber.signal.aborted is true whenever the observable itself completes the subscription. Assuming subscriber.signal.aborted is also true when the subscriber aborts its subscription, then it seems that subscriber.signal.aborted will always equal subscriber.closed, in which case I wonder if we have a strong reason to introduce this new API/state.

@benlesh am I right in thinking that subscriber.signal.aborted === subscriber.closed?

[domfarolino]

I've cleaned up the code example in https://github.com/web-platform-tests/wpt/pull/42219#discussion_r1341838611 that is trying to demonstrate the need for Subscriber#closed:

let firstTime = true;
const source = new Observable(subscriber => {  
  if (firstTime) {
    firstTime = false;
    source.subscribe({
      complete: () => {
        if (!subscriber.signal.aborted) {
          subscriber.complete();
        }
      },
      signal: subscriber.signal
    })
  } else {
    subscriber.complete();
  }
})

const ac = new AbortController();
source.subscribe({
  complete: () => console.log('complete!'),
  signal: ac.signal,
});

In Chromium with the Observables flag enabled, it seems to run fine / without issue. So I'm having a hard time figuring out why we need .closed. Sorry if I'm just being dense!

[domfarolino]

I think the point that the example is trying to make is something like: inside a subscriber's complete() handler, it is observable that the Subscriber's signal is not yet aborted, even though subsequent calls to that same Subscriber's complete() handler should not be possible to make. So basically:

let innerSubscriber = null;
const observable = new Observable(subscriber => {
  innerSubscriber = subscriber;
  subscriber.complete();
});

const ac = new AbortController();
const signal = ac.signal;

observable.subscribe({
  complete: () => {
    console.log(signal.aborted);
    // Because `signal.aborted` is observed as false, it might mislead
    // people to thinking they can somehow call `subscriber.complete()` again!

    // This would theoretically call this currently-running `complete()`
    // handler recursively forever.
    innerSubscriber.complete();
  },
  signal,
});

Is this accurate?

[domfarolino]

OK the example above does indeed recursively run infinitely until the maximum stack size is reached. However, fixing this does not require an actual API change, so I'm still curious where the .closed attribute comes into play here.

Conceptually "closing down" the subscription before actually invoking the complete() or error() handlers (so that they cannot accidentally call themselves recursively forever) is an internal change that doesn't require a new Subscriber attribute, no?

[domfarolino]

Update: Ben and I are interested in introducing this as Subscriber#isActive, in part because it's a farm more explicit name, and because subscriber.closed will likely be used as a negative check most times if (!subscriber.closed), so framing the API in positive terms seems more ergonomic.

[bakkot]

(If it's a getter or data property then it should be just Subscriber#active, per the design principles.)