MergeMap throws uncatchable errors

[kondi]

Bug Report

Current Behavior
In the sample code below, the TypeError is not caught in the catchError callback, but thrown globally. We can not detect the error in the observable stream and in NodeJS environment this error can cause the whole application to shut down.

(If the provided mergeMap callback throws an Error instead of returning null, the error is handled correctly.)

Reproduction

• REPL or Repo link: https://stackblitz.com/edit/rxjs-uncaught?file=index.ts

of(1).pipe(
  // Without the next line, the error is caught correctly
  mergeMap(async () => 2),
  // This usage should produce a runtime TypeError: You provided 'null' where a stream was expected.
  mergeMap(() => null),
  catchError(error => {
    console.error('Error in the stream caught:', error.message);
    return [];
  })
).subscribe();

Expected behavior
The TypeError should not leave the observable stream, should be catchable by catchError the same way like when the mergeMap(async () => 2) line does not present.

Environment

• Runtime: Node v12.10.0, Chrome v80.0.3987.122
• RxJS version: 6.5.4

Possible Solution
Not a solution, just a very hacky workaround for somebody who is affected by this bug: https://stackblitz.com/edit/rxjs-uncaught?file=patch.ts

[Andrei0872]

I think I found where the problem comes from:

subscribeToPromise:

export const subscribeToPromise = <T>(promise: PromiseLike<T>) => (subscriber: Subscriber<T>) => {
  promise.then(
    (value) => {
      if (!subscriber.closed) {
        subscriber.next(value); // Here's the problem! 
        subscriber.complete();
      }
    },
    (err: any) => subscriber.error(err)
  )
  .then(null, hostReportError);
  return subscriber;
};

The problem is subscriber.next(value);, where subscriber is an InnerSubscriber, which will notify the outer subscriber(mergeMap in this case), which will in turn push the value further, meaning it will reach the second mergeMap.

Because subscriber.next(value) will cause an error, it will be intercepted by hostReportError, from the second then, which will throw the error like this: setTimeout(() => { throw err; }, 0);.

Here's a simplified example:

Promise.resolve(2)
    .then(
        v => {
            produceErr();
        },
        err => console.log('ERR') // Never reached
    )
    .then(null, hostReportError);

function produceErr () {
    throw new Error('err!');
}

function hostReportError () { console.log('caught!') };

I think one solution would be to surround that logic with a try/catch

try {
 subscriber.next(value);
  subscriber.complete();
} catch (err) {
  subscriber.error(err);
}

[tonivj5]

Is there a workaround? I'm suffering from this and I'm losing any stacktrace from the original error

[tonivj5]

I thought my problem was other, related, but different of this issue...

Here is the reproduction of the problem I had: https://stackblitz.com/edit/rxjs-uncaught-jirbiw?file=index.ts

I forget to return an observable on the else branch into catchError operator and Typescript didn't warn me, and runtime error didn't help too much 😕 I thought error comes from mergeMap (because I'm using it with promises), but it was an unfortunate coincidence 😆

Should I open other issue?

[jakovljevic-mladen]

It seems that this problem is not reproducible as of v7 (7.0.0-beta.13). No matter if mergeMap(async () => 2) call is included or not, a callback passed to the catchError is always called (if mergeMap(() => null) is included). Also, mergeMap(() => null) won't work (it produces a TS error), so an explicit cast has to be performed: mergeMap(() => <any>null)

@tonivj5 In your case from stackblitz, not having the return statement produces runtime error in v7.

@cartant could you verify and possibly close this issue?

[cartant]

@cartant could you ... possibly close this issue?

Not really. The problem is reproducible in 6.6.6, so it exists in the current non-beta release and is a bug. It should be fixed, but it's not at the top of my prioritised list of things that need to be done. However, I suspect that my curiosity will get the better of me and that I'll end up fixing it soonish.

[jakovljevic-mladen]

Thanks @cartant for the confirmation and for the fix.

[cartant]

Closed by #6186