Spring/spring-integration
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

INT-2894 Do Not Leak ThrowableHolderException

Browse Source 
The AbstractRequestHandlerAdvice uses an internal wrapper for
Throwable, in a similar fashion to the TransactionInterceptor.

Exceptions are unwrapped properly when exiting the invoke method so
callers are unaware of the wrapping.

The technique is used to avoid user errors (for example catching OOM
Errors and not propagating). User code (subclasses) only catch
Exceptions.

However, in the case of the ExpressionEvaluatingRequestHandlerAdvice,
the exception is included in any ErrorMessage sent to a failureChannel.

User code should not have to navigate these internal exceptions within
the cause tree.

Add a method to the abstract class unwrapExceptionIfNecessary, which
will unwrap the root cause exception if possible.

Add a method to the abstract class unwrapThrowableIfNecessary, which
will unwrap the root cause Throwable if possible.

Update tests to reduce the cause() traversals to reflect the
ThrowableHolderExceptions are no longer in the tree.

Add a test to verify a Throwable (such as an OOM) is properly
propagated.

Add a test to verify that when the root cause is a Throwable (and not
an Exception), the ErrorMessage sent to the failureChannel does not include
the ThrowableHolderException.

qualified inner class @links (when merging)
This commit is contained in:
Gary Russell
2013-01-23 14:54:39 -05:00
committed by Mark Fisher
parent cc6f07966f
commit d928105ef1
5 changed files with 142 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

34
spring-integration-core/src/main/java/org/springframework/integration/handler/advice/AbstractRequestHandlerAdvice.java
View File

@@ -111,8 +111,38 @@ public abstract class AbstractRequestHandlerAdvice extends IntegrationObjectSupp
protected abstract Object doInvoke(ExecutionCallback callback, Object target, Message<?> message) throws Exception;
/**
* Called by subclasses in doInvoke() to proceed() the invocation.
*
* Unwrap the cause of a {@link AbstractRequestHandlerAdvice.ThrowableHolderException}.
* @param e The exception.
* @return The cause, or e, if not a {@link AbstractRequestHandlerAdvice.ThrowableHolderException}
*/
protected Exception unwrapExceptionIfNecessary(Exception e) {
Exception actualException = e;
if (e instanceof ThrowableHolderException) {
if (e.getCause() instanceof Exception) {
actualException = (Exception) e.getCause();
}
}
return actualException;
}
/**
* Unwrap the cause of a {@link AbstractRequestHandlerAdvice.ThrowableHolderException}.
* @param e The exception.
* @return The cause, or e, if not a {@link AbstractRequestHandlerAdvice.ThrowableHolderException}
*/
protected Throwable unwrapThrowableIfNecessary(Exception e) {
Throwable actualThrowable = e;
if (e instanceof ThrowableHolderException) {
actualThrowable = e.getCause();
}
return actualThrowable;
}
/**
* Called by subclasses in doInvoke() to proceed() the invocation. Callers
* unwrap {@link AbstractRequestHandlerAdvice.ThrowableHolderException}s and use
* the cause for evaluation and re-throwing purposes.
* See {@link AbstractRequestHandlerAdvice#unwrapExceptionIfNecessary(Exception)}.
*/
protected interface ExecutionCallback {

7
spring-integration-core/src/main/java/org/springframework/integration/handler/advice/ExpressionEvaluatingRequestHandlerAdvice.java
View File

@@ -120,14 +120,15 @@ public class ExpressionEvaluatingRequestHandlerAdvice extends AbstractRequestHan
return result;
}
catch (Exception e) {
Exception actualException = this.unwrapExceptionIfNecessary(e);
if (this.onFailureExpression != null) {
Object evalResult = this.evaluateFailureExpression(message, e);
Object evalResult = this.evaluateFailureExpression(message, actualException);
if (this.returnFailureExpressionResult) {
return evalResult;
}
}
if (!this.trapException) {
throw e;
throw actualException;
}
return null;
}
@@ -163,7 +164,7 @@ public class ExpressionEvaluatingRequestHandlerAdvice extends AbstractRequestHan
}
if (evalResult != null && this.failureChannel != null) {
MessagingException messagingException = new MessageHandlingExpressionEvaluatingAdviceException(message,
"Handler Failed", exception, evalResult);
"Handler Failed", this.unwrapThrowableIfNecessary(exception), evalResult);
ErrorMessage resultMessage = new ErrorMessage(messagingException);
this.messagingTemplate.send(this.failureChannel, resultMessage);
}

2
spring-integration-core/src/main/java/org/springframework/integration/handler/advice/RequestHandlerCircuitBreakerAdvice.java
View File

@@ -68,7 +68,7 @@ public class RequestHandlerCircuitBreakerAdvice extends AbstractRequestHandlerAd
catch (Exception e) {
metadata.getFailures().incrementAndGet();
metadata.setLastFailure(System.currentTimeMillis());
throw e;
throw this.unwrapExceptionIfNecessary(e);
}
}

3
spring-integration-core/src/main/java/org/springframework/integration/handler/advice/RequestHandlerRetryAdvice.java
View File

@@ -93,7 +93,8 @@ public class RequestHandlerRetryAdvice extends AbstractRequestHandlerAdvice
throw e;
}
catch (Exception e) {
throw new MessagingException(message, "Failed to invoke handler", e);
throw new MessagingException(message, "Failed to invoke handler",
unwrapExceptionIfNecessary(e));
}
}
}, this.recoveryCallback, retryState);

113
spring-integration-core/src/test/java/org/springframework/integration/handler/advice/AdvisedMessageHandlerTests.java
View File

@@ -21,7 +21,11 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -31,6 +35,9 @@ import org.aopalliance.aop.Advice;
import org.aopalliance.intercept.MethodInterceptor;
import org.aopalliance.intercept.MethodInvocation;
import org.junit.Test;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.springframework.aop.framework.ProxyFactory;
import org.springframework.integration.Message;
import org.springframework.integration.MessageHandlingException;
import org.springframework.integration.MessagingException;
@@ -39,6 +46,7 @@ import org.springframework.integration.core.PollableChannel;
import org.springframework.integration.handler.AbstractReplyProducingMessageHandler;
import org.springframework.integration.handler.advice.ExpressionEvaluatingRequestHandlerAdvice.MessageHandlingExpressionEvaluatingAdviceException;
import org.springframework.integration.message.AdviceMessage;
import org.springframework.integration.message.ErrorMessage;
import org.springframework.integration.message.GenericMessage;
import org.springframework.retry.RecoveryCallback;
import org.springframework.retry.RetryContext;
@@ -82,7 +90,7 @@ public class AdvisedMessageHandlerTests {
advice.setSuccessChannel(successChannel);
advice.setFailureChannel(failureChannel);
advice.setOnSuccessExpression("'foo'");
advice.setOnFailureExpression("'bar:' + #exception.cause.message");
advice.setOnFailureExpression("'bar:' + #exception.message");
List<Advice> adviceChain = new ArrayList<Advice>();
adviceChain.add(advice);
@@ -243,7 +251,7 @@ public class AdvisedMessageHandlerTests {
assertNotNull(failure);
assertEquals("Hello, world!", ((MessagingException) failure.getPayload()).getFailedMessage().getPayload());
assertEquals(MessageHandlingExpressionEvaluatingAdviceException.class, failure.getPayload().getClass());
assertEquals("qux", ((Exception) failure.getPayload()).getCause().getCause().getMessage());
assertEquals("qux", ((Exception) failure.getPayload()).getCause().getMessage());
// propagate failing advice with failure; expect original exception
advice.setPropagateEvaluationFailures(true);
@@ -261,7 +269,7 @@ public class AdvisedMessageHandlerTests {
assertNotNull(failure);
assertEquals("Hello, world!", ((MessagingException) failure.getPayload()).getFailedMessage().getPayload());
assertEquals(MessageHandlingExpressionEvaluatingAdviceException.class, failure.getPayload().getClass());
assertEquals("qux", ((Exception) failure.getPayload()).getCause().getCause().getMessage());
assertEquals("qux", ((Exception) failure.getPayload()).getCause().getMessage());
}
@@ -536,7 +544,7 @@ public class AdvisedMessageHandlerTests {
handler.handleMessage(message);
Message<?> error = errors.receive(1000);
assertNotNull(error);
assertEquals("fooException", ((Exception) error.getPayload()).getCause().getCause().getMessage());
assertEquals("fooException", ((Exception) error.getPayload()).getCause().getMessage());
}
@@ -607,8 +615,8 @@ public class AdvisedMessageHandlerTests {
}
catch (Exception e) {
Throwable cause = e.getCause();
assertEquals("ThrowableHolderException", cause.getClass().getSimpleName());
assertEquals("intentional", cause.getCause().getMessage());
assertEquals(RuntimeException.class, cause.getClass());
assertEquals("intentional", cause.getMessage());
}
assertTrue(counter.get() == 0);
@@ -631,8 +639,8 @@ public class AdvisedMessageHandlerTests {
List<Advice> adviceChain = new ArrayList<Advice>();
ExpressionEvaluatingRequestHandlerAdvice expressionAdvice = new ExpressionEvaluatingRequestHandlerAdvice();
// ThrowableHolderException / MessagingException / ThrowableHolderException / RuntimeException
expressionAdvice.setOnFailureExpression("#exception.cause.cause.cause.message");
// MessagingException / RuntimeException
expressionAdvice.setOnFailureExpression("#exception.cause.message");
expressionAdvice.setReturnFailureExpressionResult(true);
final AtomicInteger outerCounter = new AtomicInteger();
adviceChain.add(new AbstractRequestHandlerAdvice() {
@@ -678,7 +686,7 @@ public class AdvisedMessageHandlerTests {
List<Advice> adviceChain = new ArrayList<Advice>();
ExpressionEvaluatingRequestHandlerAdvice expressionAdvice = new ExpressionEvaluatingRequestHandlerAdvice();
expressionAdvice.setOnFailureExpression("#exception.cause.message");
expressionAdvice.setOnFailureExpression("#exception.message");
expressionAdvice.setFailureChannel(errors);
adviceChain.add(new RequestHandlerRetryAdvice());
@@ -696,7 +704,7 @@ public class AdvisedMessageHandlerTests {
handler.handleMessage(new GenericMessage<String>("test"));
}
catch (Exception e) {
assertEquals("intentional: 3", e.getCause().getCause().getMessage());
assertEquals("intentional: 3", e.getCause().getMessage());
}
for (int i = 1; i <= 3; i++) {
@@ -709,5 +717,90 @@ public class AdvisedMessageHandlerTests {
}
/**
* Verify that Errors such as OOM are properly propagated.
*/
@Test
public void throwableProperlyPropagated() throws Exception {
AbstractRequestHandlerAdvice advice = new AbstractRequestHandlerAdvice() {
@Override
protected Object doInvoke(ExecutionCallback callback, Object target, Message<?> message) throws Exception {
Object result;
try {
result = callback.execute();
}
catch (Exception e) {
// should not be unwrapped because the cause is a Throwable
throw this.unwrapExceptionIfNecessary(e);
}
return result;
}
};
final Throwable theThrowable = new Throwable("foo");
MethodInvocation methodInvocation = mock(MethodInvocation.class);
Method method = AbstractReplyProducingMessageHandler.class.getDeclaredMethod("handleRequestMessage", Message.class);
when(methodInvocation.getMethod()).thenReturn(method);
when(methodInvocation.getArguments()).thenReturn(new Object[] {new GenericMessage<String>("foo")});
try {
doAnswer(new Answer<Object>() {
public Object answer(InvocationOnMock invocation) throws Throwable {
throw theThrowable;
}
}).when(methodInvocation).proceed();
advice.invoke(methodInvocation);
fail("Expected throwable");
}
catch (Throwable t) {
assertSame(theThrowable, t);
}
}
/**
* Verify that Errors such as OOM are properly propagated and we suppress the
* ThrowableHolderException from the output message.
*/
@Test
public void throwableProperlyPropagatedAndReported() throws Exception {
QueueChannel errors = new QueueChannel();
ExpressionEvaluatingRequestHandlerAdvice expressionAdvice = new ExpressionEvaluatingRequestHandlerAdvice();
expressionAdvice.setOnFailureExpression("'foo'");
expressionAdvice.setFailureChannel(errors);
Throwable theThrowable = new Throwable("foo");
ProxyFactory proxyFactory = new ProxyFactory(new Foo(theThrowable));
proxyFactory.addAdvice(expressionAdvice);
Bar fooHandler = (Bar) proxyFactory.getProxy();
try {
fooHandler.handleRequestMessage(new GenericMessage<String>("foo"));
fail("Expected throwable");
}
catch (Throwable t) {
assertSame(theThrowable, t);
ErrorMessage error = (ErrorMessage) errors.receive(1000);
assertNotNull(error);
assertSame(theThrowable, error.getPayload().getCause());
}
}
private interface Bar {
Object handleRequestMessage(Message<?> message) throws Throwable;
}
private class Foo implements Bar {
public final Throwable throwable;
public Foo(Throwable throwable) {
this.throwable = throwable;
}
public Object handleRequestMessage(Message<?> message) throws Throwable {
throw this.throwable;
}
}
}