AbstractMethodMessageHandler processes Error as MessageHandlingException

Issue: SPR-16912

spring-messaging/src/main/java/org/springframework/messaging/MessageHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -28,6 +28,7 @@ public interface MessageHandler {
 	/**
 	 * Handle the given message.
 	 * @param message the message to be handled
+	 * @throws MessagingException if the handler failed to process the message
 	 */
 	void handleMessage(Message<?> message) throws MessagingException;
 

spring-messaging/src/main/java/org/springframework/messaging/handler/invocation/AbstractMethodMessageHandler.java

@@ -37,6 +37,7 @@ import org.springframework.core.MethodIntrospector;
 import org.springframework.core.MethodParameter;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.MessageHandler;
+import org.springframework.messaging.MessageHandlingException;
 import org.springframework.messaging.MessagingException;
 import org.springframework.messaging.handler.DestinationPatternsMessageCondition;
 import org.springframework.messaging.handler.HandlerMethod;
@@ -83,7 +84,7 @@ public abstract class AbstractMethodMessageHandler<T>
 
 	protected final Log logger = LogFactory.getLog(getClass());
 
-	private Collection<String> destinationPrefixes = new ArrayList<String>();
+	private final List<String> destinationPrefixes = new ArrayList<String>();
 
 	private final List<HandlerMethodArgumentResolver> customArgumentResolvers =
 			new ArrayList<HandlerMethodArgumentResolver>(4);
@@ -442,9 +443,9 @@ public abstract class AbstractMethodMessageHandler<T>
 			handleNoMatch(this.handlerMethods.keySet(), lookupDestination, message);
 			return;
 		}
+
 		Comparator<Match> comparator = new MatchComparator(getMappingComparator(message));
 		Collections.sort(matches, comparator);
-
 		if (logger.isTraceEnabled()) {
 			logger.trace("Found " + matches.size() + " handler methods: " + matches);
 		}
@@ -520,16 +521,16 @@ public abstract class AbstractMethodMessageHandler<T>
 			processHandlerMethodException(handlerMethod, ex, message);
 		}
 		catch (Throwable ex) {
-			if (logger.isErrorEnabled()) {
-				logger.error("Error while processing message " + message, ex);
-			}
+			Exception handlingException =
+					new MessageHandlingException(message, "Unexpected handler method invocation error", ex);
+			processHandlerMethodException(handlerMethod, handlingException, message);
 		}
 	}
 
-	protected void processHandlerMethodException(HandlerMethod handlerMethod, Exception ex, Message<?> message) {
-		InvocableHandlerMethod invocable = getExceptionHandlerMethod(handlerMethod, ex);
+	protected void processHandlerMethodException(HandlerMethod handlerMethod, Exception exception, Message<?> message) {
+		InvocableHandlerMethod invocable = getExceptionHandlerMethod(handlerMethod, exception);
 		if (invocable == null) {
-			logger.error("Unhandled exception from message handler method", ex);
+			logger.error("Unhandled exception from message handler method", exception);
 			return;
 		}
 		invocable.setMessageMethodArgumentResolvers(this.argumentResolvers);
@@ -537,7 +538,10 @@ public abstract class AbstractMethodMessageHandler<T>
 			logger.debug("Invoking " + invocable.getShortLogMessage());
 		}
 		try {
-			Object returnValue = invocable.invoke(message, ex, handlerMethod);
+			Throwable cause = exception.getCause();
+			Object returnValue = (cause != null ?
+					invocable.invoke(message, exception, cause, handlerMethod) :
+					invocable.invoke(message, exception, handlerMethod));
 			MethodParameter returnType = invocable.getReturnType();
 			if (void.class == returnType.getParameterType()) {
 				return;

spring-messaging/src/test/java/org/springframework/messaging/handler/annotation/support/AnnotationExceptionHandlerMethodResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -37,56 +37,59 @@ import static org.junit.Assert.*;
  */
 public class AnnotationExceptionHandlerMethodResolverTests {
 
+	private final AnnotationExceptionHandlerMethodResolver resolver =
+			new AnnotationExceptionHandlerMethodResolver(ExceptionController.class);
+
+
 	@Test
 	public void resolveMethodFromAnnotation() {
-		AnnotationExceptionHandlerMethodResolver resolver = new AnnotationExceptionHandlerMethodResolver(ExceptionController.class);
 		IOException exception = new IOException();
-		assertEquals("handleIOException", resolver.resolveMethod(exception).getName());
+		assertEquals("handleIOException", this.resolver.resolveMethod(exception).getName());
 	}
 
 	@Test
 	public void resolveMethodFromArgument() {
-		AnnotationExceptionHandlerMethodResolver resolver = new AnnotationExceptionHandlerMethodResolver(ExceptionController.class);
 		IllegalArgumentException exception = new IllegalArgumentException();
-		assertEquals("handleIllegalArgumentException", resolver.resolveMethod(exception).getName());
+		assertEquals("handleIllegalArgumentException", this.resolver.resolveMethod(exception).getName());
 	}
 
 	@Test
 	public void resolveMethodFromArgumentWithErrorType() {
-		AnnotationExceptionHandlerMethodResolver resolver = new AnnotationExceptionHandlerMethodResolver(ExceptionController.class);
 		AssertionError exception = new AssertionError();
-		assertEquals("handleAssertionError", resolver.resolveMethod(new IllegalStateException(exception)).getName());
+		assertEquals("handleAssertionError", this.resolver.resolveMethod(new IllegalStateException(exception)).getName());
 	}
 
 	@Test
 	public void resolveMethodExceptionSubType() {
-		AnnotationExceptionHandlerMethodResolver resolver = new AnnotationExceptionHandlerMethodResolver(ExceptionController.class);
 		IOException ioException = new FileNotFoundException();
-		assertEquals("handleIOException", resolver.resolveMethod(ioException).getName());
+		assertEquals("handleIOException", this.resolver.resolveMethod(ioException).getName());
 		SocketException bindException = new BindException();
-		assertEquals("handleSocketException", resolver.resolveMethod(bindException).getName());
+		assertEquals("handleSocketException", this.resolver.resolveMethod(bindException).getName());
 	}
 
 	@Test
 	public void resolveMethodBestMatch() {
-		AnnotationExceptionHandlerMethodResolver resolver = new AnnotationExceptionHandlerMethodResolver(ExceptionController.class);
 		SocketException exception = new SocketException();
-		assertEquals("handleSocketException", resolver.resolveMethod(exception).getName());
+		assertEquals("handleSocketException", this.resolver.resolveMethod(exception).getName());
 	}
 
 	@Test
 	public void resolveMethodNoMatch() {
-		AnnotationExceptionHandlerMethodResolver resolver = new AnnotationExceptionHandlerMethodResolver(ExceptionController.class);
 		Exception exception = new Exception();
-		assertNull("1st lookup", resolver.resolveMethod(exception));
-		assertNull("2nd lookup from cache", resolver.resolveMethod(exception));
+		assertNull("1st lookup", this.resolver.resolveMethod(exception));
+		assertNull("2nd lookup from cache", this.resolver.resolveMethod(exception));
 	}
 
 	@Test
 	public void resolveMethodInherited() {
-		AnnotationExceptionHandlerMethodResolver resolver = new AnnotationExceptionHandlerMethodResolver(InheritedController.class);
 		IOException exception = new IOException();
-		assertEquals("handleIOException", resolver.resolveMethod(exception).getName());
+		assertEquals("handleIOException", this.resolver.resolveMethod(exception).getName());
+	}
+
+	@Test
+	public void resolveMethodAgainstCause() {
+		IllegalStateException exception = new IllegalStateException(new IOException());
+		assertEquals("handleIOException", this.resolver.resolveMethod(exception).getName());
 	}
 
 	@Test(expected = IllegalStateException.class)
@@ -101,6 +104,7 @@ public class AnnotationExceptionHandlerMethodResolverTests {
 
 
 	@Controller
+	@SuppressWarnings("unused")
 	static class ExceptionController {
 
 		public void handle() {}

spring-messaging/src/test/java/org/springframework/messaging/simp/annotation/support/SimpAnnotationMethodMessageHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 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.
@@ -50,6 +50,7 @@ import org.springframework.messaging.simp.SimpAttributes;
 import org.springframework.messaging.simp.SimpAttributesContextHolder;
 import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
 import org.springframework.messaging.simp.SimpMessageSendingOperations;
+import org.springframework.messaging.simp.SimpMessageType;
 import org.springframework.messaging.simp.SimpMessagingTemplate;
 import org.springframework.messaging.simp.annotation.SubscribeMapping;
 import org.springframework.messaging.support.MessageBuilder;
@@ -73,7 +74,6 @@ import static org.mockito.BDDMockito.*;
  * @author Brian Clozel
  * @author Sebastien Deleuze
  */
-@SuppressWarnings("unused")
 public class SimpAnnotationMethodMessageHandlerTests {
 
 	private static final String TEST_INVALID_VALUE = "invalidValue";
@@ -158,7 +158,7 @@ public class SimpAnnotationMethodMessageHandlerTests {
 
 	@Test
 	public void subscribeEventDestinationVariableResolution() {
-		Message<?> message = createMessage("/pre/sub/bar/value");
+		Message<?> message = createMessage(SimpMessageType.SUBSCRIBE, "/pre/sub/bar/value", null);
 		this.messageHandler.registerHandler(this.testController);
 		this.messageHandler.handleMessage(message);
 
@@ -199,6 +199,30 @@ public class SimpAnnotationMethodMessageHandlerTests {
 		assertEquals("illegalState", handlerMethod.getMethod().getName());
 	}
 
+	@Test
+	public void exceptionAsCause() {
+		Message<?> message = createMessage("/pre/illegalStateCause");
+		this.messageHandler.registerHandler(this.testController);
+		this.messageHandler.handleMessage(message);
+
+		assertEquals("handleExceptionWithHandlerMethodArg", this.testController.method);
+		HandlerMethod handlerMethod = (HandlerMethod) this.testController.arguments.get("handlerMethod");
+		assertNotNull(handlerMethod);
+		assertEquals("illegalStateCause", handlerMethod.getMethod().getName());
+	}
+
+	@Test
+	public void errorAsMessageHandlingException() {
+		Message<?> message = createMessage("/pre/error");
+		this.messageHandler.registerHandler(this.testController);
+		this.messageHandler.handleMessage(message);
+
+		assertEquals("handleErrorWithHandlerMethodArg", this.testController.method);
+		HandlerMethod handlerMethod = (HandlerMethod) this.testController.arguments.get("handlerMethod");
+		assertNotNull(handlerMethod);
+		assertEquals("errorAsThrowable", handlerMethod.getMethod().getName());
+	}
+
 	@Test
 	public void simpScope() {
 		Map<String, Object> sessionAttributes = new ConcurrentHashMap<>();
@@ -327,7 +351,11 @@ public class SimpAnnotationMethodMessageHandlerTests {
 	}
 
 	private Message<?> createMessage(String destination, Map<String, Object> headers) {
-		SimpMessageHeaderAccessor accessor = SimpMessageHeaderAccessor.create();
+		return createMessage(SimpMessageType.MESSAGE, destination, headers);
+	}
+
+	private Message<?> createMessage(SimpMessageType messageType, String destination, Map<String, Object> headers) {
+		SimpMessageHeaderAccessor accessor = SimpMessageHeaderAccessor.create(messageType);
 		accessor.setSessionId("session1");
 		accessor.setSessionAttributes(new HashMap<>());
 		accessor.setDestination(destination);
@@ -406,7 +434,17 @@ public class SimpAnnotationMethodMessageHandlerTests {
 
 		@MessageMapping("/illegalState")
 		public void illegalState() {
-			throw new IllegalStateException();
+			throw new IllegalStateException("my cause");
+		}
+
+		@MessageMapping("/illegalStateCause")
+		public void illegalStateCause() {
+			throw new RuntimeException(new IllegalStateException("my cause"));
+		}
+
+		@MessageMapping("/error")
+		public void errorAsThrowable() {
+			throw new Error("my cause");
 		}
 
 		@MessageExceptionHandler(MethodArgumentNotValidException.class)
@@ -414,10 +452,18 @@ public class SimpAnnotationMethodMessageHandlerTests {
 			this.method = "handleValidationException";
 		}
 
-		@MessageExceptionHandler(IllegalStateException.class)
-		public void handleExceptionWithHandlerMethodArg(HandlerMethod handlerMethod) {
+		@MessageExceptionHandler
+		public void handleExceptionWithHandlerMethodArg(IllegalStateException ex, HandlerMethod handlerMethod) {
 			this.method = "handleExceptionWithHandlerMethodArg";
 			this.arguments.put("handlerMethod", handlerMethod);
+			assertEquals("my cause", ex.getMessage());
+		}
+
+		@MessageExceptionHandler
+		public void handleErrorWithHandlerMethodArg(Error ex, HandlerMethod handlerMethod) {
+			this.method = "handleErrorWithHandlerMethodArg";
+			this.arguments.put("handlerMethod", handlerMethod);
+			assertEquals("my cause", ex.getMessage());
 		}
 
 		@MessageMapping("/scope")
@@ -440,7 +486,6 @@ public class SimpAnnotationMethodMessageHandlerTests {
 
 		private String method;
 
-
 		@MessageMapping("foo")
 		public void handleFoo() {
 			this.method = "handleFoo";