INT-2943: Retry: Don't Wrap in MessagingException

Previously, `RequestHandlerRetryAdvice` wrapped a Handler's (business) Exceptions in a
`MessagingException` on each retry within `RetryCallback#doWithRetry`
and before `RetryPolicy#canRetry`.
It made useless some retry framework out-of-the-box features like `BinaryExceptionClassifier`

* Push wrapping to `MessagingException` after `retryTemplate#execute`
* Polishing `ErrorMessageSendingRecoverer` regarding new logic
* Add `retryableExceptions` test

You can now properly specify Business exceptions with the retry policy to
perform conditional retry.

JIRA: https://jira.springsource.org/browse/INT-2943

Polishing

Only wrap Throwable in ThrowableHolderException if the Throwable
is not an Exception.

INT-2943: Polishing ErrorMessageSendingRecoverer

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

@@ -71,6 +71,9 @@ public abstract class AbstractRequestHandlerAdvice extends IntegrationObjectSupp
 						try {
 							return invocation.proceed();
 						}
+						catch (Exception e) {
+							throw e;
+						}
 						catch (Throwable e) {
 							throw new ThrowableHolderException(e);
 						}
@@ -91,6 +94,9 @@ public abstract class AbstractRequestHandlerAdvice extends IntegrationObjectSupp
 												" so please raise an issue if you see this exception");
 							}
 						}
+						catch (Exception e) {
+							throw e;
+						}
 						catch (Throwable e) {
 							throw new ThrowableHolderException(e);
 						}
@@ -98,12 +104,7 @@ public abstract class AbstractRequestHandlerAdvice extends IntegrationObjectSupp
 				}, invocationThis, message);
 			}
 			catch (Exception e) {
-				if (e instanceof ThrowableHolderException) {
-					throw e.getCause();
-				}
-				else {
-					throw e;
-				}
+				throw this.unwrapThrowableIfNecessary(e);
 			}
 		}
 	}

spring-integration-core/src/main/java/org/springframework/integration/handler/advice/ErrorMessageSendingRecoverer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2013 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@ package org.springframework.integration.handler.advice;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.integration.Message;
 import org.springframework.integration.MessageChannel;
 import org.springframework.integration.MessagingException;
@@ -29,7 +30,9 @@ import org.springframework.util.Assert;
 /**
  * RecoveryCallback that sends the final throwable as an ErrorMessage after
  * retry exhaustion.
+ *
  * @author Gary Russell
+ * @author Artem Bilan
  * @since 2.2
  *
  */
@@ -57,11 +60,11 @@ public class ErrorMessageSendingRecoverer implements RecoveryCallback<Object> {
 					"this can occur, for example, if the RetryPolicy allowed zero attempts to execute the handler; " +
 					"RetryContext: " + context.toString());
 		}
+		else if (!(lastThrowable instanceof MessagingException)) {
+			lastThrowable = new MessagingException((Message<?>) context.getAttribute("message"), lastThrowable);
+		}
 		if (logger.isDebugEnabled()) {
-			String supplement = "";
-			if (lastThrowable instanceof MessagingException) {
-				supplement = ":failedMessage:" + ((MessagingException) lastThrowable).getFailedMessage();
-			}
+			String supplement = ":failedMessage:" + ((MessagingException) lastThrowable).getFailedMessage();
 			logger.debug("Sending ErrorMessage " + supplement, lastThrowable);
 		}
 		messagingTemplate.send(new ErrorMessage(lastThrowable));

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

@@ -83,22 +83,20 @@ public class RequestHandlerRetryAdvice extends AbstractRequestHandlerAdvice
 		try {
 			return retryTemplate.execute(new RetryCallback<Object>() {
 				public Object doWithRetry(RetryContext context) throws Exception {
-					try {
-						return callback.cloneAndExecute();
-					}
-					catch (MessagingException e) {
-						if (e.getFailedMessage() == null) {
-							e.setFailedMessage(message);
-						}
-						throw e;
-					}
-					catch (Exception e) {
-						throw new MessagingException(message, "Failed to invoke handler",
-								unwrapExceptionIfNecessary(e));
-					}
+					return callback.cloneAndExecute();
 				}
 			}, this.recoveryCallback, retryState);
 		}
+		catch (MessagingException e) {
+			if (e.getFailedMessage() == null) {
+				e.setFailedMessage(message);
+			}
+			throw e;
+		}
+		catch (Exception e) {
+			throw new MessagingException(message, "Failed to invoke handler",
+					unwrapExceptionIfNecessary(e));
+		}
 		finally {
 			messageHolder.remove();
 		}

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

@@ -20,6 +20,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.doAnswer;
@@ -30,7 +31,9 @@ import static org.mockito.Mockito.when;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.Executors;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -41,6 +44,7 @@ import org.aopalliance.aop.Advice;
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
 import org.apache.commons.logging.Log;
+import org.hamcrest.Matchers;
 import org.junit.Test;
 import org.mockito.Mockito;
 import org.mockito.invocation.InvocationOnMock;
@@ -931,6 +935,66 @@ public class AdvisedMessageHandlerTests {
 		assertNotNull(discardChannel.receive(0));
 	}
 
+	@Test
+	public void testInt2943RetryWithExceptionClassifierFalse() {
+		testInt2943RetryWithExceptionClassifier(false, 1);
+	}
+
+	@Test
+	public void testInt2943RetryWithExceptionClassifierTrue() {
+		testInt2943RetryWithExceptionClassifier(true, 3);
+	}
+
+	private void testInt2943RetryWithExceptionClassifier(boolean retryForMyException, int expected) {
+		final AtomicInteger counter = new AtomicInteger(0);
+
+		@SuppressWarnings("serial")
+		class MyException extends RuntimeException {
+
+		}
+
+		AbstractReplyProducingMessageHandler handler = new AbstractReplyProducingMessageHandler() {
+
+			@Override
+			protected Object handleRequestMessage(Message<?> requestMessage) {
+				counter.incrementAndGet();
+				throw new MyException();
+
+			}
+		};
+		QueueChannel replies = new QueueChannel();
+		handler.setOutputChannel(replies);
+		RequestHandlerRetryAdvice advice = new RequestHandlerRetryAdvice();
+
+		RetryTemplate retryTemplate = new RetryTemplate();
+
+		Map<Class<? extends Throwable>, Boolean> retryableExceptions = new HashMap<Class<? extends Throwable>, Boolean>();
+		retryableExceptions.put(MyException.class, retryForMyException);
+		retryableExceptions.put(MessagingException.class, true);
+
+		retryTemplate.setRetryPolicy(new SimpleRetryPolicy(3, retryableExceptions));
+
+		advice.setRetryTemplate(retryTemplate);
+
+		List<Advice> adviceChain = new ArrayList<Advice>();
+		adviceChain.add(advice);
+		handler.setAdviceChain(adviceChain);
+		handler.afterPropertiesSet();
+
+		Message<String> message = new GenericMessage<String>("Hello, world!");
+		try {
+			handler.handleMessage(message);
+			fail("MessagingException expected.");
+		}
+		catch (Exception e) {
+			assertThat(e, Matchers.instanceOf(MessagingException.class));
+			assertThat(e.getCause(), Matchers.instanceOf(MyException.class));
+		}
+
+		assertEquals(expected, counter.get());
+
+	}
+
 	private interface Bar {
 		Object handleRequestMessage(Message<?> message) throws Throwable;
 	}