From 9c357df18c67337469b475326f8ffe7df00314cc Mon Sep 17 00:00:00 2001 From: Tim van Heugten Date: Sun, 6 Nov 2016 23:13:26 +0100 Subject: [PATCH] Introduce NotWrappedByHystrix to mark Exceptions that should not be wrapped in HystrixRunetimeException --- .../com/netflix/hystrix/AbstractCommand.java | 27 +++- .../com/netflix/hystrix/HystrixCommand.java | 13 +- .../ExceptionNotWrappedByHystrix.java | 8 ++ .../com/netflix/hystrix/util/Exceptions.java | 20 +++ .../hystrix/AbstractTestHystrixCommand.java | 2 +- .../netflix/hystrix/HystrixCommandTest.java | 115 ++++++++++++++++++ .../hystrix/HystrixObservableCommandTest.java | 4 +- .../NotWrappedByHystrixTestException.java | 8 ++ ...tWrappedByHystrixTestRuntimeException.java | 8 ++ .../netflix/hystrix/util/ExceptionsTest.java | 23 ++++ 10 files changed, 214 insertions(+), 14 deletions(-) create mode 100644 hystrix-core/src/main/java/com/netflix/hystrix/exception/ExceptionNotWrappedByHystrix.java create mode 100644 hystrix-core/src/main/java/com/netflix/hystrix/util/Exceptions.java create mode 100644 hystrix-core/src/test/java/com/netflix/hystrix/NotWrappedByHystrixTestException.java create mode 100644 hystrix-core/src/test/java/com/netflix/hystrix/NotWrappedByHystrixTestRuntimeException.java create mode 100644 hystrix-core/src/test/java/com/netflix/hystrix/util/ExceptionsTest.java 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 f11ef7c4d..554c27116 100644 --- 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 getFallbackOrThrowException(final AbstractCommand _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 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 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. *

- * 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 index 4e03958bb..4b7ccbed6 100755 --- 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 new file mode 100644 index 000000000..e283a0163 --- /dev/null +++ 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 new file mode 100644 index 000000000..249856270 --- /dev/null +++ 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.doThrow(t); + } + + private static 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 index 2a2204681..a4eeface0 100644 --- 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 extends HystrixObservable, 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 index f533cc9f0..a59c3f622 100644 --- 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 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 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 t = new AtomicReference(); + final CountDownLatch latch = new CountDownLatch(1); + try { + command.observe().subscribe(new Observer() { + + @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 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 { + + 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 { 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 index 8b38bba4d..527e5c62d 100644 --- 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 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 new file mode 100644 index 000000000..69f8ed888 --- /dev/null +++ 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 new file mode 100644 index 000000000..13a4eb8e8 --- /dev/null +++ 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 new file mode 100644 index 000000000..6e891440b --- /dev/null +++ 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); + } + } +}