Added ExecutionHook.onError call to HystrixBadRequestException handling

Netflix · Dec 27, 2014 · 7d740d8 · 7d740d8
1 parent 65ed5fb
commit 7d740d8
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 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
@@ -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
@@ -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
@@ -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());