WICG / observable

Observable API proposal
https://wicg.github.io/observable/
Other
542 stars 12 forks source link

`finally()` operator timing semantics #151

Open domfarolino opened 1 week ago

domfarolino commented 1 week ago

TL;DR: On unsubscription (i.e., abort()), what should the timing of the finally() operator's callback be with respect to the source Observable's teardowns?

When writing the spec for, and implementing, the currently-unspecified finally() operator, I was reviewing the proposed web platform tests and had a comment about this bit: https://github.com/web-platform-tests/wpt/pull/44482/files#diff-b1a6cf94fa2c551227215a7556fbc3114006fca82549ba48a27f3dafe2564f01R186. This made me think more about the timing of the finally() operator's callback with respect to the source Observable's teardowns.

Background: unsubscription teardown semantics

When you subscribe (with an AbortSignal) to an Observable returned by an operator, two things happen:

  1. A dependent signal is created from the signal that was passed into subscribe().
  2. The intermediate Observable is subscribed to with that dependent signal (example), and its native subscribe callback runs.
  3. The operator subscribes to its upstream Observable with the dependent signal (example). This dependent signal goes through (1) above and gets turned into a new dependent signal before subscription, of course.

So in the following code:

const source = new Observable(subscriber => ...);

const ac = new AbortController();
source.take(1).subscribe({}, {signal: ac.signal});

There are three AbortSignals involved:

  1. The parent one: ac.signal.
  2. A dependent of (1), for take()s Subscriber's Subscriber#signal. (It is purely internal and not web accessible).
  3. A dependent of (2), exposed in subscriber above.

If you call ac.abort(), then the abort signals are aborted in the order they're listed above. That's because DOM defines that a parent signal gets fully aborted before any of its dependents: https://dom.spec.whatwg.org/#abortsignal-signal-abort. Practically speaking, this means if you added an abort event listener to ac.signal and subscriber.signal, the ac.signal one would fire first, and the subscriber.signal one would fire last.

Integrating finally()

Now consider the requirement that the finally() callback needs to be called when a consumer unsubscribes from an Observable:

const ac = new AbortController();

const source = new Observable(() => {});
source
  .finally(() => console.log('finally'))
  .subscribe({}, {ac.signal});

ac.abort(); // logs `finally` to the console

The simplest way to make this happen is just to add finally()'s callback to its Subscriber's teardown list. This essentially means finally() is syntactic sugar around that operator's Subscriber's addTeardown() method. But this would break the ordering suggested in the test: https://github.com/web-platform-tests/wpt/pull/44482/files#diff-b1a6cf94fa2c551227215a7556fbc3114006fca82549ba48a27f3dafe2564f01R186. Specifically, in the following example:

const ac = new AbortController();
const source = new Observable(subscriber => {
  subscriber.addTeardown(() => console.log('teardown'));
});

source
  .finally(() => console.log('finally()'))
  .subscribe({}, {signal: ac.signal});

// Logs `finally()`, and then `teardown`. But the test expects
// `teardown` and then `finally()`.
ac.abort();

If the finally callback were part of its intermediate Subscriber's teardown list, then it would necessarily run before any of the source Observable's teardowns, since the intermediate signal (associated with finally()) aborts before its dependent signal (the one associated with the source Observable's Subscriber). Is this OK? It doesn't match RxJS's behavior, where finalize() callbacks execute after the teardown callback runs.

If we reallllly wanted to, we could probably match RxJS's behavior by building some bespoke infrastructure on Subscriber, or rejiggering around the dependencies of AbortSignals. But I'm really not sure what that would look like, or if it is even desirable at all. It would break the general precedent with AbortSignal.

I lean towards the code example / output immediately above, but I'd like to get opinions from others, to know if this is a real problem.

benlesh commented 3 days ago

Teardowns should be executed during finalization of observation. They are roughly what you'd put in a "finally" block if we had syntax for observable in JavaScript. Since the result of the finally() method is a new observable that is "downstream" from the source, I would expect the source's upstream finalization to be complete before the downstream could trigger it's finally.

In fact, once an upstream source is complete or errored, or when it's notified of abort, the downstream consumer should not be able to act until finalization is complete.

Abort notifications should be sent upstream immediately.

During abort, I'd expect the following order of operations within a given subscription:

  1. Abort any upstream subscriptions
  2. Execute teardowns in reverse order

Similarly in a situation where the subscription was errored or completed by the producer, I would expect:

  1. Abort any upstream subscriptions
  2. Execute teardowns in reverse order
  3. Notify any downstream consumers of completion (or error)

So as prior art here, we can look at generators or async generators:

With generators set up as so:

function* a() {
  try { yield 'a'; } finally { console.log('finally a'); }
}

function* b() {
  try { yield* a(); } finally { console.log('finally b'); }
}

function* c() {
  try { yield* b(); } finally { console.log('finally c'); }
}

If you create an instance of the generator and prime it:

const g = c();
g.next(); // primed { value: 'c', done: false }

And then you either next (resulting in a completion notification of { done: true }), or you return() which is signaling that you're done with the generator (similar to an abort in observable), you'll see the order in which the finally blocks are logged.

c.next(); // { done: true }

// logs
// "finally a"
// "finally b"
// "finally c"

or

c.return();

// logs
// "finally a"
// "finally b"
// "finally c"
benlesh commented 3 days ago

Other prior art: Promise.prototype.finally

Promise.resolve(1)
  .finally(() => console.log('one'))
  .finally(() => console.log('two'))

// Logs
// "one"
// "two"

Yeah... the observable implementation needs to abort the upstream subscription before it fires the finalizer.

Similarly, it should abort upstream subscriptions as soon as it knows it can in the case of "complete" or "error" being called in the subscriber.