2.x: Wrapping onError life cycle violations in a marker exception

[hvisser]

Related to my question in #5009, I've run into more situations where onError() might be called outside of the correct observable lifecycle (as a result of an async operation using a subject for example). David noted in #5009, "RxJava can't distinguish between wanted and unwanted errors that are raised beyond the lifecycle (or cancellation)."

However, because these life cycle errors are raised using the default uncaught error exception, they are very hard to distinguish from "normal" unhandled rx errors by the developer too; they look the same and only after verifying that all errors are handled correctly in the stream you'll know for sure that an error must be related to the life cycle. Because they look and act the same, it's not possible to handle those exception in a different way if required either.

Therefore I'd like to propose to wrap errors that are caused by life cycle violations to be wrapped in an exception like RxLifecycleException to signal that a particular error is caused by a programming mistake related to the life cycle, as opposed to an unhandled exception resulting from the stream.

Wrapping life cycle errors like that would also allow developers to choose how to handle these using the global error handler. For example: crash in debug builds, but warn only in production builds if appropriate.

This would keep the general design of not ignoring errors and would make it easier to identify life cycle related errors.

[akarnokd]

as opposed to an unhandled exception resulting from the stream

In 1.x, you got an OnErrorNotImplementedException fatal exception if you used the 0 - 1 argument subscribe(). In 2.x, unhandled errors by subscribe() go into the RxJavaPlugins.onError as is. I see the option to reintroduce this exception on the 0-1 subscribe() to distinguish those errors from lifecycle errors. I think @JakeWharton agrees that such errors should crash the app due to being a programmer error.

Beyond this, I still don't see any other distinguishing factor with errors beyond lifecycle. Unfortunate that is, programmers are usually not responsible and can't do much about it (beyond perhaps using delayError flags or overloads).

• merge() streams where multiple sources call onError
• map() crashes at the very last item and the upstream emits an onError as the next signal
• doAfterTerminate() crashes due to programming error but the consumer was already signalled
• multiple manual calls to Subject.onError

Beyond these, there could be cases when some operation throws an InterruptedException but isDisposed() reports false.

There are a bunch of scheduling operators that may call worker.dispose() before calling upstream.dispose(); I thought interrupting an ongoing work on these Workers was more important than calling an unknown chain (and thus cost) dispose() before the interrupt. Helping me hunt these down (with unit tests) would be great.

[hvisser]

Yes, wrapping "normal" errors in OnErrorNotImplementedException would also work I think.

I agree that crashing on programming errors is good. My main gripe right now is that both code paths result in the same stack trace so it's very hard figure out if I "forgot" onError handling by calling subscribe(o -> {}) or if some part of my code invoked onError on a PublishSubject when it wasn't supposed to due to some circumstance I didn't foresee.

I notice that it isn't just me struggling with this in our code base so I worry that as more people migrate to RxJava 2.x they will run into the same problems.

[akarnokd]

Yes, wrapping "normal" errors in OnErrorNotImplementedException would also work I think.

I'll prepare a PR for this then.

I notice that it isn't just me struggling with this in our code base so I worry that as more people migrate to RxJava 2.x they will run into the same problems.

These errors where there in 1.x as well but were practically ignored as the default handler was a no-op handler.

[hvisser]

These errors where there in 1.x as well but were practically ignored as the default handler was a no-op handler.

I meant struggling in the sense that it's hard to find the source of the error. Having OnErrorNotImplementedException for cases where we, well, did not implement onError vs all other uncaught errors coming from RxJava should help I think. And we can then decide how to deal with those errors in the error handler, worse case having a no-op like RxJava 1.x has, but letting onErrorNotImplemented fall through.

[akarnokd]

See #5036.

[akarnokd]

Closing via #5036