From 1042f275fca07685d426f47ef56b20e94b40a583 Mon Sep 17 00:00:00 2001 From: dmgcodevil Date: Tue, 18 Oct 2016 22:48:16 -0400 Subject: [PATCH] iss1344: send fallback exception to client instead of primary command --- .../aop/aspectj/HystrixCommandAspect.java | 29 +++- .../javanica/command/BatchHystrixCommand.java | 3 +- .../javanica/command/GenericCommand.java | 3 +- .../javanica/exception/ExceptionUtils.java | 19 +++ .../BasicDefaultIgnoreExceptionsTest.java | 2 +- .../error/BasicErrorPropagationTest.java | 158 +++++++++++++++++- 6 files changed, 202 insertions(+), 12 deletions(-) diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java index 577c3ef54..448f2b236 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java @@ -26,6 +26,7 @@ import com.netflix.hystrix.contrib.javanica.command.HystrixCommandFactory; import com.netflix.hystrix.contrib.javanica.command.MetaHolder; import com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException; +import com.netflix.hystrix.contrib.javanica.exception.FallbackInvocationException; import com.netflix.hystrix.contrib.javanica.utils.AopUtils; import com.netflix.hystrix.contrib.javanica.utils.FallbackMethod; import com.netflix.hystrix.contrib.javanica.utils.MethodProvider; @@ -102,7 +103,7 @@ public Object methodsAnnotatedWithHystrixCommand(final ProceedingJoinPoint joinP } catch (HystrixBadRequestException e) { throw e.getCause(); } catch (HystrixRuntimeException e) { - throw getCauseOrDefault(e, e); + throw getCause(e); } return result; } @@ -116,20 +117,32 @@ public Observable call(Throwable throwable) { return Observable.error(throwable.getCause()); } else if (throwable instanceof HystrixRuntimeException) { HystrixRuntimeException hystrixRuntimeException = (HystrixRuntimeException) throwable; - return Observable.error(getCauseOrDefault(hystrixRuntimeException, hystrixRuntimeException)); + return Observable.error(getCause(hystrixRuntimeException)); } return Observable.error(throwable); } }); } - private Throwable getCauseOrDefault(RuntimeException e, RuntimeException defaultException) { - if (e.getCause() == null) return defaultException; - if (e.getCause() instanceof CommandActionExecutionException) { - CommandActionExecutionException commandActionExecutionException = (CommandActionExecutionException) e.getCause(); - return Optional.fromNullable(commandActionExecutionException.getCause()).or(defaultException); + private Throwable getCause(HystrixRuntimeException e) { + if (e.getFailureType() != HystrixRuntimeException.FailureType.COMMAND_EXCEPTION) { + return e; } - return e.getCause(); + + Throwable cause = e.getCause(); + + // latest exception in flow should be propagated to end user + if (e.getFallbackException() instanceof FallbackInvocationException) { + cause = e.getFallbackException().getCause(); + if (cause instanceof HystrixRuntimeException) { + cause = getCause((HystrixRuntimeException) cause); + } + } else if (cause instanceof CommandActionExecutionException) { // this situation is possible only if a callee throws an exception which type extends Throwable directly + CommandActionExecutionException commandActionExecutionException = (CommandActionExecutionException) cause; + cause = commandActionExecutionException.getCause(); + } + + return Optional.fromNullable(cause).or(e); } /** diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java index d552a4738..702b60798 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java @@ -26,6 +26,7 @@ import java.util.Collection; import java.util.List; +import static com.netflix.hystrix.contrib.javanica.exception.ExceptionUtils.unwrapCause; import static com.netflix.hystrix.contrib.javanica.utils.CommonUtils.createArgsForFallback; /** @@ -79,7 +80,7 @@ Object execute() { } catch (Throwable e) { LOGGER.error(FallbackErrorMessageBuilder.create() .append(commandAction, e).build()); - throw new FallbackInvocationException(e.getCause()); + throw new FallbackInvocationException(unwrapCause(e)); } } else { return super.getFallback(); diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/GenericCommand.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/GenericCommand.java index 4cdc0d66a..fbe8a250b 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/GenericCommand.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/GenericCommand.java @@ -21,6 +21,7 @@ import javax.annotation.concurrent.ThreadSafe; +import static com.netflix.hystrix.contrib.javanica.exception.ExceptionUtils.unwrapCause; import static com.netflix.hystrix.contrib.javanica.utils.CommonUtils.createArgsForFallback; /** @@ -78,7 +79,7 @@ Object execute() { } catch (Throwable e) { LOGGER.error(FallbackErrorMessageBuilder.create() .append(commandAction, e).build()); - throw new FallbackInvocationException(e.getCause()); + throw new FallbackInvocationException(unwrapCause(e)); } } else { return super.getFallback(); diff --git a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/exception/ExceptionUtils.java b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/exception/ExceptionUtils.java index 65bb9c764..0dab38dd0 100644 --- a/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/exception/ExceptionUtils.java +++ b/hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/exception/ExceptionUtils.java @@ -15,6 +15,8 @@ */ package com.netflix.hystrix.contrib.javanica.exception; +import com.netflix.hystrix.exception.HystrixBadRequestException; + /** * Util class to work with exceptions. */ @@ -37,4 +39,21 @@ public static void propagateCause(Throwable throwable) throws CommandActionExecu public static CommandActionExecutionException wrapCause(Throwable throwable) { return new CommandActionExecutionException(throwable.getCause()); } + + /** + * Gets actual exception if it's wrapped in {@link CommandActionExecutionException} or {@link HystrixBadRequestException}. + * + * @param e the exception + * @return unwrapped + */ + public static Throwable unwrapCause(Throwable e) { + if (e instanceof CommandActionExecutionException) { + return e.getCause(); + } + if (e instanceof HystrixBadRequestException) { + return e.getCause(); + } + return e; + } + } diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicDefaultIgnoreExceptionsTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicDefaultIgnoreExceptionsTest.java index 3347c1b4d..7b1c0e1e6 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicDefaultIgnoreExceptionsTest.java +++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicDefaultIgnoreExceptionsTest.java @@ -51,7 +51,7 @@ public void testFallbackCommandOverridesDefaultIgnoreExceptions() { service.commandWithFallbackOverridesDefaultIgnoreExceptions(SpecificException.class); } - @Test(expected = SpecificException.class) + @Test(expected = BadRequestException.class) public void testFallbackCommandOverridesDefaultIgnoreExceptions_nonIgnoreExceptionShouldBePropagated() { service.commandWithFallbackOverridesDefaultIgnoreExceptions(BadRequestException.class); } diff --git a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicErrorPropagationTest.java b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicErrorPropagationTest.java index 8f5941268..c48b3eeaa 100644 --- a/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicErrorPropagationTest.java +++ b/hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicErrorPropagationTest.java @@ -18,6 +18,7 @@ import com.netflix.hystrix.HystrixEventType; import com.netflix.hystrix.HystrixRequestLog; import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand; +import com.netflix.hystrix.contrib.javanica.annotation.HystrixProperty; import com.netflix.hystrix.contrib.javanica.test.common.BasicHystrixTest; import com.netflix.hystrix.contrib.javanica.test.common.domain.User; import com.netflix.hystrix.exception.HystrixRuntimeException; @@ -27,6 +28,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.concurrent.TimeoutException; import static com.netflix.hystrix.contrib.javanica.test.common.CommonUtils.getHystrixCommandByKey; import static org.junit.Assert.assertEquals; @@ -110,7 +112,7 @@ public void testActivateUser() throws NotFoundException, ActivationException { } } - @Test(expected = OperationException.class) + @Test(expected = RuntimeOperationException.class) public void testBlockUser() throws NotFoundException, ActivationException, OperationException { try { userService.blockUser("1"); // this method always throws ActivationException @@ -128,6 +130,74 @@ public void testPropagateCauseException() throws NotFoundException { userService.deleteUser(""); } + @Test(expected = UserException.class) + public void testUserExceptionThrownFromCommand() { + userService.userFailureWithoutFallback(); + } + + @Test + public void testHystrixExceptionThrownFromCommand() { + try { + userService.timedOutWithoutFallback(); + } catch (HystrixRuntimeException e) { + assertEquals(TimeoutException.class, e.getCause().getClass()); + } catch (Throwable e) { + assertTrue("'HystrixRuntimeException' is expected exception.", false); + } + } + + @Test + public void testUserExceptionThrownFromFallback() { + try { + userService.userFailureWithFallback(); + } catch (UserException e) { + assertEquals(1, e.level); + } catch (Throwable e) { + assertTrue("'UserException' is expected exception.", false); + } + } + + @Test + public void testUserExceptionThrownFromFallbackCommand() { + try { + userService.userFailureWithFallbackCommand(); + } catch (UserException e) { + assertEquals(2, e.level); + } catch (Throwable e) { + assertTrue("'UserException' is expected exception.", false); + } + } + + @Test + public void testCommandAndFallbackErrorsComposition() { + try { + userService.commandAndFallbackErrorsComposition(); + } catch (HystrixFlowException e) { + assertEquals(UserException.class, e.commandException.getClass()); + assertEquals(UserException.class, e.fallbackException.getClass()); + + UserException commandException = (UserException) e.commandException; + UserException fallbackException = (UserException) e.fallbackException; + assertEquals(0, commandException.level); + assertEquals(1, fallbackException.level); + + + } catch (Throwable e) { + assertTrue("'HystrixFlowException' is expected exception.", false); + } + } + + @Test + public void testCommandWithFallbackThatFailsByTimeOut() { + try { + userService.commandWithFallbackThatFailsByTimeOut(); + } catch (HystrixRuntimeException e) { + assertEquals(TimeoutException.class, e.getCause().getClass()); + } catch (Throwable e) { + assertTrue("'HystrixRuntimeException' is expected exception.", false); + } + } + public static class UserService { private FailoverService failoverService; @@ -199,6 +269,70 @@ private void validate(String val) throws BadRequestException { throw new BadRequestException("parameter cannot be null ot empty"); } } + + /*********************************************************************************/ + + @HystrixCommand + String userFailureWithoutFallback() throws UserException { + throw new UserException(); + } + + @HystrixCommand(commandProperties = {@HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "1")}) + String timedOutWithoutFallback() { + return ""; + } + + /*********************************************************************************/ + + @HystrixCommand(fallbackMethod = "userFailureWithFallback_f_0") + String userFailureWithFallback() { + throw new UserException(); + } + + String userFailureWithFallback_f_0() { + throw new UserException(1); + } + + /*********************************************************************************/ + + @HystrixCommand(fallbackMethod = "userFailureWithFallbackCommand_f_0") + String userFailureWithFallbackCommand() { + throw new UserException(0); + } + + @HystrixCommand(fallbackMethod = "userFailureWithFallbackCommand_f_1") + String userFailureWithFallbackCommand_f_0() { + throw new UserException(1); + } + + @HystrixCommand + String userFailureWithFallbackCommand_f_1() { + throw new UserException(2); + } + + /*********************************************************************************/ + + @HystrixCommand(fallbackMethod = "commandAndFallbackErrorsComposition_f_0") + String commandAndFallbackErrorsComposition() { + throw new UserException(); + } + + + String commandAndFallbackErrorsComposition_f_0(Throwable commandError) { + throw new HystrixFlowException(commandError, new UserException(1)); + } + + /*********************************************************************************/ + + @HystrixCommand(fallbackMethod = "commandWithFallbackThatFailsByTimeOut_f_0") + String commandWithFallbackThatFailsByTimeOut() { + throw new UserException(0); + } + + @HystrixCommand(commandProperties = {@HystrixProperty(name = "execution.isolation.thread.timeoutInMilliseconds", value = "1")}) + String commandWithFallbackThatFailsByTimeOut_f_0() { + return ""; + } } private class FailoverService { @@ -241,4 +375,26 @@ private RuntimeOperationException(String message) { } } + static class UserException extends RuntimeException { + final int level; + + public UserException() { + this(0); + } + + public UserException(int level) { + this.level = level; + } + } + + static class HystrixFlowException extends RuntimeException { + final Throwable commandException; + final Throwable fallbackException; + + public HystrixFlowException(Throwable commandException, Throwable fallbackException) { + this.commandException = commandException; + this.fallbackException = fallbackException; + } + } + }