From f6cb30b165cb8bc9f09bc6b662500ddfe462f4dc Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 31 Mar 2016 11:52:36 +0200 Subject: [PATCH] @ExceptionHandler is able to process Error thrown from handler method Issue: SPR-11106 --- .../ExceptionHandlerMethodResolver.java | 15 ++++-- .../web/servlet/DispatcherServlet.java | 18 +++---- .../ExceptionHandlerExceptionResolver.java | 12 ++++- ...xceptionHandlerExceptionResolverTests.java | 30 ++++++++--- ...nnotationControllerHandlerMethodTests.java | 52 ++++++++++++++----- 5 files changed, 89 insertions(+), 38 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/method/annotation/ExceptionHandlerMethodResolver.java b/spring-web/src/main/java/org/springframework/web/method/annotation/ExceptionHandlerMethodResolver.java index 5d307cba90c..9531af84330 100644 --- a/spring-web/src/main/java/org/springframework/web/method/annotation/ExceptionHandlerMethodResolver.java +++ b/spring-web/src/main/java/org/springframework/web/method/annotation/ExceptionHandlerMethodResolver.java @@ -31,6 +31,7 @@ import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils.MethodFilter; import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.util.NestedServletException; /** * Discovers {@linkplain ExceptionHandler @ExceptionHandler} methods in a given class, @@ -98,8 +99,8 @@ public class ExceptionHandlerMethodResolver { } protected void detectAnnotationExceptionMappings(Method method, List> result) { - ExceptionHandler annot = AnnotationUtils.findAnnotation(method, ExceptionHandler.class); - result.addAll(Arrays.asList(annot.value())); + ExceptionHandler ann = AnnotationUtils.findAnnotation(method, ExceptionHandler.class); + result.addAll(Arrays.asList(ann.value())); } private void addExceptionMapping(Class exceptionType, Method method) { @@ -124,7 +125,11 @@ public class ExceptionHandlerMethodResolver { * @return a Method to handle the exception, or {@code null} if none found */ public Method resolveMethod(Exception exception) { - return resolveMethodByExceptionType(exception.getClass()); + Method method = resolveMethodByExceptionType(exception.getClass()); + if (method == null && exception instanceof NestedServletException && exception.getCause() != null) { + method = resolveMethodByExceptionType(exception.getCause().getClass()); + } + return method; } /** @@ -133,7 +138,7 @@ public class ExceptionHandlerMethodResolver { * @param exceptionType the exception type * @return a Method to handle the exception, or {@code null} if none found */ - public Method resolveMethodByExceptionType(Class exceptionType) { + public Method resolveMethodByExceptionType(Class exceptionType) { Method method = this.exceptionLookupCache.get(exceptionType); if (method == null) { method = getMappedMethod(exceptionType); @@ -145,7 +150,7 @@ public class ExceptionHandlerMethodResolver { /** * Return the {@link Method} mapped to the given exception type, or {@code null} if none. */ - private Method getMappedMethod(Class exceptionType) { + private Method getMappedMethod(Class exceptionType) { List> matches = new ArrayList>(); for (Class mappedException : this.mappedMethods.keySet()) { if (mappedException.isAssignableFrom(exceptionType)) { diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index 5d535250ff8..5155fd9f3bd 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -972,13 +972,19 @@ public class DispatcherServlet extends FrameworkServlet { catch (Exception ex) { dispatchException = ex; } + catch (Error err) { + // As of 4.3, we're processing Errors thrown from handler methods as well, + // making them available for @ExceptionHandler methods and other scenarios. + dispatchException = new NestedServletException("Handler dispatch failed", err); + } processDispatchResult(processedRequest, response, mappedHandler, mv, dispatchException); } catch (Exception ex) { triggerAfterCompletion(processedRequest, response, mappedHandler, ex); } catch (Error err) { - triggerAfterCompletionWithError(processedRequest, response, mappedHandler, err); + triggerAfterCompletion(processedRequest, response, mappedHandler, + new NestedServletException("Handler processing failed", err)); } finally { if (asyncManager.isConcurrentHandlingStarted()) { @@ -1304,16 +1310,6 @@ public class DispatcherServlet extends FrameworkServlet { throw ex; } - private void triggerAfterCompletionWithError(HttpServletRequest request, HttpServletResponse response, - HandlerExecutionChain mappedHandler, Error error) throws Exception { - - ServletException ex = new NestedServletException("Handler processing failed", error); - if (mappedHandler != null) { - mappedHandler.triggerAfterCompletion(request, response, ex); - } - throw ex; - } - /** * Restore the request attributes after an include. * @param request current HTTP request diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java index cd304924edf..63938c7f821 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java @@ -95,7 +95,7 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce public ExceptionHandlerExceptionResolver() { StringHttpMessageConverter stringHttpMessageConverter = new StringHttpMessageConverter(); - stringHttpMessageConverter.setWriteAcceptCharset(false); // See SPR-7316 + stringHttpMessageConverter.setWriteAcceptCharset(false); // see SPR-7316 this.messageConverters = new ArrayList>(); this.messageConverters.add(new ByteArrayHttpMessageConverter()); @@ -364,7 +364,15 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce if (logger.isDebugEnabled()) { logger.debug("Invoking @ExceptionHandler method: " + exceptionHandlerMethod); } - exceptionHandlerMethod.invokeAndHandle(webRequest, mavContainer, exception, handlerMethod); + if (exception.getCause() != null) { + // Expose root cause as provided argument as well + exceptionHandlerMethod.invokeAndHandle( + webRequest, mavContainer, exception, exception.getCause(), handlerMethod); + } + else { + // Otherwise, just the given exception as-is + exceptionHandlerMethod.invokeAndHandle(webRequest, mavContainer, exception, handlerMethod); + } } catch (Exception invocationEx) { if (logger.isDebugEnabled()) { diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java index 1dc3c7f00f2..be2926fdf64 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -42,12 +42,9 @@ import org.springframework.web.method.annotation.ModelMethodProcessor; import org.springframework.web.method.support.HandlerMethodArgumentResolver; import org.springframework.web.method.support.HandlerMethodReturnValueHandler; import org.springframework.web.servlet.ModelAndView; +import org.springframework.web.util.NestedServletException; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; /** * Test fixture with {@link ExceptionHandlerExceptionResolver}. @@ -240,6 +237,22 @@ public class ExceptionHandlerExceptionResolverTests { assertEquals("HandlerMethod: handle", this.response.getContentAsString()); } + @Test + public void resolveExceptionWithAssertionError() throws Exception { + AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class); + this.resolver.setApplicationContext(cxt); + this.resolver.afterPropertiesSet(); + + AssertionError err = new AssertionError("argh"); + HandlerMethod handlerMethod = new HandlerMethod(new ResponseBodyController(), "handle"); + ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, + new NestedServletException("Handler dispatch failed", err)); + + assertNotNull("Exception was not handled", mav); + assertTrue(mav.isEmpty()); + assertEquals(err.toString(), this.response.getContentAsString()); + } + @Test public void resolveExceptionControllerAdviceHandler() throws Exception { AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); @@ -349,6 +362,11 @@ public class ExceptionHandlerExceptionResolverTests { public String handleWithHandlerMethod(HandlerMethod handlerMethod) { return "HandlerMethod: " + handlerMethod.getMethod().getName(); } + + @ExceptionHandler(AssertionError.class) + public String handleAssertionError(Error err) { + return err.toString(); + } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java index 369fdf8ea08..24d9ea2c05e 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ServletAnnotationControllerHandlerMethodTests.java @@ -139,30 +139,23 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes; import org.springframework.web.servlet.support.RequestContextUtils; import org.springframework.web.servlet.view.InternalResourceViewResolver; -import static org.junit.Assert.assertArrayEquals; -import static org.junit.Assert.assertEquals; -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.assertTrue; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; /** * The origin of this test class is {@link ServletAnnotationControllerHandlerMethodTests}. * * Tests in this class run against the {@link HandlerMethod} infrastructure: *
    - *
  • RequestMappingHandlerMapping - *
  • RequestMappingHandlerAdapter - *
  • ExceptionHandlerExceptionResolver + *
  • RequestMappingHandlerMapping + *
  • RequestMappingHandlerAdapter + *
  • ExceptionHandlerExceptionResolver *
* *

Rather than against the existing infrastructure: *

    - *
  • DefaultAnnotationHandlerMapping - *
  • AnnotationMethodHandlerAdapter - *
  • AnnotationMethodHandlerExceptionResolver + *
  • DefaultAnnotationHandlerMapping + *
  • AnnotationMethodHandlerAdapter + *
  • AnnotationMethodHandlerExceptionResolver *
* * @author Rossen Stoyanchev @@ -181,6 +174,18 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl assertEquals("test", response.getContentAsString()); } + @Test + public void errorThrownFromHandlerMethod() throws Exception { + initServletWithControllers(ControllerWithErrorThrown.class); + + MockHttpServletRequest request = new MockHttpServletRequest("GET", "/foo"); + request.setContextPath("/foo"); + request.setServletPath(""); + MockHttpServletResponse response = new MockHttpServletResponse(); + getServlet().service(request, response); + assertEquals("test", response.getContentAsString()); + } + @Test public void customAnnotationController() throws Exception { initServletWithControllers(CustomAnnotationController.class); @@ -1808,6 +1813,25 @@ public class ServletAnnotationControllerHandlerMethodTests extends AbstractServl } } + @Controller + private static class ControllerWithErrorThrown { + + @RequestMapping("") + public void myPath2(HttpServletResponse response) throws IOException { + throw new AssertionError("test"); + } + + @RequestMapping("/bar") + public void myPath3(HttpServletResponse response) throws IOException { + response.getWriter().write("testX"); + } + + @ExceptionHandler + public void myPath2(Error err, HttpServletResponse response) throws IOException { + response.getWriter().write(err.getMessage()); + } + } + @Controller static class MyAdaptedController {