From 7d740d8a459c176552fa01bce87577c9e9a8e569 Mon Sep 17 00:00:00 2001
From: Matt Jacobs <mjacobs@netflix.com>
Date: Sat, 27 Dec 2014 15:41:55 -0500
Subject: [PATCH] Added ExecutionHook.onError call to
 HystrixBadRequestException handling

---
 .../java/com/netflix/hystrix/AbstractCommand.java    | 12 +++++++++++-
 .../hystrix/exception/HystrixRuntimeException.java   |  2 +-
 .../java/com/netflix/hystrix/HystrixCommandTest.java |  8 ++++----
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java
index f4a9830a3..a64afdd8d 100644
--- a/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java
+++ b/hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java
@@ -561,7 +561,6 @@ public void call(Notification<? super R> n) {
 
             @Override
             public R call(R t1) {
-                System.out.println("map " + t1);
                 if (!once) {
                     // report success
                     executionResult = executionResult.addEvents(HystrixEventType.SUCCESS);
@@ -617,6 +616,17 @@ public Observable<R> call(Throwable t) {
                         logger.warn("Error calling ExecutionHook.onRunError", hookException);
                     }
 
+                    try {
+                        Exception decorated = executionHook.onError(_self, FailureType.BAD_REQUEST_EXCEPTION, (Exception) t);
+
+                        if (decorated instanceof HystrixBadRequestException) {
+                            t = (HystrixBadRequestException) decorated;
+                        } else {
+                            logger.warn("ExecutionHook.onError returned an exception that was not an instance of HystrixBadRequestException so will be ignored.", decorated);
+                        }
+                    } catch (Exception hookException) {
+                        logger.warn("Error calling ExecutionHook.onError", hookException);
+                    }
                     /*
                      * HystrixBadRequestException is treated differently and allowed to propagate without any stats tracking or fallback logic
                      */
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/exception/HystrixRuntimeException.java b/hystrix-core/src/main/java/com/netflix/hystrix/exception/HystrixRuntimeException.java
index 6c1dd5052..b70a877f2 100644
--- a/hystrix-core/src/main/java/com/netflix/hystrix/exception/HystrixRuntimeException.java
+++ b/hystrix-core/src/main/java/com/netflix/hystrix/exception/HystrixRuntimeException.java
@@ -32,7 +32,7 @@ public class HystrixRuntimeException extends RuntimeException {
     private final FailureType failureCause;
 
     public static enum FailureType {
-        COMMAND_EXCEPTION, TIMEOUT, SHORTCIRCUIT, REJECTED_THREAD_EXECUTION, REJECTED_SEMAPHORE_EXECUTION, REJECTED_SEMAPHORE_FALLBACK
+        BAD_REQUEST_EXCEPTION, COMMAND_EXCEPTION, TIMEOUT, SHORTCIRCUIT, REJECTED_THREAD_EXECUTION, REJECTED_SEMAPHORE_EXECUTION, REJECTED_SEMAPHORE_FALLBACK
     }
 
     public HystrixRuntimeException(FailureType failureCause, Class<? extends HystrixInvokable> commandClass, String message, Exception cause, Throwable fallbackException) {
diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java
index 30430edc8..5f2e1736b 100644
--- a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java
+++ b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java
@@ -4184,12 +4184,12 @@ public void testExecutionHookFailedOnHystrixBadRequestWithSemaphoreIsolation() {
 
         // the run() method should run as we're not short-circuited or rejected
         assertEquals(1, command.builder.executionHook.startRun.get());
-        // we expect a successful response from run()
+        // we expect no response from run()
         assertNull(command.builder.executionHook.runSuccessResponse);
         // we expect an exception
         assertNotNull(command.builder.executionHook.runFailureException);
 
-        // the fallback() method should not be run as we were successful
+        // the fallback() method should not be run as BadRequestException is handled by immediate propagation
         assertEquals(0, command.builder.executionHook.startFallback.get());
         // null since it didn't run
         assertNull(command.builder.executionHook.fallbackSuccessResponse);
@@ -4200,8 +4200,8 @@ public void testExecutionHookFailedOnHystrixBadRequestWithSemaphoreIsolation() {
         assertEquals(1, command.builder.executionHook.startExecute.get());
         // we should not have a response from execute()
         assertNull(command.builder.executionHook.endExecuteSuccessResponse);
-        // we should not have an exception since run() succeeded
-        assertNull(command.builder.executionHook.endExecuteFailureException);
+        // we should have a HystrixBadRequest exception since run() succeeded
+        assertNotNull(command.builder.executionHook.endExecuteFailureException);
 
         // thread execution
         assertEquals(0, command.builder.executionHook.threadStart.get());