1.x: Add missing error handler call in Completable

[bryant1410]

No description provided.

[akarnokd]

Why do you think it's missing? In operators, only undeliverable exceptions are supposed to be sent to the global handler.

[bryant1410]

I'm not pretty sure if this call to the handler goes there. But it arises from using the Retrofit's RxJava adapter, and realizing that Completable's endpoints errors are not caught by an error handler, while Observable's and Single's are.

[akarnokd]

Interesting, could be that Completable doesn't use SafeSubscriber, which is responsible to deliver some errors thrown by onXXX methods. To be sure, could you add a unit test that demonstrates how Single gets your exception and how Completable isn't?

[bryant1410]

Okey

[bryant1410]

I think this comment is related: Completable.java#L1968. From what I see, Completable, in a different way to Observable and Single, does calls to the error handler plugin by itself, instead of delegating it to a SafeCompletableSubscriber (which btw doesn't exist). Why there is no safe subscriber for Completable and why is this done this way?

The call to the handler is missing from onError in subscribe(Action1<? super Throwable>, Action0), subscribe(CompletableSubscriber) and subscribe(Subscriber) methods, but subscribe() and subscribe(Action0) have it. This should be refactored to have just one place for calling the handler, as in Observable and Single.

[akarnokd]

Completable was designed with a modern mindset where end-CompletableSubscribers are not expected to throw from the onXXX methods, but apparently, there was no safeSubscribe() added to the API. The current 1.x convention is to have subscribe() do additional safeguards and have an unsafeSubscribe() as direct as possible.

[akarnokd]

See #3942 for other forms of handling errors.

[zsxwing]

Closing via #3942