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

2.x: reintroduce OnErrorNotImplementedException for 0-1 argument subscribe() #5036

Merged
merged 1 commit into from
Feb 1, 2017

Conversation

akarnokd
Copy link
Member

This PR reintroduces the OnErrorNotImplementedException wrapper from 1.x and applies it to the subscribe() methods that don't define an onError handler.

The errors are still routed to the RxJavaPlugins.onError handler but now wrapped with OnErrorNotImplementedException.

This should help with cases where the developer forgot to add the handler and distinguish such unhandled errors from other undeliverable errors due to lifecycle limitations.

@akarnokd
Copy link
Member Author

Coverage: 95.51%

https://codecov.io/gh/ReactiveX/RxJava/pull/5036

@akarnokd akarnokd closed this Jan 31, 2017
@akarnokd akarnokd reopened this Jan 31, 2017
@akarnokd
Copy link
Member Author

(Messing with codecov settings, had to reopen this to trigger a webhook of theirs.)

@akarnokd akarnokd closed this Feb 1, 2017
@akarnokd akarnokd reopened this Feb 1, 2017
@akarnokd akarnokd merged commit ceded86 into ReactiveX:2.x Feb 1, 2017
@akarnokd akarnokd deleted the OnErrorNotImplemented branch February 1, 2017 09:39
@akarnokd
Copy link
Member Author

akarnokd commented Feb 1, 2017

I couldn't get Travis re-execute the job, likely due to the problems they are facing right now (even though they claim its only MacOSX builds).

Copy link
Contributor

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@PaulWoitaschek
Copy link
Contributor

So for the application usage it makes sense to re-throw everything of instance of OnErrorNotImplementedException and to log everything else?

My app became quite unstable with RxJava 2 because I'm silently swallowing errors and can't distinguish between my errors and errors thrown after I unsubscribed.

Really looking forward for this change if it does what I think it does!

@akarnokd
Copy link
Member Author

akarnokd commented Feb 6, 2017

If you are using subscribe() or subscribe(Consumer) and the sequence runs into an error, the error handler will receive an OnErrorNotImplementedException(originalError).

@@ -1702,6 +1702,10 @@ public final Disposable subscribe(final Action onComplete, final Consumer<? supe
* Subscribes to this Completable and calls the given Action when this Completable
* completes normally.
* <p>
* If the Completable emits an error, it is wrapped into an
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's twice.

If the completable emits an error...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once with the note about OnErrorNotImplemented..., once as swallowed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great opportunity to post a PR!

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

Successfully merging this pull request may close these issues.

4 participants