Errors sometimes not rethrown

[matthewwithanm]

Been tracking down this tricky bug that was causing some real-world pain 😢

RxJS version: 5.3.1 & 5.4.3

Code to reproduce:

Runnable version here.

Observable.of(0, Scheduler.async)
  .mergeMap(() =>
    Observable.combineLatest(Observable.of(0))
  )
  .subscribe(() => { throw new Error('uh oh'); });

Expected behavior:

Error is re-thrown asynchronously and can be handled by normal uncaught exception means. (window.onerror, uncaughtException).

Actual behavior:

Error gets swallowed.

Additional information:

combineLatest seems to be involved here—if you get rid of it, the example behaves as expected.

[kwonoj]

Have to verify, but my current guess is this is related with #2313 - I've added few fixes around via #2626, #2796 but it seems not suffecient to cover all cases.

[wachri]

I run into the same problem. The interesting thing is that when I change it to:

Observable.of(0, Scheduler.async)
    .mergeMap(() => Observable.of(0))
    .combineLatest(Observable.of(0), Observable.of(0))
    .subscribe(() => { throw new Error('uh oh'); });

it throws the error.

[thetechnick]

We run into this issue in our production application, rxjs is swallowing errors in some cases which is very hard to debug and leads to strange behavior in our application.

The only way to work around this issue in our case is to async rethrow (this sounds as evil as it is) the error in the subscribe error handler.
This ensures that the global angular error handler receives the error and it does not disappear into the deeps of rxjs.

Observable.of(0)
    .subscribe(() => {}, (error) => { setTimeout(() => { throw error; }) });

[shakyShane]

Would anyone happen to know how this is not considered a show-stopper given it's so easy to reproduce?

https://jsbin.com/ribeyeyolo/edit?html,js,console

[benlesh]

FYI: in order to be compliant with the tc39 proposal, error rethrowing is going to have to go away. This is for a variety of reasons. Probably the biggest reason is that if an error is synchronous the throne after a multicast, it causes producer interference. The error unwinds to callstack back to the loop that's notifying everyone in the multicast, and breaks it.

The plan is to remove the rethrowing behavior in 6.0.

[benlesh]

FWIW I think we can provide some Rx specific mechanism of notification for unhandled errors.

[shakyShane]

@benlesh thanks!

Whilst I understand the need to remove it, it seems such a step backwards (from a safety POV) - the idea of not 'swallowing errors' was a major selling point and determining factor (especially when switching from promise-based flows).

Are you suggesting in the near future errors thrown in this way (from my link above) will just be swallowed regardless then?

I'm only curious on the path going forward, not trying to be awkward :)

[jayphelps]

@shakyShane it's a very complex topic when dealing with multicast subscriber interference and actually the same problem Promises have which solved it the same way; by not rethrowing aka swallowing. (I believe you were also acknowledging this but just to clarify for everyone)

If it means anything, I used to be stringently opposed until someone sat down and walked me through the problems of the current approach and how not rethrowing is the better tradeoff.

Although the backstory is many years old (with Promises), some of the more recent discussion: #2145, tc39/proposal-observable#119

Since Promises do this too they later made a global unhandledrejection event you could listen for which will be called with all promise rejections that weren't handled, so they were "swallowed" by JS itself. In addition, modern browsers debug consoles automatically listen to this and log the errors in the console so that you still know they happened. It's highly likely Observables will get a very similar (or even the same) solutions.

You can reproduce this in the context of Promises with this:

https://jsbin.com/hifavaq/edit?js,output

console.log('first');

try {
  const promise = new Promise(() => {
    console.log('second');
    throw new Error('Silent, but deadly');
  });
} catch (e) {
  // this never runs, no error is caught
  // it is "swallowed" per spec
  console.log('NEVER RUNS AS IT DOESNT RETHROW');
}

console.log('third');

[matthewwithanm]

The TC39 observable discussions about errors are super interesting and important but can we keep this particular issue about RxJS 5? The current behavior is inconsistent, surprising, and a real source of pain. 😞

[jayphelps]

Fixed in 5.5.6 https://github.com/ReactiveX/rxjs/blob/stable/CHANGELOG.md#556-2017-12-21

Fingers crossed that it covered all situations this time; previous fixes only fixed certain cases but not others.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.