Regression from 6 to 7 with subscribing to a subject from within a next call

[jecraig]

Bug Report

Current Behavior
With an AsyncSubject, previously if you subscribed to the same subject from within a next call, your inner subscription would also receive the next call. As of RxJs 7, that no longer happens. The inner subscription does not receive the next. I am aware this is not the recommended behavior but it did cause a regression in our system.

Expected behavior
When subscribing to an AsyncSubject, I would expect to receive the next value immediately regardless of the context from which that subscription was wired up.

Reproduction

Version 6: https://jsfiddle.net/4e3tdabg/6/
Version 7: https://jsfiddle.net/6px4nb3a/

const sub = new rxjs.AsyncSubject();

sub.subscribe({
   next: x => {
      sub.subscribe({
         next: xx => console.log('inner next', xx + xx),
         complete: () => console.log('inner complete'),
      });

      console.log('outer next', x);
   },
   complete: () => console.log('outer complete')
});

sub.next(3);
sub.complete();

Environment

• Runtime: Latest Chrome, Latest Firefox
• RxJS version: 7

Additional context/Screenshots
Expected

Actual

[kwonoj]

Unless I misread shared snippet, this looks like typical reentrant case (only except reentrate subscribes, instead of trying to emit).

[jecraig]

@kwonoj Not sure on the lingo you used. This is a subscription being wired from within a subscription. Now it's not a great pattern to use, but our use case was not this overt. We had a subscription call that ended up resubscribing several functions deep because the AsyncSubject is used as part of a state machine.

One way to "fix it" is to add a delay, but that requires knowing you're going to loop back to the same initial subject ahead of time. If it's not a supported use case, then a console error would be expected as it can be difficult to know ahead of time in a large application.

[kwonoj]

This is a subscription being wired from within a subscription

Yes, that's what I meant for the reentrant:

const a = SomeSubject();

a.subscribe(() => {
   a.next() // reentrant to a
  //or a.subscribe..
});

If it's not a supported use case, then a console error would be expected as it can be difficult to know ahead of time in a large application.

From rxjs@5, rxjs doesn't gaurantee behavior around reentrant cause due to nature of source reentrant behavior can appear differently. In most cases synchronous reenttrant is nearly gauranteed to fail, while async reentrant may work depends on source / usecases - as you mentioned like add a delay could possible fix on your case.

If you choose to use reentrant, from there you're on your own and have to fix edge cases. We can't throw / error ahead, since we cannot detect all of the reentrant cases correctly before trigger: for example, most obivous reenttrant failure is synchronous infinite loop but we can't raise error either before (cause we can't detect) / or after enter loop (since it'll create sync inifite loop we can't escape). Same goes for async for various other reasons.

Personally, while other core team member might represent differently, I wouldn't call this as breaking changes or regression. This is undefined semantics of rx we intentionally designed and as said consumer have to take care if internal behavior changes.

[kwonoj]

to add a delay

Instead of delay I presume scheduling via async schedule would be possible workaround meanwhile.

[jecraig]

This is a very major change to behavior from v6 and one that is impossible to find without either extensive investigation or waiting for random things to break. It's also very hard to know that this is the cause since it's from a missed Next call which just looks like a stalled process.

It's not the fix that bothers me, it's the silent and unforeseeable result. Something could work just fine due to a natural delay from a webcall and then completely stop if, for instance, someone adds caching.

[benlesh]

This is a bug. I'll have a fix shortly. Thank you for pointing it out @jecraig