Report synchronous errors thrown after subscriber completion

[cartant]

Feature Request

Is your feature request related to a problem? Please describe.

The issue is a re-write (as discussed in the last core team meeting) of issue #3461 - which is not as clearly described as it could have been.

There have been two recent bugs in which errors thrown synchronously - after subscriber completion - were not reported:

• in repeatWhen operator broken since 5.5.6 #3444, the repeatWhen swallowed an error that was thrown due to a re-enterant to complete (described here).
• in bindCallback swallows error if it was thrown after the callback was executed, and the entire function is sycnhronous #3487, a call to bindCallback swallowed an error thrown synchronously after the callback was invoked.

The errors were swallowed because the implementation of subscribe attempts to report them in _trySubscribe by calling the subscriber's error method:

_trySubscribe(sink: Subscriber<T>): TeardownLogic {
  try {
    return this._subscribe(sink);
  } catch (err) {
    if (config.useDeprecatedSynchronousErrorHandling) {
      sink.syncErrorThrown = true;
      sink.syncErrorValue = err;
    }
    sink.error(err);
  }
}

However, in both the repeatWhen and bindCallback issues, the subscriber's complete method had already been called. That means the call to error was ineffectual and no error was reported because the SafeSubscriber was stopped:

error(err?: any): void {
  if (!this.isStopped) {
    // ...
  }
}

Describe the solution you'd like

The errors should not be swallowed; they need to be reported somehow.

Ideally, any errors that cannot be reported via the subscriber's error method should be reported via hostReportError.

However, doing so has the potential to break client code, as throwing errors that were previously swallowed would be a breaking change.

Using the mechanism described in #3469, it's possible to navigate the chain of subscribers to determine if any are in fact stopped. If none is stopped, the error can be reported by calling the subscriber's error function. If a stopped subscriber is encountered, the error cannot be reported to the subscriber and should instead be reported to the console.

Something like this implementation of reportError, which could be called in _trySubscribe:

export function reportError(err: any, observer: ErrorObserver<any>): void {
  if (canReportError(observer)) {
    observer.error(err);
  } else {
    consoleReportError(err);
  }
}
function canReportError(observer: ErrorObserver<any>): boolean {
  const { closed, destination, isStopped } = observer as any;
  if (closed || isStopped) {
    return false;
  } else if (destination) {
    return canReportError(destination);
  }
  return true;
}

Reporting these errors to the console might not be ideal, but it's far better than swallowing them.

Describe alternatives you've considered

See above, re: reporting via hostReportError.

Additional context

• The repeatWhen issue.
• The bindCallback issue.
• The swallowed error issue.
• The original PR.

[benlesh]

I'm generally in favor of the solution of sending them to hostReportErrors, but as stated, that could be a breaking change for some people.