-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Cleaner error propagation in hystrix javanica #241
Comments
Hi, good point, thanks. I made PL with fix for this issue. |
benjchristensen
added a commit
that referenced
this issue
Apr 15, 2014
Fix for issue #241 (leaner error propagation in hystrix javanica)
Merged. |
Could you guys update the javanica docs to reflect these changes (re-thrown exceptions are not actually wrapped in HystrixBadRequestException)? |
Yes the docs should be updated accordingly 'cuz I was almost fooled.. |
Is this PR clear? #1081 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
From:
https://github.com/Netflix/Hystrix/tree/master/hystrix-contrib/hystrix-javanica
"Based on this description, @HystrixCommand has an ability to specify exceptions types which should be ignored and wrapped to throw in HystrixBadRequestException.
If userResource.getUserById(id); throws an exception which type is BadRequestException then this exception will be wrapped in HystrixBadRequestException and will not affect metrics and will not trigger fallback logic."
Wouldn't it be cleaner to unwrap the original exception and rethrow it, like I do in my prototype:
https://github.com/davidkarlsen/hystrix-aop/blob/master/src/main/java/com/davidkarlsen/hystrix/aop/HystrixAspect.java
Because when adding hystrix as an AOP aspect I'd like not to alter the service signature/exceptions thrown. I like the idea that you can tell hystrix which exceptions to ignore as errors, but would like to get them thrown unwrapped, so hystrix is not change how exceptions should be catched by a client. Ideally hystrix should be invisible when throwing "business exceptions".
The text was updated successfully, but these errors were encountered: