You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 extend FailsafeException, like BulkheadFullException, can be caught directly. Application specific exceptions like IOException are wrapped as the cause of a vanilla FailsafeException. With asynchronous getAsync, both kinds of exceptions are wrapped as the cause of an ExecutionException.
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:
The text was updated successfully, but these errors were encountered:
Tembrel
changed the title
Document delicate semantics of FailsafeException
Add FailsafeExecutionException extending FailsafeException for wrapping Throwables in sync get
Jan 23, 2023
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 FailsafeExecutionException as a subclass of FailsafeException and use it to wrap exceptions thrown during synchronous get, instead of FailsafeException. This wouldn't break any catch (FailsafeException ...) {...} blocks, but it would allow a runtime distinction between FailsafeException as wrapper vs. as superclass.
Only two FailsafeException to FailsafeExecutionException changes would be required, that I could find, here:
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: