#1536 handle not wrapped exceptions same way as all other

hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicErrorPropagationTest.java

@@ -16,6 +16,7 @@
 package com.netflix.hystrix.contrib.javanica.test.common.error;
 
 import com.netflix.hystrix.HystrixEventType;
+import com.netflix.hystrix.HystrixInvokableInfo;
 import com.netflix.hystrix.HystrixRequestLog;
 import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand;
 import com.netflix.hystrix.contrib.javanica.annotation.HystrixProperty;
@@ -198,24 +199,39 @@ public void testCommandWithFallbackThatFailsByTimeOut() {
  }
 
  @Test
- public void testCommandThrowsNotWrappedException() {
+ public void testCommandWithNotWrappedExceptionAndNoFallback() {
  try {
- userService.throwNotWrappedCheckedException();
+ userService.throwNotWrappedCheckedExceptionWithoutFallback();
  fail();
  } catch (NotWrappedCheckedException e) {
  // pass
  } catch (Throwable e) {
  fail("'NotWrappedCheckedException' is expected exception.");
  }finally {
  assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
- com.netflix.hystrix.HystrixInvokableInfo getUserCommand = getHystrixCommandByKey("throwNotWrappedCheckedException");
+ HystrixInvokableInfo getUserCommand = getHystrixCommandByKey("throwNotWrappedCheckedExceptionWithoutFallback");
  // record failure in metrics
  assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.FAILURE));
  // and will not trigger fallback logic
  verify(failoverService, never()).activate();
  }
  }
 
+ @Test
+ public void testCommandWithNotWrappedExceptionAndFallback() {
+ try {
+ userService.throwNotWrappedCheckedExceptionWithFallback();
+ } catch (NotWrappedCheckedException e) {
+ fail();
+ } finally {
+ assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
+ HystrixInvokableInfo getUserCommand = getHystrixCommandByKey("throwNotWrappedCheckedExceptionWithFallback");
+ assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.FAILURE));
+ assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.FALLBACK_SUCCESS));
+ verify(failoverService).activate();
+ }
+ }
+
  public static class UserService {
 
  private FailoverService failoverService;
@@ -288,8 +304,13 @@ private void validate(String val) throws BadRequestException {
  }
  }
 
+ @HystrixCommand
+ void throwNotWrappedCheckedExceptionWithoutFallback() throws NotWrappedCheckedException {
+ throw new NotWrappedCheckedException();
+ }
+
  @HystrixCommand(fallbackMethod = "voidFallback")
- void throwNotWrappedCheckedException() throws NotWrappedCheckedException {
+ void throwNotWrappedCheckedExceptionWithFallback() throws NotWrappedCheckedException {
  throw new NotWrappedCheckedException();
  }
 

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

@@ -760,11 +760,7 @@ private Observable<R> getFallbackOrThrowException(final AbstractCommand<R> _cmd,
  // do this before executing fallback so it can be queried from within getFallback (see See https://github.com/Netflix/Hystrix/pull/144)
  executionResult = executionResult.addEvent((int) latency, eventType);
 
- if (shouldNotBeWrapped(originalException)){
- /* executionHook for all errors */
- Exception e = wrapWithOnErrorHook(failureType, originalException);
- return Observable.error(e);
- } else if (isUnrecoverable(originalException)) {
+ if (isUnrecoverable(originalException)) {
  logger.error("Unrecoverable Error for HystrixCommand so will throw HystrixRuntimeException and not apply fallback. ", originalException);
 
  /* executionHook for all errors */
@@ -807,30 +803,33 @@ public void call() {
  final Func1<Throwable, Observable<R>> handleFallbackError = new Func1<Throwable, Observable<R>>() {
  @Override
  public Observable<R> call(Throwable t) {
- Exception e = originalException;
+ /* executionHook for all errors */
+ Exception e = wrapWithOnErrorHook(failureType, originalException);
  Exception fe = getExceptionFromThrowable(t);
 
+ long latency = System.currentTimeMillis() - executionResult.getStartTimestamp();
+ Exception toEmit;
+
  if (fe instanceof UnsupportedOperationException) {
- long latency = System.currentTimeMillis() - executionResult.getStartTimestamp();
  logger.debug("No fallback for HystrixCommand. ", fe); // debug only since we're throwing the exception and someone higher will do something with it
  eventNotifier.markEvent(HystrixEventType.FALLBACK_MISSING, commandKey);
  executionResult = executionResult.addEvent((int) latency, HystrixEventType.FALLBACK_MISSING);
 
- /* executionHook for all errors */
- e = wrapWithOnErrorHook(failureType, e);
-
- return Observable.error(new HystrixRuntimeException(failureType, _cmd.getClass(), getLogMessagePrefix() + " " + message + " and no fallback available.", e, fe));
+ toEmit = new HystrixRuntimeException(failureType, _cmd.getClass(), getLogMessagePrefix() + " " + message + " and no fallback available.", e, fe);
  } else {
- long latency = System.currentTimeMillis() - executionResult.getStartTimestamp();
  logger.debug("HystrixCommand execution " + failureType.name() + " and fallback failed.", fe);
  eventNotifier.markEvent(HystrixEventType.FALLBACK_FAILURE, commandKey);
  executionResult = executionResult.addEvent((int) latency, HystrixEventType.FALLBACK_FAILURE);
 
- /* executionHook for all errors */
-  e = wrapWithOnErrorHook(failureType, e);
+ toEmit = new HystrixRuntimeException(failureType, _cmd.getClass(), getLogMessagePrefix() + " " + message + " and fallback failed.", e, fe);
+ }
 
- return Observable.error(new HystrixRuntimeException(failureType, _cmd.getClass(), getLogMessagePrefix() + " " + message + " and fallback failed.", e, fe));
+ // NOTE: we're suppressing fallback exception here
+ if (shouldNotBeWrapped(originalException)) {
+ return Observable.error(e);
  }
+
+ return Observable.error(toEmit);
  }
  };
 

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

@@ -190,7 +190,7 @@ public void testNotWrappedExceptionWithNoFallback() {
 
  assertTrue(command.getExecutionTimeInMilliseconds() > -1);
  assertTrue(command.isFailedExecution());
- assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
+ assertCommandExecutionEvents(command, HystrixEventType.FAILURE, HystrixEventType.FALLBACK_MISSING);
  assertNotNull(command.getExecutionException());
  assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestRuntimeException);
  assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
@@ -223,6 +223,28 @@ public void testNotWrappedBadRequestWithNoFallback() {
  assertSaneHystrixRequestLog(1);
  }
 
+ @Test
+ public void testNotWrappedBadRequestWithFallback() throws Exception {
+ TestHystrixCommand<Integer> command = getCommand(ExecutionIsolationStrategy.THREAD, AbstractTestHystrixCommand.ExecutionResult.BAD_REQUEST_NOT_WRAPPED, AbstractTestHystrixCommand.FallbackResult.SUCCESS);
+ try {
+ command.execute();
+ fail("we shouldn't get here");
+ } catch (HystrixRuntimeException e) {
+ e.printStackTrace();
+ fail("we shouldn't get a HystrixRuntimeException");
+ } catch (RuntimeException e) {
+ assertTrue(e instanceof NotWrappedByHystrixTestRuntimeException);
+ }
+
+ assertTrue(command.getExecutionTimeInMilliseconds() > -1);
+ assertTrue(command.getEventCounts().contains(HystrixEventType.BAD_REQUEST));
+ assertCommandExecutionEvents(command, HystrixEventType.BAD_REQUEST);
+ assertNotNull(command.getExecutionException());
+ assertTrue(command.getExecutionException() instanceof HystrixBadRequestException);
+ assertTrue(command.getExecutionException().getCause() instanceof NotWrappedByHystrixTestRuntimeException);
+ assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
+ assertSaneHystrixRequestLog(1);
+ }
 
  /**
  * Test a command execution that fails but has a fallback.
@@ -246,20 +268,12 @@ public void testExecutionFailureWithFallback() {
  @Test
  public void testNotWrappedExceptionWithFallback() {
  TestHystrixCommand<Integer> command = getCommand(ExecutionIsolationStrategy.THREAD, AbstractTestHystrixCommand.ExecutionResult.NOT_WRAPPED_FAILURE, AbstractTestHystrixCommand.FallbackResult.SUCCESS);
- try {
- command.execute();
- fail("we shouldn't get here");
- } catch (HystrixRuntimeException e) {
- e.printStackTrace();
- fail("we shouldn't get a HystrixRuntimeException");
- } catch (RuntimeException e) {
- assertTrue(e instanceof NotWrappedByHystrixTestRuntimeException);
- }
+ assertEquals(FlexibleTestHystrixCommand.FALLBACK_VALUE, command.execute());
+ assertEquals("Raw exception for TestHystrixCommand", command.getFailedExecutionException().getMessage());
  assertTrue(command.getExecutionTimeInMilliseconds() > -1);
  assertTrue(command.isFailedExecution());
- assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
+ assertCommandExecutionEvents(command, HystrixEventType.FAILURE, HystrixEventType.FALLBACK_SUCCESS);
  assertNotNull(command.getExecutionException());
- assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestRuntimeException);
  assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
  assertSaneHystrixRequestLog(1);
  }
@@ -2377,7 +2391,7 @@ public void onNext(Boolean args) {
  assertTrue(t.get() instanceof NotWrappedByHystrixTestException);
  assertTrue(command.getExecutionTimeInMilliseconds() > -1);
  assertTrue(command.isFailedExecution());
- assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
+ assertCommandExecutionEvents(command, HystrixEventType.FAILURE, HystrixEventType.FALLBACK_MISSING);
  assertNotNull(command.getExecutionException());
  assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestException);
  assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());

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

@@ -5,4 +5,7 @@
 public class NotWrappedByHystrixTestRuntimeException extends RuntimeException implements ExceptionNotWrappedByHystrix {
  private static final long serialVersionUID = 1L;
 
+ public NotWrappedByHystrixTestRuntimeException() {
+ super("Raw exception for TestHystrixCommand");
+ }
 }