Add basic observable constructor tests

[benlesh]

Attn @domfarolino ... here's a basic battery of tests covering new Observable, observation semantics, handled and unhandled errors in various scenarios, addTeardown, subscribe and forEach.

[wpt-pr-bot]

There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you!

[domfarolino]

A few comments, nothing too serious. The biggest concern I have is .closed which I don't think was discussed in the explainer repo at all. I'd prefer to leave that out here and address it there first to collect more feedback and thoughts if you're OK with that. In part because its utility is not 100% clear to me, so having that discussion outside of this test PR seems good.

[domfarolino]

Hmm, we haven't really discussed the .closed attribute on the Subscriber interface yet, so maybe we should hold off on this assertion. Can we assert other side-effects, (i.e., would subscriber.signal.aborted be true? Maybe not since the outer AbortController didn't actually abort this).

Maybe it would just be good to assert the teardown is called, and talk about this attribute later.

[benlesh]

(i.e., would subscriber.signal.aborted be true? Maybe not since the outer AbortController didn't actually abort this).

There's some "dead space" where there's a difference between "closed" and "signal.aborted"

Basically, the signal is aborted after your complete handler is fired. But at the moment subscriber.complete is called, the subscriber has to be shut down (closed).

Consider the following: (this may seem outlandish, but it's just a small bit of code simulating some of the things that can happen with home brew observable operators, in particular, reentrancy, where someone composes an observable that emits into itself in a roundabout way)

let level = 0;
const source = new Observable(subscriber => {
  subscriber.addTeardown(() => {
    console.log('torn down!');
  });
  
  if (level++ === 0) {
    // Rentrant code
    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 the above case, the provided subscriber.signal, which is following whatever signal was passed into the subscribe call, would not be "aborted" yet, because it should semantically abort AFTER the complete handler fires. So we'll get stuck in a loop.

There's already a pieces of state we have to track internally in the Subscriber so that next, error, and complete can't be called if error or complete have already been called. This is an important part of the guarantee.

So the lifecycle inside the subscriber (if complete or error is called):

1. "close" the subscriber to new calls/actions (disable next, report errors passed to error, disable complete, and put addTeardown in a state of "execute immediately". This is a safety mechanism and a valuable guarantee.
2. emit complete (or error)
3. Tear down (by aborting the signal attached to this, which also controls everything registed with addTeardown.

The closed property of Subscriber exposes this "close" state from 1 above.

[domfarolino]

I think that makes sense, I just think we should put this in the explainer with a PR first, with the justification above etc. Also I think it's worth having some discussion about:

In the above case, the provided subscriber.signal, which is following whatever signal was passed into the subscribe call, would not be "aborted" yet, because it should semantically abort AFTER the complete handler fires.

It sounds like you're saying that even with .closed existing, you think the AbortSignal should be aborted after complete() or error() is fired, which seems weird to me. I think only AbortController should be able to abort an AbortSignal. But maybe you're only saying this in the theoretical world where .closed didn't exist (like the example you have above)... either way, all of this discussion should probably happen on https://github.com/WICG/observable instead of here, if that makes sense.

[domfarolino]

So we'll get stuck in a loop.

This part I'm not sure I see. It seems like the re-entrant (i.e., the most nested) call to complete() would just trigger the outermost complete() handler, right? I don't think I see an actual infinite loop here.

[benlesh]

you think the AbortSignal should be aborted after complete() or error() is fired, which seems weird to me. I think only AbortController should be able to abort an AbortSignal

There's the AbortSignal that was passed into the subscription, and then there's an AbortSignal related to the status of the subscription. If the subscription is "complete" or "errors" we need to teardown resources and cancel any upcoming work. Therefor, if you complete() or error() the signal that is on the Subscriber should be aborted. This signal would not be the same signal that was passed into the subscription.

new Observable(subscriber => {
  console.log(subscriber.signal.aborted); // false
  subscriber.complete();
  console.log(subscriber.signal.aborted); // true.
})

[domfarolino]

Similar to the above comment about .closed. It isn't totally clear to me what this buys us, so maybe we could discuss it on the explainer before writing test expectations around it?

[benlesh]

closed is necessary for "synchronous firehose" scenarios. If you want take to work in this scenario:

const result = new Observable(subscriber => {
  let n = 0;
  while (!subscriber.closed) {
    subscriber.next(n++);
  }
})

result.take(3).subscribe({ 
  next: console.log
})

If there's no way to examine if the subscription has been ended by the take(3), then there's no way to break that while loop.

[domfarolino]

ditto

[domfarolino]

Should the error reporting handler be called twice? And if so, should we assert that one way or another?