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

DoOnEach: report both original exception and callback exception. #3408

Merged
merged 1 commit into from Oct 7, 2015

Conversation

akarnokd
Copy link
Member

@akarnokd akarnokd commented Oct 2, 2015

This came up in a Stackoverflow answer. If the doOnError's callback or the doOnEach's onError method throws, any non-fatal exception replaced the original error which got lost. This PR will wrap them both into a CompositeException.

2.x note: since Java 8 supports addSuppressed all callbacks in this situation either attach to the original exception or the original exception is attached to the callback's exception.

This came up in a
[Stackoverflow](http://stackoverflow.com/questions/32889008/do-operators-instead-of-a-whole-subscriber)
answer. If the doOnError's callback or the doOnEach's onError method
throws, any non-fatal exception replaced the original error which got
lost. This PR will wrap them both into a CompositeException.

2.x note: since Java 8 supports `addSuppressed` all callbacks in this
situation either attach to the original exception or the original
exception is attached to the callback's exception.
@akarnokd akarnokd added the Bug label Oct 2, 2015
@akarnokd akarnokd added this to the 1.0.x milestone Oct 2, 2015
@abersnaze
Copy link
Contributor

👍

abersnaze added a commit that referenced this pull request Oct 7, 2015
DoOnEach: report both original exception and callback exception.
@abersnaze abersnaze merged commit 36742c9 into ReactiveX:1.x Oct 7, 2015
@akarnokd akarnokd deleted the DoOnErrorReportBoth branch October 8, 2015 09:15
@frvi
Copy link

frvi commented Mar 14, 2016

Is there a way to avoid this wrapping?
I want to throw a specific exception in my doOnError, which then gets handled in a certain way.

@akarnokd
Copy link
Member Author

No, but you can unwrap via onErrorResumeNext:

source.doOnError(e -> { throw new RuntimeException(); })
.onErrorResumeNext(e -> {
    if (e instanceof CompositeException) {
       return Observable.error(((CompositeException)e).getExceptions().get(1));
    }
    return Observable.error(e);
});

@frvi
Copy link

frvi commented Mar 15, 2016

@akarnokd -- ah, thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants