Proposal: Move the checks for missing onError() higher on the chain

[pavlospt]

Hello to everyone.

I would like to propose a change on the RxJava API, in order to avoid crashes of application's that have not properly implemented onError in the defined Subscribers.

Actually a real case scenario, happened the other day at Droidcon Berlin's 2016 App. The onError method was not implemented and on the case that anyone opened the application while being offline, the application itself would crash just because it produced an error that was not able to be propagated anywhere because there was not any onError method available.

This granted and knowing that this is a mistake that most if not all engineers would stumble upon at least once when they start with RxJava, I think that there should be a pattern of fail-very-early for this case.

It is completely understandable, that this is a Developer's/Engineer's mistake and wrong implementation, but it does not clearly set a restriction or does not force at all, anyone who uses a .subscribe() on an Observable, to implement the .onError() method.

So if you do not happen to write tests, which is not the common case scenario, and you also do not happen to stumble upon an Error that throws an Exception, you might possibly if not 100% ship an application that would crash in the future just because your subscription is missing the .onError() method.

I will be very glad to discuss it and read the different opinions on this issue :)

[JakeWharton]

OnErrorNotImplementedException is not a check, it's a default implementation of onError when you use the subscribe overrides you describe.

The (poorly named) rxlint library adds a lint check for missing onError implementations which can help. Otherwise #3937 and #3965 have been kicked around for better reporting of where the problem actually lies.

So if you do not happen to write tests, which is not the common case scenario, and you also do not happen to stumble upon an Error that throws an Exception, you might possibly if not 100% ship an application that would crash in the future just because your subscription is missing the .onError() method.

These types of things are true of every platform and every library though. How many developers have empty catch blocks or log statements instead of proper error handling. RxJava is superior in this regard since it refuses to drop errors on the floor and allow your application to silently continue in a unknown state.

[pavlospt]

Hello @JakeWharton, i fixed the title.

My point is that there should be something that would throw an Exception at compile time. It is not a 100% possibility that you would catch an Exception on your Observable's flow, in order to know about the missing onError() method.

Why not fail-early, instead of having to add another plugin/linter/dependency on your dev stack, such as rxlint that you mentioned?

[JakeWharton]

Because it's perfectly reasonable to omit error handling in streams for which it is not required or in which things like the onError* operators are applied.

[pavlospt]

Agree on that. So what you are saying is that there is no reason to fail-early in such case and just leave it as is?

Couldn't this be balanced in some way? Provide at least some NonNull annotation or something so it could be at least reported to the developer and he/she will choose or not to Disable the Inspection?

[JakeWharton]

I'm not saying there's no reason to fail early. RxLint seems like a way to accomplish what you want, in that it will fail your build if you use one of the subscribe overloads which doesn't handle errors. If you knew this wasn't a problem the @SuppressLint annotation lets you explicitly opt-out.

[pavlospt]

Great! Thank you for your insight on this :) Have a nice day!