Introduce NotWrappedByHystrix to mark Exceptions that should not be w…

…rapped in HystrixRunetimeException
buaaxzl · Nov 6, 2016 · 9c357df · 9c357df
1 parent 7598e9a
commit 9c357df
Show file tree

Hide file tree

Showing 10 changed files with 214 additions and 14 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
@@ -17,6 +17,7 @@
 
 import com.netflix.hystrix.HystrixCircuitBreaker.NoOpCircuitBreaker;
 import com.netflix.hystrix.HystrixCommandProperties.ExecutionIsolationStrategy;
+import com.netflix.hystrix.exception.ExceptionNotWrappedByHystrix;
 import com.netflix.hystrix.exception.HystrixBadRequestException;
 import com.netflix.hystrix.exception.HystrixRuntimeException;
 import com.netflix.hystrix.exception.HystrixRuntimeException.FailureType;
@@ -749,12 +750,15 @@ 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 (isUnrecoverable(originalException)) {
-            Exception e = originalException;
-            logger.error("Unrecoverable Error for HystrixCommand so will throw HystrixRuntimeException and not apply fallback. ", e);
+        if (shouldNotBeWrapped(originalException)){
+            /* executionHook for all errors */
+            Exception e = wrapWithOnErrorHook(failureType, originalException);
+            return Observable.error(e);
+        } else if (isUnrecoverable(originalException)) {
+            logger.error("Unrecoverable Error for HystrixCommand so will throw HystrixRuntimeException and not apply fallback. ", originalException);
 
             /* executionHook for all errors */
-            e = wrapWithOnErrorHook(failureType, e);
+            Exception e = wrapWithOnErrorHook(failureType, originalException);
             return Observable.error(new HystrixRuntimeException(failureType, this.getClass(), getLogMessagePrefix() + " " + message + " and encountered unrecoverable error.", e, null));
         } else {
             if (isRecoverableError(originalException)) {
@@ -1038,6 +1042,10 @@ private Observable<R> handleFallbackDisabledByEmittingError(Exception underlying
         return Observable.error(new HystrixRuntimeException(failureType, this.getClass(), getLogMessagePrefix() + " " + message + " and fallback disabled.", wrapped, null));
     }
 
+    protected boolean shouldNotBeWrapped(Throwable underlying) {
+        return underlying instanceof ExceptionNotWrappedByHystrix;
+    }
+
     /**
      * Returns true iff the t was caused by a java.lang.Error that is unrecoverable.  Note: not all java.lang.Errors are unrecoverable.
      * @see <a href="https://github.com/Netflix/Hystrix/issues/713"></a> for more context
@@ -1532,12 +1540,13 @@ private R wrapWithOnEmitHook(R r) {
     /**
      * Take an Exception and determine whether to throw it, its cause or a new HystrixRuntimeException.
      * <p>
-     * This will only throw an HystrixRuntimeException, HystrixBadRequestException or IllegalStateException
+     * This will only throw an HystrixRuntimeException, HystrixBadRequestException, IllegalStateException
+     * or any exception that implements ExceptionNotWrappedByHystrix.
      * 
      * @param e initial exception
      * @return HystrixRuntimeException, HystrixBadRequestException or IllegalStateException
      */
-    protected RuntimeException decomposeException(Exception e) {
+    protected Throwable decomposeException(Exception e) {
         if (e instanceof IllegalStateException) {
             return (IllegalStateException) e;
         }
@@ -1554,6 +1563,12 @@ protected RuntimeException decomposeException(Exception e) {
         if (e.getCause() instanceof HystrixRuntimeException) {
             return (HystrixRuntimeException) e.getCause();
         }
+        if (shouldNotBeWrapped(e)) {
+            return e;
+        }
+        if (shouldNotBeWrapped(e.getCause())) {
+            return e.getCause();
+        }
         // we don't know what kind of exception this is so create a generic message and throw a new HystrixRuntimeException
         String message = getLogMessagePrefix() + " failed while executing.";
         logger.debug(message, e); // debug only since we're throwing the exception and someone higher will do something with it

diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java b/hystrix-core/src/main/java/com/netflix/hystrix/HystrixCommand.java
@@ -23,6 +23,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
+import com.netflix.hystrix.util.Exceptions;
 import rx.Observable;
 import rx.functions.Action0;
 
@@ -342,7 +343,7 @@ public R execute() {
         try {
             return queue().get();
         } catch (Exception e) {
-            throw decomposeException(e);
+            throw Exceptions.sneakyThrow(decomposeException(e));
         }
     }
 
@@ -435,11 +436,11 @@ public R get(long timeout, TimeUnit unit) throws InterruptedException, Execution
                 f.get();
                 return f;
             } catch (Exception e) {
-                RuntimeException re = decomposeException(e);
-                if (re instanceof HystrixBadRequestException) {
+                Throwable t = decomposeException(e);
+                if (t instanceof HystrixBadRequestException) {
                     return f;
-                } else if (re instanceof HystrixRuntimeException) {
-                    HystrixRuntimeException hre = (HystrixRuntimeException) re;
+                } else if (t instanceof HystrixRuntimeException) {
+                    HystrixRuntimeException hre = (HystrixRuntimeException) t;
                     switch (hre.getFailureType()) {
 					case COMMAND_EXCEPTION:
 					case TIMEOUT:
@@ -450,7 +451,7 @@ public R get(long timeout, TimeUnit unit) throws InterruptedException, Execution
 						throw hre;
 					}
                 } else {
-                    throw re;
+                    throw Exceptions.sneakyThrow(t);
                 }
             }
         }

diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/exception/ExceptionNotWrappedByHystrix.java b/hystrix-core/src/main/java/com/netflix/hystrix/exception/ExceptionNotWrappedByHystrix.java
@@ -0,0 +1,8 @@
+package com.netflix.hystrix.exception;
+
+/**
+ * Exceptions can implement this interface to prevent Hystrix from wrapping detected exceptions in a HystrixRuntimeException
+ */
+public interface ExceptionNotWrappedByHystrix {
+
+}
diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/util/Exceptions.java b/hystrix-core/src/main/java/com/netflix/hystrix/util/Exceptions.java
@@ -0,0 +1,20 @@
+package com.netflix.hystrix.util;
+
+import java.util.LinkedList;
+import java.util.List;
+
+public class Exceptions {
+    private Exceptions() {
+    }
+
+    /**
+     * Throws the argument, return-type is RuntimeException so the caller can use a throw statement break out of the method
+     */
+    public static RuntimeException sneakyThrow(Throwable t) {
+        return Exceptions.<RuntimeException>doThrow(t);
+    }
+
+    private static <T extends Throwable> T doThrow(Throwable ex) throws T {
+        throw (T) ex;
+    }
+}
diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/AbstractTestHystrixCommand.java b/hystrix-core/src/test/java/com/netflix/hystrix/AbstractTestHystrixCommand.java
@@ -20,7 +20,7 @@
 public interface AbstractTestHystrixCommand<R> extends HystrixObservable<R>, InspectableBuilder {
 
     enum ExecutionResult {
-        SUCCESS, FAILURE, ASYNC_FAILURE, HYSTRIX_FAILURE, ASYNC_HYSTRIX_FAILURE, RECOVERABLE_ERROR, ASYNC_RECOVERABLE_ERROR, UNRECOVERABLE_ERROR, ASYNC_UNRECOVERABLE_ERROR, BAD_REQUEST, ASYNC_BAD_REQUEST, MULTIPLE_EMITS_THEN_SUCCESS, MULTIPLE_EMITS_THEN_FAILURE, NO_EMITS_THEN_SUCCESS
+        SUCCESS, FAILURE, ASYNC_FAILURE, HYSTRIX_FAILURE, NOT_WRAPPED_FAILURE, ASYNC_HYSTRIX_FAILURE, RECOVERABLE_ERROR, ASYNC_RECOVERABLE_ERROR, UNRECOVERABLE_ERROR, ASYNC_UNRECOVERABLE_ERROR, BAD_REQUEST, ASYNC_BAD_REQUEST, MULTIPLE_EMITS_THEN_SUCCESS, MULTIPLE_EMITS_THEN_FAILURE, NO_EMITS_THEN_SUCCESS
     }
 
     enum FallbackResult {

diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java
@@ -171,6 +171,31 @@ public void testExecutionFailureWithNoFallback() {
         assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
         assertSaneHystrixRequestLog(1);
     }
+
+    /**
+     * Test a command execution that throws an exception that should not be wrapped.
+     */
+    @Test
+    public void testNotWrappedExceptionWithNoFallback() {
+        TestHystrixCommand<Integer> command = getCommand(ExecutionIsolationStrategy.THREAD, AbstractTestHystrixCommand.ExecutionResult.NOT_WRAPPED_FAILURE, AbstractTestHystrixCommand.FallbackResult.UNIMPLEMENTED);
+        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.isFailedExecution());
+        assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
+        assertNotNull(command.getExecutionException());
+        assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestRuntimeException);
+        assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
+        assertSaneHystrixRequestLog(1);
+    }
 
     /**
      * Test a command execution that fails but has a fallback.
@@ -188,6 +213,30 @@ public void testExecutionFailureWithFallback() {
         assertSaneHystrixRequestLog(1);
     }
 
+    /**
+     * Test a command execution that throws exception that should not be wrapped but has a fallback.
+     */
+    @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);
+        }
+        assertTrue(command.getExecutionTimeInMilliseconds() > -1);
+        assertTrue(command.isFailedExecution());
+        assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
+        assertNotNull(command.getExecutionException());
+        assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestRuntimeException);
+        assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
+        assertSaneHystrixRequestLog(1);
+    }
+
     /**
      * Test a command execution that fails, has getFallback implemented but that fails as well.
      */
@@ -2258,6 +2307,56 @@ public void onNext(Boolean args) {
         assertSaneHystrixRequestLog(1);
     }
 
+    /**
+     * Test an Exception implementing NotWrappedByHystrix being thrown
+     *
+     * @throws InterruptedException
+     */
+    @Test
+    public void testNotWrappedExceptionViaObserve() throws InterruptedException {
+        TestCircuitBreaker circuitBreaker = new TestCircuitBreaker();
+        CommandWithNotWrappedByHystrixException command = new CommandWithNotWrappedByHystrixException(circuitBreaker);
+        final AtomicReference<Throwable> t = new AtomicReference<Throwable>();
+        final CountDownLatch latch = new CountDownLatch(1);
+        try {
+            command.observe().subscribe(new Observer<Boolean>() {
+
+                @Override
+                public void onCompleted() {
+                    latch.countDown();
+                }
+
+                @Override
+                public void onError(Throwable e) {
+                    t.set(e);
+                    latch.countDown();
+                }
+
+                @Override
+                public void onNext(Boolean args) {
+
+                }
+
+            });
+        } catch (Exception e) {
+            e.printStackTrace();
+            fail("we should not get anything thrown, it should be emitted via the Observer#onError method");
+        }
+
+        latch.await(1, TimeUnit.SECONDS);
+        assertNotNull(t.get());
+        t.get().printStackTrace();
+
+        assertTrue(t.get() instanceof NotWrappedByHystrixTestException);
+        assertTrue(command.getExecutionTimeInMilliseconds() > -1);
+        assertTrue(command.isFailedExecution());
+        assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
+        assertNotNull(command.getExecutionException());
+        assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestException);
+        assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
+        assertSaneHystrixRequestLog(1);
+    }
+
     @Test
     public void testSemaphoreExecutionWithTimeout() {
         TestHystrixCommand<Boolean> cmd = new InterruptibleCommand(new TestCircuitBreaker(), false);
@@ -4750,6 +4849,8 @@ protected Integer run() throws Exception {
                 return FlexibleTestHystrixCommand.EXECUTE_VALUE;
             } else if (executionResult == AbstractTestHystrixCommand.ExecutionResult.FAILURE) {
                 throw new RuntimeException("Execution Failure for TestHystrixCommand");
+            } else if (executionResult == AbstractTestHystrixCommand.ExecutionResult.NOT_WRAPPED_FAILURE) {
+                throw new NotWrappedByHystrixTestRuntimeException();
             } else if (executionResult == AbstractTestHystrixCommand.ExecutionResult.HYSTRIX_FAILURE) {
                 throw new HystrixRuntimeException(HystrixRuntimeException.FailureType.COMMAND_EXCEPTION, AbstractFlexibleTestHystrixCommand.class, "Execution Hystrix Failure for TestHystrixCommand", new RuntimeException("Execution Failure for TestHystrixCommand"), new RuntimeException("Fallback Failure for TestHystrixCommand"));
             } else if (executionResult == AbstractTestHystrixCommand.ExecutionResult.RECOVERABLE_ERROR) {
@@ -5534,6 +5635,20 @@ protected Boolean run() throws Exception {
 
     }
 
+    private static class CommandWithNotWrappedByHystrixException extends TestHystrixCommand<Boolean> {
+
+        public CommandWithNotWrappedByHystrixException(TestCircuitBreaker circuitBreaker) {
+            super(testPropsBuilder()
+                    .setCircuitBreaker(circuitBreaker).setMetrics(circuitBreaker.metrics));
+        }
+
+        @Override
+        protected Boolean run() throws Exception {
+            throw new NotWrappedByHystrixTestException();
+        }
+
+    }
+
     private static class InterruptibleCommand extends TestHystrixCommand<Boolean> {
 
         public InterruptibleCommand(TestCircuitBreaker circuitBreaker, boolean shouldInterrupt, boolean shouldInterruptOnCancel, int timeoutInMillis) {

diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/HystrixObservableCommandTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/HystrixObservableCommandTest.java
@@ -28,7 +28,6 @@
 import com.netflix.hystrix.strategy.concurrency.HystrixRequestContext;
 import com.netflix.hystrix.strategy.properties.HystrixProperty;
 import org.junit.After;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import rx.Notification;
@@ -4679,6 +4678,9 @@ protected Observable<Integer> construct() {
             } else if (executionResult == AbstractTestHystrixCommand.ExecutionResult.HYSTRIX_FAILURE) {
                 addLatency(executionLatency);
                 throw new HystrixRuntimeException(HystrixRuntimeException.FailureType.COMMAND_EXCEPTION, AbstractFlexibleTestHystrixObservableCommand.class, "Execution Hystrix Failure for TestHystrixObservableCommand", new RuntimeException("Execution Failure for TestHystrixObservableCommand"), new RuntimeException("Fallback Failure for TestHystrixObservableCommand"));
+            } else if (executionResult == AbstractTestHystrixCommand.ExecutionResult.NOT_WRAPPED_FAILURE) {
+                addLatency(executionLatency);
+                throw new NotWrappedByHystrixTestRuntimeException();
             } else if (executionResult == AbstractTestHystrixCommand.ExecutionResult.RECOVERABLE_ERROR) {
                 addLatency(executionLatency);
                 throw new java.lang.Error("Execution Sync Error for TestHystrixObservableCommand");

diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/NotWrappedByHystrixTestException.java b/hystrix-core/src/test/java/com/netflix/hystrix/NotWrappedByHystrixTestException.java
@@ -0,0 +1,8 @@
+package com.netflix.hystrix;
+
+import com.netflix.hystrix.exception.ExceptionNotWrappedByHystrix;
+
+public class NotWrappedByHystrixTestException extends Exception implements ExceptionNotWrappedByHystrix {
+    private static final long serialVersionUID = 1L;
+
+}
diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/NotWrappedByHystrixTestRuntimeException.java b/hystrix-core/src/test/java/com/netflix/hystrix/NotWrappedByHystrixTestRuntimeException.java
@@ -0,0 +1,8 @@
+package com.netflix.hystrix;
+
+import com.netflix.hystrix.exception.ExceptionNotWrappedByHystrix;
+
+public class NotWrappedByHystrixTestRuntimeException extends RuntimeException implements ExceptionNotWrappedByHystrix {
+    private static final long serialVersionUID = 1L;
+
+}
diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/util/ExceptionsTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/util/ExceptionsTest.java
@@ -0,0 +1,23 @@
+package com.netflix.hystrix.util;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+public class ExceptionsTest {
+
+    @Test
+    public void testCastOfException(){
+        Exception exception = new IOException("simulated checked exception message");
+        try{
+            Exceptions.sneakyThrow(exception);
+            fail();
+        } catch(Exception e){
+            assertTrue(e instanceof  IOException);
+        }
+    }
+}