-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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: wrap undeliverable errors #5080
Conversation
Codecov Report@@ Coverage Diff @@
## 2.x #5080 +/- ##
============================================
- Coverage 95.58% 95.57% -0.02%
- Complexity 5537 5552 +15
============================================
Files 612 614 +2
Lines 39561 39580 +19
Branches 5553 5559 +6
============================================
+ Hits 37816 37827 +11
- Misses 760 772 +12
+ Partials 985 981 -4
Continue to review full report at Codecov.
|
@@ -0,0 +1,35 @@ | |||
/** | |||
* Copyright (c) 2016-present, RxJava Contributors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 2017
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All our headers are like this, no current year.
@@ -0,0 +1,35 @@ | |||
/** | |||
* Copyright (c) 2016-present, RxJava Contributors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: 2017
* @return true if the error should pass throug, false if | ||
* it may be wrapped into an UndeliverableException | ||
*/ | ||
static boolean isBug(Throwable error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't CompositeException
be in here as well? While technically they're always undeliverable, they're very crash-worthy exceptions since they indicate an error while processing an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. A few tests have been reverted from assertUndeliverable to assertError. I've also added an isBug unit test.
This PR adds the
UndeliverableException
andProtocolViolationException
. The former wraps exceptions that happen beyond the lifecycle of a flow and the latter is added to distinguish validation bugs.The
RxJavaPlugins.onError
wraps errors intoUndeliverableException
unlessRxJavaPlugins.isBug()
returns true when theThrowable
is relayed to the (default) handler.Having the
UndeliverableException
should help with understanding the source of a crash and by having its own stack trace, help locating the offending positions (for example, a missedisDisposed()
check in a 3rd party library).