Skip to content

Commit

Permalink
Merge pull request #1414 from tbvh/master
Browse files Browse the repository at this point in the history
Allow exceptions without HystrixRuntimeException wrapper
  • Loading branch information
mattrjacobs authored Dec 19, 2016
2 parents e08676e + 9c357df commit 0dd3cf5
Show file tree
Hide file tree
Showing 10 changed files with 214 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -759,12 +760,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)) {
Expand Down Expand Up @@ -1048,6 +1052,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
Expand Down Expand Up @@ -1542,12 +1550,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;
}
Expand All @@ -1564,6 +1573,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -342,7 +343,7 @@ public R execute() {
try {
return queue().get();
} catch (Exception e) {
throw decomposeException(e);
throw Exceptions.sneakyThrow(decomposeException(e));
}
}

Expand Down Expand Up @@ -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:
Expand All @@ -450,7 +451,7 @@ public R get(long timeout, TimeUnit unit) throws InterruptedException, Execution
throw hre;
}
} else {
throw re;
throw Exceptions.sneakyThrow(t);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {

}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
115 changes: 115 additions & 0 deletions hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*/
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -4799,6 +4898,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) {
Expand Down Expand Up @@ -5583,6 +5684,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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

}
Original file line number Diff line number Diff line change
@@ -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;

}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}

0 comments on commit 0dd3cf5

Please sign in to comment.