Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception bubbling in BlockingObservable #7267

Closed
blacelle opened this issue May 11, 2021 · 5 comments
Closed

Exception bubbling in BlockingObservable #7267

blacelle opened this issue May 11, 2021 · 5 comments

Comments

@blacelle
Copy link

blacelle commented May 11, 2021

I consider RxJava 1.3.8 and its use by Azure Management Library. I consider a case of exception bubbling through their use of BlockingObservable. However, i'm not fully comfortable with BlockingObservable own exception handling.
Azure/azure-libraries-for-java#1397

Azure Management Library enables most of this API through Async and Sync operations. I focus here on the Sync API, which relies essentially over .toBlocking().single().body(). In such a case, if an exception occured in the flow of events (sorry for my poor choice of words, I'm not comfortable with Reactive programmation), I end in my own code with an exception produced by a different thread (in my case, a okhttp3 thread) : which is bad as I would expect an exception with a stack build on current thread, wrapping the exception from okhttp3 thread.

Some people state it is better to have less wrappers around an exception. (e.g. https://blog.danlew.net/2015/12/08/error-handling-in-rxjava/: I find it's easiest to deal with exceptions when they have the fewest wrappers around them. Exceptions.propagate() helps; it only wraps the exception if it's checked.)

I disagree with this statement. Indeed, exception wrapping is especially useful when one receive an exception generated from a different thread: wrapping the exception enables building a proper chain of stacks (from the calling thread+method to the failing thread+method).

I feel BlockingObservable fails to this as it relies on Exceptions.propagate which does not wrap RuntimeException, even through they would come from a different thread: as the API does not catch such a RuntimeException (which is OK in my perspective), it implies my own code receive directly the AzureLibrary exception (i.e. from OkHttp3 thread). Hence, if I myself leave the exception bubbling up without wrapping it, my final Exception Processor (e.g. Spring MVC @ControllerAdvice, or a generic UncaughtExceptionHandler) will process an exception from the OkHttp thread while it would much helpful to receive this exception wrapped in an exception attached to current thread.

Should Rxjava wrap exception while it is bubbling at least on any change of threads?
Should BlockingObservable force wrap the exception (instead of Exceptions.propagate wrapping all but RuntimeExceptions)?
Should Libraries and users of BlockingObservable catch for wrapping systematically to ensure propagating chained-exceptions?

@akarnokd
Copy link
Member

I would expect an exception with a stack build on current thread

Why do you need this? Do you have multiple blocking calls one after the other you can't distinguish?

Blocking methods don't compose well so we generally suggest avoiding them as much as possible.

I consider RxJava 1.3.8 and its use by Azure Management Library.

RxJava 1.x and 2.x are end-of-life and thus are no longer supported.

@blacelle
Copy link
Author

blacelle commented May 11, 2021

Why do you need this? Do you have multiple blocking calls one after the other you can't distinguish?

Yes, I call given library multiple times (my app does not rely on Reactive for its own logic: hence I rely exclusively on the sync API of given library, which relies for each call on a BlockingObservable). Given multiple API calls, it is difficult to know which one failed.

Does it suggest it is a bad-practise to rely on RxJava to offer a sync API (while relying on Rx for internal processes)?

Blocking methods don't compose well so we generally suggest avoiding them as much as possible.

I hesitated open this ticket after opening a ticket for Azure Management Library team, but I preferred having a clear understanding of RxJava own opinion on this.

RxJava 1.x and 2.x are end-of-life and thus are no longer supported.

I tried having a look in RxJava3, but the "Go to file" feature is not available at https://github.com/ReactiveX/RxJava

@akarnokd
Copy link
Member

I could wrap each blocking call into its own try-catch and then wrap the exception as you see fit. Because those methods are blocking, you have the current thread and its stacktrace implicitly.

BlockingObservable has been removed in 2.x. You may use the blockingXXX methods directly on an Observable. Their behavior regarding RuntimeExceptions is still the same and won't be reconsidered.

@blacelle
Copy link
Author

I could wrap each blocking call into its own try-catch and then wrap the exception as you see fit. Because those methods are blocking, you have the current thread and its stacktrace implicitly.

Wrapping the exception would help. Definitely.

BlockingObservable has been removed in 2.x. You may use the blockingXXX methods directly on an Observable. Their behavior regarding RuntimeExceptions is still the same and won't be reconsidered.

Does it mean the lack of chained-exceptions (i.e. throwing the original Exception even if from a different thread) is a feature, not a bug, in 2.x ?

@akarnokd
Copy link
Member

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants