Make ExceptionHandler invokable when matching deeply nested cause

Closes spring-projectsgh-26317

spring-web/src/main/java/org/springframework/web/method/HandlerMethod.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -58,6 +58,7 @@
  * @author Rossen Stoyanchev
  * @author Juergen Hoeller
  * @author Sam Brannen
+ * @author Rodolphe Lecocq
  * @since 3.1
  */
 public class HandlerMethod {
@@ -412,10 +413,34 @@ public String toString() {
 	@Nullable
 	protected static Object findProvidedArgument(MethodParameter parameter, @Nullable Object... providedArgs) {
 		if (!ObjectUtils.isEmpty(providedArgs)) {
+			boolean parameterIsThrowable = Throwable.class.isAssignableFrom(parameter.getParameterType());
 			for (Object providedArg : providedArgs) {
 				if (parameter.getParameterType().isInstance(providedArg)) {
 					return providedArg;
 				}
+				else if (parameterIsThrowable && providedArg instanceof Throwable) {
+					return findProvidedThrowableArgument(
+							parameter.getParameterType().asSubclass(Throwable.class),
+							((Throwable) providedArg).getCause());
+				}
+			}
+		}
+		return null;
+	}
+
+	/**
+	 * @since 5.3.3
+	 */
+	@Nullable
+	protected static Object findProvidedThrowableArgument(Class<? extends Throwable> parameterType,
+			@Nullable Throwable providedArg) {
+
+		if (providedArg != null) {
+			if (parameterType.isInstance(providedArg)) {
+				return providedArg;
+			}
+			else if (providedArg.getCause() != null) {
+				return findProvidedThrowableArgument(parameterType, providedArg.getCause());
 			}
 		}
 		return null;

spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -16,6 +16,7 @@
 
 package org.springframework.web.method.support;
 
+import java.io.IOException;
 import java.lang.reflect.Method;
 
 import org.junit.jupiter.api.BeforeEach;
@@ -38,6 +39,7 @@
  * Unit tests for {@link InvocableHandlerMethod}.
  *
  * @author Rossen Stoyanchev
+ * @author Rodolphe Lecocq
  */
 public class InvocableHandlerMethodTests {
 
@@ -156,6 +158,51 @@ public void invocationErrorMessage() throws Exception {
 			.withMessageContaining("Illegal argument");
 	}
 
+	@Test // gh-26317
+	public void resolveMatchedExceptionArguments() throws Exception {
+		//this.composite.addResolver(new StubArgumentResolver(IOException.class));
+
+		IOException matchedThrowable = new IOException("io-error");
+
+		// handler method should receive the matched exception as argument when it is the main exception
+		Object value = getInvocable(IOException.class).invokeForRequest(request, null,
+				matchedThrowable);
+		assertThat(value).isEqualTo("io-error");
+
+		// handler method should receive the matched exception as argument when it is the 1st cause
+		value = getInvocable(IOException.class).invokeForRequest(request, null,
+				new Exception(matchedThrowable));
+		assertThat(value).isEqualTo("io-error");
+
+		// handler method should receive the matched exception as argument when it is a cause with level > 1
+		value = getInvocable(IOException.class).invokeForRequest(request, null,
+				new Exception(new Exception(new Exception(matchedThrowable))));
+		assertThat(value).isEqualTo("io-error");
+	}
+
+	@Test // gh-26317
+	public void resolveMatchedExceptionAndMainExceptionArguments() throws Exception {
+		//this.composite.addResolver(new StubArgumentResolver(IOException.class));
+		//this.composite.addResolver(new StubArgumentResolver(IOException.class));
+
+		IOException matchedThrowable = new IOException("io-error");
+
+		// handler method should receive the main exception for both arguments
+		Object value = getInvocable(Exception.class, IOException.class).invokeForRequest(request, null,
+				matchedThrowable);
+		assertThat(value).isEqualTo("io-error-io-error");
+
+		// handler method should receive the main exception and the matched exception
+		value = getInvocable(Exception.class, IOException.class).invokeForRequest(request, null,
+				new Exception("main", matchedThrowable));
+		assertThat(value).isEqualTo("main-io-error");
+
+		// handler method should receive the main exception and the matched exception
+		value = getInvocable(Exception.class, IOException.class).invokeForRequest(request, null,
+				new Exception("main", new Exception("cause1", new Exception("cause2", matchedThrowable))));
+		assertThat(value).isEqualTo("main-io-error");
+	}
+
 	private InvocableHandlerMethod getInvocable(Class<?>... argTypes) {
 		Method method = ResolvableMethod.on(Handler.class).argTypes(argTypes).resolveMethod();
 		InvocableHandlerMethod handlerMethod = new InvocableHandlerMethod(new Handler(), method);
@@ -168,7 +215,6 @@ private StubArgumentResolver getStubResolver(int index) {
 	}
 
 
-
 	@SuppressWarnings("unused")
 	private static class Handler {
 
@@ -182,6 +228,14 @@ public void handle(double amount) {
 		public void handleWithException(Throwable ex) throws Throwable {
 			throw ex;
 		}
+
+		public String handleExceptionCause(IOException cause) throws Exception {
+			return cause.getMessage();
+		}
+
+		public String handleExceptionCauseAndMain(Exception mainEx, IOException cause) throws Exception {
+			return mainEx.getMessage() + "-" + cause.getMessage();
+		}
 	}
 
 

spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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,8 @@
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.commons.lang.exception.ExceptionUtils;
+
 import org.springframework.aop.support.AopUtils;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.context.ApplicationContext;
@@ -72,6 +74,7 @@
  * @author Rossen Stoyanchev
  * @author Juergen Hoeller
  * @author Sebastien Deleuze
+ * @author Rodolphe Lecocq
  * @since 3.1
  */
 public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExceptionResolver
@@ -414,20 +417,14 @@ protected ModelAndView doResolveHandlerMethodException(HttpServletRequest reques
 			if (logger.isDebugEnabled()) {
 				logger.debug("Using @ExceptionHandler " + exceptionHandlerMethod);
 			}
-			Throwable cause = exception.getCause();
-			if (cause != null) {
-				// Expose cause as provided argument as well
-				exceptionHandlerMethod.invokeAndHandle(webRequest, mavContainer, exception, cause, handlerMethod);
-			}
-			else {
-				// Otherwise, just the given exception as-is
-				exceptionHandlerMethod.invokeAndHandle(webRequest, mavContainer, exception, handlerMethod);
-			}
+			// Expose exception as provided argument: Throwable arguments will have
+			// their causes inspected as potential arguments for the method parameters
+			exceptionHandlerMethod.invokeAndHandle(webRequest, mavContainer, exception, handlerMethod);
 		}
 		catch (Throwable invocationEx) {
-			// Any other than the original exception (or its cause) is unintended here,
+			// Any other than the original exception (or one of its causes) is unintended here,
 			// probably an accident (e.g. failed assertion or the like).
-			if (invocationEx != exception && invocationEx != exception.getCause() && logger.isWarnEnabled()) {
+			if (!ExceptionUtils.getThrowableList(exception).contains(invocationEx) && logger.isWarnEnabled()) {
 				logger.warn("Failure in @ExceptionHandler " + exceptionHandlerMethod, invocationEx);
 			}
 			// Continue with default processing of the original exception...

spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -66,6 +66,7 @@
  * @author Arjen Poutsma
  * @author Kazuki Shimizu
  * @author Brian Clozel
+ * @author Rodolphe Lecocq
  * @since 3.1
  */
 @SuppressWarnings("unused")
@@ -189,6 +190,18 @@ void resolveExceptionResponseBody() throws UnsupportedEncodingException, NoSuchM
 		assertThat(this.response.getContentAsString()).isEqualTo("IllegalArgumentException");
 	}
 
+	@Test // gh-26317
+	void resolveExceptionResponseBodyMatchingCauseLevel2() throws UnsupportedEncodingException, NoSuchMethodException {
+		Exception ex = new Exception(new Exception(new IllegalArgumentException()));
+		HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle");
+		this.resolver.afterPropertiesSet();
+		ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex);
+
+		assertThat(mav).isNotNull();
+		assertThat(mav.isEmpty()).isTrue();
+		assertThat(this.response.getContentAsString()).isEqualTo("IllegalArgumentException");
+	}
+
 	@Test
 	void resolveExceptionResponseWriter() throws Exception {
 		IllegalArgumentException ex = new IllegalArgumentException();
@@ -257,6 +270,21 @@ void resolveExceptionGlobalHandlerOrdered() throws Exception {
 		assertThat(this.response.getContentAsString()).isEqualTo("TestExceptionResolver: IllegalStateException");
 	}
 
+	@Test // gh-26317
+	void resolveExceptionGlobalHandlerOrderedMatchingCauseLevel2() throws Exception {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
+		this.resolver.setApplicationContext(ctx);
+		this.resolver.afterPropertiesSet();
+
+		Exception ex = new Exception(new Exception(new IllegalStateException()));
+		HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle");
+		ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex);
+
+		assertThat(mav).as("Exception was not handled").isNotNull();
+		assertThat(mav.isEmpty()).isTrue();
+		assertThat(this.response.getContentAsString()).isEqualTo("TestExceptionResolver: IllegalStateException");
+	}
+
 	@Test  // SPR-12605
 	void resolveExceptionWithHandlerMethodArg() throws Exception {
 		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class);
@@ -294,14 +322,15 @@ void resolveExceptionWithAssertionErrorAsRootCause() throws Exception {
 		this.resolver.setApplicationContext(ctx);
 		this.resolver.afterPropertiesSet();
 
-		AssertionError err = new AssertionError("argh");
-		FatalBeanException ex = new FatalBeanException("wrapped", err);
+		AssertionError rootCause = new AssertionError("argh");
+		FatalBeanException cause = new FatalBeanException("wrapped", rootCause);
+		Exception ex = new Exception(cause); // gh-26317
 		HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle");
 		ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex);
 
 		assertThat(mav).as("Exception was not handled").isNotNull();
 		assertThat(mav.isEmpty()).isTrue();
-		assertThat(this.response.getContentAsString()).isEqualTo(err.toString());
+		assertThat(this.response.getContentAsString()).isEqualTo(rootCause.toString());
 	}
 
 	@Test
@@ -318,6 +347,21 @@ void resolveExceptionControllerAdviceHandler() throws Exception {
 		assertThat(mav.isEmpty()).isTrue();
 		assertThat(this.response.getContentAsString()).isEqualTo("BasePackageTestExceptionResolver: IllegalStateException");
 	}
+
+	@Test // gh-26317
+	void resolveExceptionControllerAdviceHandlerMatchingCauseLevel2() throws Exception {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class);
+		this.resolver.setApplicationContext(ctx);
+		this.resolver.afterPropertiesSet();
+
+		Exception ex = new Exception(new IllegalStateException());
+		HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle");
+		ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex);
+
+		assertThat(mav).as("Exception was not handled").isNotNull();
+		assertThat(mav.isEmpty()).isTrue();
+		assertThat(this.response.getContentAsString()).isEqualTo("BasePackageTestExceptionResolver: IllegalStateException");
+	}
 
 	@Test
 	void resolveExceptionControllerAdviceNoHandler() throws Exception {