Fix for issue #332 (Hystrix Javanica Error Propagation does not work cor...

hystrix-contrib/hystrix-javanica/build.gradle

@@ -5,22 +5,24 @@ apply plugin: 'idea'
 aspectjVersion = '1.7.4'
 
 dependencies {
-  compile project(':hystrix-core')
+    compile project(':hystrix-core')
 
-  compile "org.aspectj:aspectjweaver:$aspectjVersion"
-  compile "org.aspectj:aspectjrt:$aspectjVersion"
+    compile "org.aspectj:aspectjweaver:$aspectjVersion"
+    compile "org.aspectj:aspectjrt:$aspectjVersion"
 
-  compile 'com.google.guava:guava:15.0'
-  compile 'commons-collections:commons-collections:3.2.1'
-  compile 'org.apache.commons:commons-lang3:3.1'
-  compile 'com.google.code.findbugs:jsr305:2.0.0'
-  testCompile 'junit:junit-dep:4.10'
-  testCompile 'org.springframework:spring-core:4.0.0.RELEASE'
-  testCompile 'org.springframework:spring-context:4.0.0.RELEASE'
-  testCompile 'org.springframework:spring-aop:4.0.0.RELEASE'
-  testCompile 'org.springframework:spring-test:4.0.0.RELEASE'
-  testCompile 'cglib:cglib:3.1'
-  testCompile 'org.mockito:mockito-all:1.9.5'
+    compile 'com.google.guava:guava:15.0'
+    compile 'commons-collections:commons-collections:3.2.1'
+    compile 'org.apache.commons:commons-lang3:3.1'
+    compile 'com.google.code.findbugs:jsr305:2.0.0'
+    testCompile 'junit:junit-dep:4.10'
+    testCompile 'org.springframework:spring-core:4.0.0.RELEASE'
+    testCompile 'org.springframework:spring-context:4.0.0.RELEASE'
+    testCompile 'org.springframework:spring-aop:4.0.0.RELEASE'
+    testCompile 'org.springframework:spring-test:4.0.0.RELEASE'
+    testCompile 'cglib:cglib:3.1'
+    testCompile 'org.mockito:mockito-all:1.9.5'
+    testCompile 'log4j:log4j:1.2.17'
+    testCompile 'org.slf4j:slf4j-log4j12:1.7.7'
 }
 
 eclipse {

hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java

@@ -15,6 +15,7 @@
  */
 package com.netflix.hystrix.contrib.javanica.collapser;
 
+import com.google.common.base.Optional;
 import com.netflix.hystrix.HystrixCollapser;
 import com.netflix.hystrix.HystrixCollapserKey;
 import com.netflix.hystrix.HystrixCommand;
@@ -33,7 +34,7 @@
  * Collapses multiple requests into a single {@link HystrixCommand} execution based
  * on a time window and optionally a max batch size.
  */
-public class CommandCollapser extends HystrixCollapser<List<Object>, Object, Object> {
+public class CommandCollapser extends HystrixCollapser<List<Optional<Object>>, Object, Object> {
 
     private MetaHolder metaHolder;
 
@@ -69,7 +70,7 @@ public Object getRequestArgument() {
      * Creates batch command.
      */
     @Override
-    protected HystrixCommand<List<Object>> createCommand(
+    protected HystrixCommand<List<Optional<Object>>> createCommand(
             Collection<CollapsedRequest<Object, Object>> collapsedRequests) {
         BatchHystrixCommand command = BatchHystrixCommandFactory.getInstance().create(metaHolder, collapsedRequests);
         command.setFallbackEnabled(metaHolder.getHystrixCollapser().fallbackEnabled());
@@ -80,14 +81,17 @@ protected HystrixCommand<List<Object>> createCommand(
      * {@inheritDoc}
      */
     @Override
-    protected void mapResponseToRequests(List<Object> batchResponse,
+    protected void mapResponseToRequests(List<Optional<Object>> batchResponse,
                                          Collection<CollapsedRequest<Object, Object>> collapsedRequests) {
         if (batchResponse.size() < collapsedRequests.size()) {
             throw new RuntimeException(createMessage(collapsedRequests, batchResponse));
         }
         int count = 0;
         for (CollapsedRequest<Object, Object> request : collapsedRequests) {
-            request.setResponse(batchResponse.get(count++));
+            Optional response = batchResponse.get(count++);
+            if (response.isPresent()) { // allows prevent IllegalStateException
+                request.setResponse(response.get());
+            }
         }
     }
 
@@ -116,7 +120,7 @@ public Setter build() {
     }
 
     private String createMessage(Collection<CollapsedRequest<Object, Object>> requests,
-                                 List<Object> response) {
+                                 List<Optional<Object>> response) {
         return ERROR_MSG + arrayFormat(ERROR_MSF_TEMPLATE, new Object[]{getCollapserKey().name(), requests.size(), response.size()}).getMessage();
     }
 

hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java

@@ -19,7 +19,9 @@
 import com.google.common.base.Throwables;
 import com.netflix.hystrix.HystrixCollapser;
 import com.netflix.hystrix.contrib.javanica.conf.HystrixPropertiesManager;
+import com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException;
 import com.netflix.hystrix.exception.HystrixBadRequestException;
+import com.netflix.hystrix.exception.HystrixRuntimeException;
 
 import javax.annotation.concurrent.ThreadSafe;
 import java.util.Collection;
@@ -136,12 +138,18 @@ protected String getCacheKey() {
         return key;
     }
 
+    /**
+     * Check whether triggered exception is ignorable.
+     *
+     * @param throwable the exception occurred during a command execution
+     * @return true if exception is ignorable, otherwise - false
+     */
     boolean isIgnorable(Throwable throwable) {
         if (ignoreExceptions == null || ignoreExceptions.length == 0) {
             return false;
         }
         for (Class<? extends Throwable> ignoreException : ignoreExceptions) {
-            if (throwable.getClass().isAssignableFrom(ignoreException)) {
+            if (ignoreException.isAssignableFrom(throwable.getClass())) {
                 return true;
             }
         }
@@ -150,20 +158,28 @@ boolean isIgnorable(Throwable throwable) {
 
     /**
      * Executes an action. If an action has failed and an exception is ignorable then propagate it as HystrixBadRequestException
-     * otherwise propagate it as RuntimeException to trigger fallback method.
+     * otherwise propagate original exception to trigger fallback method.
+     * Note: If an exception occurred in a command directly extends {@link java.lang.Throwable} then this exception cannot be re-thrown
+     * as original exception because HystrixCommand.run() allows throw subclasses of {@link java.lang.Exception}.
+     * Thus we need to wrap cause in RuntimeException, anyway in this case the fallback logic will be triggered.
      *
      * @param action the action
      * @return result of command action execution
      */
-    Object process(Action action) throws RuntimeException {
+    Object process(Action action) throws Exception {
         Object result;
         try {
             result = action.execute();
-        } catch (Throwable throwable) {
-            if (isIgnorable(throwable)) {
-                throw new HystrixBadRequestException(throwable.getMessage(), throwable);
+        } catch (CommandActionExecutionException throwable) {
+            Throwable cause = throwable.getCause();
+            if (isIgnorable(cause)) {
+                throw new HystrixBadRequestException(cause.getMessage(), cause);
+            }
+            if (cause instanceof Exception) {
+                throw (Exception) cause;
+            } else {
+                throw Throwables.propagate(cause);
             }
-            throw Throwables.propagate(throwable);
         }
         return result;
     }
@@ -186,7 +202,54 @@ protected T getFallback() {
      * Common action.
      */
     abstract class Action {
-        abstract Object execute();
+        /**
+         * Each implementation of this method should wrap any exceptions in CommandActionExecutionException.
+         *
+         * @return execution result
+         * @throws CommandActionExecutionException
+         */
+        abstract Object execute() throws CommandActionExecutionException;
+    }
+
+
+    /**
+     * Builder to create error message for failed fallback operation.
+     */
+    static class FallbackErrorMessageBuilder {
+        private StringBuilder builder = new StringBuilder("failed to processed fallback");
+
+        static FallbackErrorMessageBuilder create() {
+            return new FallbackErrorMessageBuilder();
+        }
+
+        public FallbackErrorMessageBuilder append(CommandAction action, Throwable throwable) {
+            return commandAction(action).exception(throwable);
+        }
+
+        private FallbackErrorMessageBuilder commandAction(CommandAction action) {
+            if (action instanceof CommandExecutionAction || action instanceof LazyCommandExecutionAction) {
+                builder.append(": '").append(action.getActionName()).append("'. ")
+                        .append(action.getActionName()).append(" fallback is a hystrix command. ");
+            } else if (action instanceof MethodExecutionAction) {
+                builder.append(" is the method: '").append(action.getActionName()).append("'. ");
+            }
+            return this;
+        }
+
+        private FallbackErrorMessageBuilder exception(Throwable throwable) {
+            if (throwable instanceof HystrixBadRequestException) {
+                builder.append("exception: '").append(throwable.getCause().getClass())
+                        .append("' occurred in fallback was ignored and wrapped to HystrixBadRequestException.\n");
+            } else if (throwable instanceof HystrixRuntimeException) {
+                builder.append("exception: '").append(throwable.getCause().getClass())
+                        .append("' occurred in fallback wasn't ignored.\n");
+            }
+            return this;
+        }
+
+        public String build() {
+            return builder.toString();
+        }
     }
 
 }

hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java

@@ -16,9 +16,13 @@
 package com.netflix.hystrix.contrib.javanica.command;
 
 
+import com.google.common.base.Optional;
 import com.google.common.collect.Lists;
 import com.netflix.hystrix.HystrixCollapser;
+import com.netflix.hystrix.contrib.javanica.exception.FallbackInvocationException;
 import com.netflix.hystrix.exception.HystrixBadRequestException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.annotation.concurrent.ThreadSafe;
 import java.util.Collection;
@@ -29,7 +33,9 @@
  * This command is used in collapser.
  */
 @ThreadSafe
-public class BatchHystrixCommand extends AbstractHystrixCommand<List<Object>> {
+public class BatchHystrixCommand extends AbstractHystrixCommand<List<Optional<Object>>> {
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(GenericCommand.class);
 
     /**
      * If some error occurs during the processing in run() then if {@link #fallbackEnabled} is true then the {@link #processWithFallback}
@@ -63,22 +69,21 @@ public void setFallbackEnabled(boolean fallbackEnabled) {
      * {@inheritDoc}
      */
     @Override
-    protected List<Object> run() throws Exception {
-        List<Object> response = Lists.newArrayList();
+    protected List<Optional<Object>> run() throws Exception {
+        List<Optional<Object>> response = Lists.newArrayList();
         for (HystrixCollapser.CollapsedRequest<Object, Object> request : getCollapsedRequests()) {
             final Object[] args = (Object[]) request.getArgument();
             try {
-                response.add(fallbackEnabled ? processWithFallback(args) : process(args));
-            } catch (RuntimeException ex) {
+                response.add(Optional.of(fallbackEnabled ? processWithFallback(args) : process(args)));
+            } catch (Exception ex) {
                 request.setException(ex);
-                response.add(null);
+                response.add(Optional.absent());
             }
-
         }
         return response;
     }
 
-    private Object process(final Object[] args) {
+    private Object process(final Object[] args) throws Exception {
         return process(new Action() {
             @Override
             Object execute() {
@@ -87,29 +92,44 @@ Object execute() {
         });
     }
 
-    private Object processWithFallback(final Object[] args) {
-        Object result = null;
+    private Object processWithFallback(final Object[] args) throws Exception {
+        Object result;
         try {
             result = process(args);
-        } catch (RuntimeException ex) {
-            if (ex instanceof HystrixBadRequestException || getFallbackAction() == null) {
+        } catch (Exception ex) {
+            if (ex instanceof HystrixBadRequestException) {
                 throw ex;
             } else {
                 if (getFallbackAction() != null) {
                     result = processFallback(args);
+                } else {
+                    // if command doesn't have fallback then
+                    // call super.getFallback() that throws exception by default.
+                    result = super.getFallback();
                 }
             }
         }
         return result;
     }
 
     private Object processFallback(final Object[] args) {
-        return process(new Action() {
-            @Override
-            Object execute() {
-                return getFallbackAction().executeWithArgs(ExecutionType.SYNCHRONOUS, args);
+        if (getFallbackAction() != null) {
+            final CommandAction commandAction = getFallbackAction();
+            try {
+                return process(new Action() {
+                    @Override
+                    Object execute() {
+                        return commandAction.executeWithArgs(ExecutionType.SYNCHRONOUS, args);
+                    }
+                });
+            } catch (Throwable e) {
+                LOGGER.error(FallbackErrorMessageBuilder.create()
+                        .append(commandAction, e).build());
+                throw new FallbackInvocationException(e.getCause());
             }
-        });
+        } else {
+            return super.getFallback();
+        }
     }
 
 

hystrix-contrib/hystrix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/CommandAction.java

@@ -15,13 +15,37 @@
  */
 package com.netflix.hystrix.contrib.javanica.command;
 
+import com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException;
+
 /**
  * Simple action to encapsulate some logic to process it in a Hystrix command.
  */
 public abstract class CommandAction {
 
-    public abstract Object execute(ExecutionType executionType);
+    /**
+     * Executes action in accordance with the given execution type.
+     *
+     * @param executionType the execution type
+     * @return result of execution
+     * @throws com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException
+     */
+    public abstract Object execute(ExecutionType executionType) throws CommandActionExecutionException;
+
+    /**
+     * Executes action with parameters in accordance with the given execution ty
+     *
+     * @param executionType the execution type
+     * @param args          the parameters of the action
+     * @return result of execution
+     * @throws com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException
+     */
+    public abstract Object executeWithArgs(ExecutionType executionType, Object[] args) throws CommandActionExecutionException;
 
-    public abstract Object executeWithArgs(ExecutionType executionType, Object[] args);
+    /**
+     * Gets action name. Useful for debugging.
+     *
+     * @return the action name
+     */
+    public abstract String getActionName();
 
 }