iss1344: send fallback exception to client instead of primary command

elivn · Oct 19, 2016 · 1042f27 · 1042f27
1 parent c175837
commit 1042f27
Show file tree

Hide file tree

Showing 6 changed files with 202 additions and 12 deletions.
diff --git a/.../src/main/java/com/netflix/hystrix/contrib/javanica/aop/aspectj/HystrixCommandAspect.java b/.../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/...anica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java b/...anica/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/...x-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/GenericCommand.java b/...x-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/...javanica/src/main/java/com/netflix/hystrix/contrib/javanica/exception/ExceptionUtils.java b/...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/.../netflix/hystrix/contrib/javanica/test/common/error/BasicDefaultIgnoreExceptionsTest.java b/.../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/...ava/com/netflix/hystrix/contrib/javanica/test/common/error/BasicErrorPropagationTest.java b/...ava/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;
+        }
+    }
+
 }