-
Notifications
You must be signed in to change notification settings - Fork 297
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
Add FailsafeExecutionException extending FailsafeException for wrapping Throwables in sync get #356
Comments
No response to this, which is fair, since it doesn't really ask any questions or propose any action, other than better documentation. But there's a way to make things a little nicer without any compatibility issues (I think): Add Only two failsafe/core/src/main/java/dev/failsafe/SyncExecutionImpl.java Lines 192 to 196 in 0f3f66d
and here: failsafe/core/src/main/java/dev/failsafe/Functions.java Lines 272 to 276 in 0f3f66d
(Incidentally, couldn't these be consolidated?) So I changed the issue title to reflect this proposal. |
There's a subtle semantic difference between how
FailsafeException
is used synchronously vs. asynchronously.With synchronous
get
, exceptions thrown by Failsafe policies, i.e., those that extendFailsafeException
, likeBulkheadFullException
, can be caught directly. Application specific exceptions likeIOException
are wrapped as the cause of a vanillaFailsafeException
. With asynchronousgetAsync
, both kinds of exceptions are wrapped as the cause of anExecutionException
.There's nothing wrong with that, but it feels as though
FailsafeException
is filling two very different roles: On the one hand, it's a superclass for a several policy-related exceptions. On the other hand, it's a way to wrap application-specific exceptions thrown by tasks executing synchronously.I have a dim memory of some long ago issue comments where this was explicitly acknowledged and accepted. So I don't think this warrants an API change, but it would be nice if this information were presented clearly somewhere, perhaps in a table like this:
I wrote a gist to make sure that my understanding of the semantics was correct.
The text was updated successfully, but these errors were encountered: