@ExceptionHandler methods not invokable if matched on exception's cause level > 1 #26317
Description
Affects: 5.3.2
It looks like PR #23380 introduced a major regression in the way exceptions are handled by @ControllerAdvice components.
Before that PR was merged, Spring resolved @ExceptionHandler methods only on the main exception or its 1st level cause.
ExceptionHandlerExceptionResolver was aligned with that, and after resolution it would invoke the resolved method by using the exception and its cause as 'candidate' arguments for the target method:
protected ModelAndView doResolveHandlerMethodException(HttpServletRequest request,
HttpServletResponse response, @Nullable HandlerMethod handlerMethod, Exception exception) {
ServletInvocableHandlerMethod exceptionHandlerMethod = getExceptionHandlerMethod(handlerMethod, exception);
if (exceptionHandlerMethod == null) {
return null;
}
if (this.argumentResolvers != null) {
exceptionHandlerMethod.setHandlerMethodArgumentResolvers(this.argumentResolvers);
}
if (this.returnValueHandlers != null) {
exceptionHandlerMethod.setHandlerMethodReturnValueHandlers(this.returnValueHandlers);
}
ServletWebRequest webRequest = new ServletWebRequest(request, response);
ModelAndViewContainer mavContainer = new ModelAndViewContainer();
try {
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);
}
}
catch (Throwable invocationEx) {
// Any other than the original exception (or its cause) is unintended here,
// probably an accident (e.g. failed assertion or the like).
if (invocationEx != exception && invocationEx != exception.getCause() && logger.isWarnEnabled()) {
logger.warn("Failure in @ExceptionHandler " + exceptionHandlerMethod, invocationEx);
}
// Continue with default processing of the original exception...
return null;
}But PR #23380 didn't come with any change in the way arguments of the target method are resolved... and that's the problem.
Whenever the @ExceptionHandler method is resolved due to a match on a cause above the 1st level, and if a throwable of this type is used as an argument of the method, ExceptionHandlerExceptionResolver has no way to resolve this argument as it still only tries the main exception and its 1st cause.
So if we do this in a controller...
@GetMapping("/greeting")
public Greeting greeting(@RequestParam(value = "name", defaultValue = "World") String name) {
if ("err3".equals(name)) {
throw new RuntimeException(
new Exception(
new IllegalArgumentException("ExceptionHandler will be resolved but cannot be invoked")));
}
...
}... and have this kind of @ControllerAdvice :
@ControllerAdvice
public class MyControllerAdvice {
@ExceptionHandler(IllegalArgumentException.class)
public ResponseEntity<?> handleIllegalArgumentException(IllegalArgumentException ex) {
return ResponseEntity.badRequest().body(ex.toString());
}
}... then the handleIllegalArgumentException is resolved but actually not invokable when exception thrown is like in the code above, and we get this in the log:
2020-12-22 19:52:45.387 WARN 29244 --- [ main] .m.m.a.ExceptionHandlerExceptionResolver : Failure in @ExceptionHandler com.example.restservice.MyControllerAdvice#handleIllegalArgumentException(IllegalArgumentException)
java.lang.IllegalStateException: Could not resolve parameter [0] in public org.springframework.http.ResponseEntity<?> com.example.restservice.MyControllerAdvice.handleIllegalArgumentException(java.lang.IllegalArgumentException): No suitable resolver
at org.springframework.web.method.support.InvocableHandlerMethod.getMethodArgumentValues(InvocableHandlerMethod.java:167) ~[spring-web-5.3.2.jar:5.3.2]
at org.springframework.web.method.support.InvocableHandlerMethod.invokeForRequest(InvocableHandlerMethod.java:137) ~[spring-web-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.mvc.method.annotation.ServletInvocableHandlerMethod.invokeAndHandle(ServletInvocableHandlerMethod.java:106) ~[spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.mvc.method.annotation.ExceptionHandlerExceptionResolver.doResolveHandlerMethodException(ExceptionHandlerExceptionResolver.java:420) ~[spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.handler.AbstractHandlerMethodExceptionResolver.doResolveException(AbstractHandlerMethodExceptionResolver.java:75) [spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.handler.AbstractHandlerExceptionResolver.resolveException(AbstractHandlerExceptionResolver.java:141) [spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.handler.HandlerExceptionResolverComposite.resolveException(HandlerExceptionResolverComposite.java:80) [spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.DispatcherServlet.processHandlerException(DispatcherServlet.java:1321) [spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.test.web.servlet.TestDispatcherServlet.processHandlerException(TestDispatcherServlet.java:149) [spring-test-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.DispatcherServlet.processDispatchResult(DispatcherServlet.java:1132) [spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1078) [spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:961) [spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) [spring-webmvc-5.3.2.jar:5.3.2]
at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:898) [spring-webmvc-5.3.2.jar:5.3.2]
So if the changes made by PR #23380 are to be kept, i.e. @ExceptionHandler methods have to be able to handle any cause in the causality chain of an exception, a change of design is necessary in arguments resolution.
Regards