`inspect()` operator's `abort()` handler

The WPT PR https://github.com/web-platform-tests/wpt/pull/44480 proposed some original tests for the do() operator — now inspect(). It proposed an abort() callback (in the dictionary for inspect()) that would be called only on explicit unsubscription, i.e., when the AbortController associated with a subscription is aborted. It is not called when error() or complete() is called: https://github.com/web-platform-tests/wpt/pull/44480/files#diff-f21c17324eba5c76ded6707d6a82fa569293f1fc099af6b2a25efadd34049643R390.

Apparently this matches RxJS's tap() operator behavior, but I don't think it makes as much sense with a token-based cancellation mechanism. Right now it isn't possible for a Subscriber to distinguish between (a) unsubscription via producer-initiated complete()/error(), and (b) unsubscription via upstream abortion of a passed-in AbortSignal. Subscriber#signal gets aborted in both cases, and all teardowns run in both cases[^1].

So it seems weird to give inspect() a new ability that other Observables generally don't have — a hook that runs only on so-called consumer-initiated unsubscription, and not during the general teardown phase. Not only would this introduce an inconsistency, but it's also weird layering-wise. When you subscribe to an Observable we immediately create a Subscriber object whose signal member is a composite signal based on two input AbortSignals:

The signal passed in as the signal member of SubscribeOptions (i.e., what the "user" uses to "unsubscribe")
The signal associated with any internal AbortControllers that might need to cancel the source subscription at any time. A simple example of this is the AbortController owned by Subscriber, which gets aborted when complete() is called.

AbortSignals are great because they compose really easily like this, allowing us to encapsulate all of this behavior inside of a single composite AbortSignal (Subscriber#signal) that gets exposed to the source Observable. We don't have to keep references to any of the far-away input signals, or have specific logic tied to any one of them.

But the way inspect() is proposed (via tests, matching how it works in RxJS) seems to require that we break this encapsulation. We'd have to tie the logic that calls the abort() callback, specifically to the (1) signal above so we catch consumer-initiated unsubscriptions, and not any aborts caused by (2) above etc. For example:

  source
    .take(2)
    .subscribe();

Once source emits two values, take(2) calls complete() on its Subscriber. This aborts the take() Subscriber's internal signal, which was one of the input signals to source's subscription. Now imagine there was an inspect() before take(2); should its abort() handler get triggered in this case?

  source
    inspect({
      abort: () => {…},
    })
    .take(2)
    .subscribe({}, {signal});

Right now, because of how AbortSignals compose, the internal-take()-initiated unsubscription from source is indistinguishable from true upstream unsubscription via signal. I personally don't think we should change this.

Imagine a more complicated example, where inspect() is further away from the subscription signal:

  source
    .inspect({
      abort: () => {…},
    })
    .map(…)
    .map(…)
    .map(…)
    .subscribe({}, {signal});

This triggers a number of internal subscriptions, and by the time we subscribe to the inspect() Observable, the Subscriber#signal down there is far-removed (but composed from) the ancestor one passed into subscribe(). If we wanted inspect()'s abort() callback to be based on that far ancestor signal (and not Subscriber#signal), then inspect() would need to internally have access to that higher-level signal. In that case it feels like we're fighting against how AbortSignal is designed, since it would require keeping an explicit reference to that input signal around for a bit, and tying downstream behavior to it.

Proposal

My proposal is to rename abort() to teardown(), and have it get fired whenever the Subscriber#signal for that Observable gets aborted, just like everything else. It would essentially be a teardown for inspect() which I think makes a lot of sense. It would run whenever complete() and error() run, but that's how other Observables work in general, so I don't think that's an issue or otherwise inconsistent. In fact I think it makes things more consistent.

[^1]: To distinguish between these, you would have to just keep track of whether you explicitly called complete() or error() yourself or not, from within your teardown callbacks.

WICG / observable

`inspect()` operator's `abort()` handler #138

Proposal