Merge pull request #456 from gerrit-hntschl/badrequestexception-from-executionhook

Bypass fallback/circuit breaker for BadRequestExceptions from ExecutionHook

Author: mattrjacobs

hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java

@@ -624,6 +624,12 @@ public Observable<R> call(Throwable t) {
                     } catch (Exception hookException) {
                         logger.warn("Error calling ExecutionHook.endRunFailure", hookException);
                     }
+                    /*
+                     * Treat HystrixBadRequestException from ExecutionHook like a plain HystrixBadRequestException.
+                     */
+                    if (e instanceof HystrixBadRequestException){
+                        return Observable.error(e);
+                    }
 
                     logger.debug("Error executing HystrixCommand.run(). Proceeding to fallback logic ...", e);
 

hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java

@@ -4347,6 +4347,25 @@ public void call(Throwable t1) {
         assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
     }
 
+    @Test
+    public void testExceptionConvertedToBadRequestExceptionInExecutionHookBypassesCircuitBreaker(){
+        TestCircuitBreaker circuitBreaker = new TestCircuitBreaker();
+        try {
+            new ExceptionToBadRequestByExecutionHookCommand(circuitBreaker, ExecutionIsolationStrategy.THREAD).execute();
+            fail("we expect to receive a " + HystrixBadRequestException.class.getSimpleName());
+        } catch (HystrixBadRequestException e) {
+            // success
+            e.printStackTrace();
+        } catch (Exception e) {
+            e.printStackTrace();
+            fail("We expect a " + HystrixBadRequestException.class.getSimpleName() + " but got a " + e.getClass().getSimpleName());
+        }
+
+        assertEquals(0, circuitBreaker.metrics.getRollingCount(HystrixRollingNumberEvent.SUCCESS));
+        assertEquals(0, circuitBreaker.metrics.getRollingCount(HystrixRollingNumberEvent.EXCEPTION_THROWN));
+        assertEquals(0, circuitBreaker.metrics.getRollingCount(HystrixRollingNumberEvent.FAILURE));
+    }
+
     /* ******************************************************************************** */
     /* ******************************************************************************** */
     /* private HystrixCommand class implementations for unit testing */
@@ -4435,6 +4454,11 @@ TestCommandBuilder setExecutionSemaphore(TryableSemaphore executionSemaphore) {
                 return this;
             }
 
+            TestCommandBuilder setExecutionHook(TestExecutionHook executionHook) {
+                this.executionHook = executionHook;
+                return this;
+            }
+
         }
 
     }
@@ -5217,6 +5241,37 @@ protected String getCacheKey() {
 
     }
 
+    private static class BusinessException extends Exception {
+        public BusinessException(String msg) {
+            super(msg);
+        }
+    }
+
+    private static class ExceptionToBadRequestByExecutionHookCommand extends TestHystrixCommand<Boolean> {
+        public ExceptionToBadRequestByExecutionHookCommand(TestCircuitBreaker circuitBreaker, ExecutionIsolationStrategy isolationType) {
+            super(testPropsBuilder()
+                    .setCircuitBreaker(circuitBreaker)
+                    .setCommandPropertiesDefaults(HystrixCommandPropertiesTest.getUnitTestPropertiesSetter().withExecutionIsolationStrategy(isolationType))
+                    .setExecutionHook(new TestExecutionHook(){
+                        @Override
+                        public <T> Exception onRunError(HystrixInvokable<T> commandInstance, Exception e) {
+                            super.onRunError(commandInstance, e);
+                            return new HystrixBadRequestException("autoconverted exception", e);
+                        }
+                    }));
+        }
+
+        @Override
+        protected Boolean run() throws BusinessException {
+            throw new BusinessException("invalid input by the user");
+        }
+
+        @Override
+        protected String getCacheKey() {
+            return "nein";
+        }
+    }
+
     private static class CommandWithErrorThrown extends TestHystrixCommand<Boolean> {
 
         public CommandWithErrorThrown(TestCircuitBreaker circuitBreaker) {
@@ -5404,4 +5459,5 @@ public <T> void onThreadComplete(HystrixInvokable<T> commandInstance) {
         }
 
     }
+
 }