diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java index e7300c8686d..6bad41006e8 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandler.java @@ -69,7 +69,7 @@ import org.springframework.web.util.WebUtils; * using view resolution (e.g., via {@code ContentNegotiatingViewResolver}), * then {@code DefaultHandlerExceptionResolver} is good enough. * - *

Note that in order for an {@code @ControllerAdvice} sub-class to be + *

Note that in order for an {@code @ControllerAdvice} subclass to be * detected, {@link ExceptionHandlerExceptionResolver} must be configured. * * @author Rossen Stoyanchev @@ -121,7 +121,7 @@ public abstract class ResponseEntityExceptionHandler { NoHandlerFoundException.class, AsyncRequestTimeoutException.class }) - public final ResponseEntity handleException(Exception ex, WebRequest request) { + public final ResponseEntity handleException(Exception ex, WebRequest request) throws Exception { HttpHeaders headers = new HttpHeaders(); if (ex instanceof org.springframework.web.servlet.mvc.multiaction.NoSuchRequestHandlingMethodException) { HttpStatus status = HttpStatus.NOT_FOUND; @@ -185,38 +185,17 @@ public abstract class ResponseEntityExceptionHandler { } else if (ex instanceof AsyncRequestTimeoutException) { HttpStatus status = HttpStatus.SERVICE_UNAVAILABLE; - return handleAsyncRequestTimeoutException( - (AsyncRequestTimeoutException) ex, headers, status, request); + return handleAsyncRequestTimeoutException((AsyncRequestTimeoutException) ex, headers, status, request); } else { - if (logger.isWarnEnabled()) { - logger.warn("Unknown exception type: " + ex.getClass().getName()); - } - HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR; - return handleExceptionInternal(ex, null, headers, status, request); + // Unknown exception, typically a wrapper with a common MVC exception as cause + // (since @ExceptionHandler type declarations also match first-level causes): + // We only deal with top-level MVC exceptions here, so let's rethrow the given + // exception for further processing through the HandlerExceptionResolver chain. + throw ex; } } - /** - * A single place to customize the response body of all Exception types. - *

The default implementation sets the {@link WebUtils#ERROR_EXCEPTION_ATTRIBUTE} - * request attribute and creates a {@link ResponseEntity} from the given - * body, headers, and status. - * @param ex the exception - * @param body the body for the response - * @param headers the headers for the response - * @param status the response status - * @param request the current request - */ - protected ResponseEntity handleExceptionInternal(Exception ex, Object body, - HttpHeaders headers, HttpStatus status, WebRequest request) { - - if (HttpStatus.INTERNAL_SERVER_ERROR.equals(status)) { - request.setAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE, ex, WebRequest.SCOPE_REQUEST); - } - return new ResponseEntity(body, headers, status); - } - /** * Customize the response for NoSuchRequestHandlingMethodException. *

This method logs a warning and delegates to {@link #handleExceptionInternal}. @@ -228,7 +207,8 @@ public abstract class ResponseEntityExceptionHandler { * @deprecated as of 4.3, along with {@link org.springframework.web.servlet.mvc.multiaction.NoSuchRequestHandlingMethodException} */ @Deprecated - protected ResponseEntity handleNoSuchRequestHandlingMethod(org.springframework.web.servlet.mvc.multiaction.NoSuchRequestHandlingMethodException ex, + protected ResponseEntity handleNoSuchRequestHandlingMethod( + org.springframework.web.servlet.mvc.multiaction.NoSuchRequestHandlingMethodException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { pageNotFoundLogger.warn(ex.getMessage()); @@ -246,8 +226,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleHttpRequestMethodNotSupported(HttpRequestMethodNotSupportedException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleHttpRequestMethodNotSupported( + HttpRequestMethodNotSupportedException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { pageNotFoundLogger.warn(ex.getMessage()); @@ -268,8 +248,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleHttpMediaTypeNotSupported(HttpMediaTypeNotSupportedException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleHttpMediaTypeNotSupported( + HttpMediaTypeNotSupportedException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { List mediaTypes = ex.getSupportedMediaTypes(); if (!CollectionUtils.isEmpty(mediaTypes)) { @@ -288,8 +268,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleHttpMediaTypeNotAcceptable(HttpMediaTypeNotAcceptableException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleHttpMediaTypeNotAcceptable( + HttpMediaTypeNotAcceptableException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -304,8 +284,8 @@ public abstract class ResponseEntityExceptionHandler { * @return a {@code ResponseEntity} instance * @since 4.2 */ - protected ResponseEntity handleMissingPathVariable(MissingPathVariableException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleMissingPathVariable( + MissingPathVariableException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -319,8 +299,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleMissingServletRequestParameter(MissingServletRequestParameterException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleMissingServletRequestParameter( + MissingServletRequestParameterException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -334,8 +314,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleServletRequestBindingException(ServletRequestBindingException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleServletRequestBindingException( + ServletRequestBindingException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -349,8 +329,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleConversionNotSupported(ConversionNotSupportedException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleConversionNotSupported( + ConversionNotSupportedException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -364,8 +344,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleTypeMismatch(TypeMismatchException ex, HttpHeaders headers, - HttpStatus status, WebRequest request) { + protected ResponseEntity handleTypeMismatch( + TypeMismatchException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -379,8 +359,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleHttpMessageNotReadable(HttpMessageNotReadableException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleHttpMessageNotReadable( + HttpMessageNotReadableException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -394,8 +374,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleHttpMessageNotWritable(HttpMessageNotWritableException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleHttpMessageNotWritable( + HttpMessageNotWritableException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -409,8 +389,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleMethodArgumentNotValid(MethodArgumentNotValidException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleMethodArgumentNotValid( + MethodArgumentNotValidException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -424,8 +404,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleMissingServletRequestPart(MissingServletRequestPartException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleMissingServletRequestPart( + MissingServletRequestPartException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -439,8 +419,8 @@ public abstract class ResponseEntityExceptionHandler { * @param request the current request * @return a {@code ResponseEntity} instance */ - protected ResponseEntity handleBindException(BindException ex, HttpHeaders headers, - HttpStatus status, WebRequest request) { + protected ResponseEntity handleBindException( + BindException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { return handleExceptionInternal(ex, null, headers, status, request); } @@ -489,4 +469,24 @@ public abstract class ResponseEntityExceptionHandler { return handleExceptionInternal(ex, null, headers, status, webRequest); } + /** + * A single place to customize the response body of all Exception types. + *

The default implementation sets the {@link WebUtils#ERROR_EXCEPTION_ATTRIBUTE} + * request attribute and creates a {@link ResponseEntity} from the given + * body, headers, and status. + * @param ex the exception + * @param body the body for the response + * @param headers the headers for the response + * @param status the response status + * @param request the current request + */ + protected ResponseEntity handleExceptionInternal( + Exception ex, Object body, HttpHeaders headers, HttpStatus status, WebRequest request) { + + if (HttpStatus.INTERNAL_SERVER_ERROR.equals(status)) { + request.setAttribute(WebUtils.ERROR_EXCEPTION_ATTRIBUTE, ex, WebRequest.SCOPE_REQUEST); + } + return new ResponseEntity(body, headers, status); + } + } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java index c35b6218f25..1352582bef4 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ResponseEntityExceptionHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 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. @@ -20,6 +20,7 @@ import java.lang.reflect.Method; import java.util.Arrays; import java.util.EnumSet; import java.util.List; +import javax.servlet.ServletException; import org.junit.Before; import org.junit.Test; @@ -38,6 +39,8 @@ import org.springframework.http.converter.HttpMessageNotWritableException; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.mock.web.test.MockHttpServletRequest; import org.springframework.mock.web.test.MockHttpServletResponse; +import org.springframework.mock.web.test.MockServletConfig; +import org.springframework.stereotype.Controller; import org.springframework.validation.BindException; import org.springframework.web.HttpMediaTypeNotAcceptableException; import org.springframework.web.HttpMediaTypeNotSupportedException; @@ -48,11 +51,13 @@ import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.ServletRequestBindingException; import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.context.request.ServletWebRequest; import org.springframework.web.context.request.WebRequest; import org.springframework.web.context.request.async.AsyncRequestTimeoutException; import org.springframework.web.context.support.StaticWebApplicationContext; import org.springframework.web.multipart.support.MissingServletRequestPartException; +import org.springframework.web.servlet.DispatcherServlet; import org.springframework.web.servlet.NoHandlerFoundException; import org.springframework.web.servlet.mvc.multiaction.NoSuchRequestHandlingMethodException; import org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolver; @@ -87,9 +92,9 @@ public class ResponseEntityExceptionHandlerTests { this.defaultExceptionResolver = new DefaultHandlerExceptionResolver(); } + @Test public void supportsAllDefaultHandlerExceptionResolverExceptionTypes() throws Exception { - Class clazz = ResponseEntityExceptionHandler.class; Method handleExceptionMethod = clazz.getMethod("handleException", Exception.class, WebRequest.class); ExceptionHandler annotation = handleExceptionMethod.getAnnotation(ExceptionHandler.class); @@ -106,7 +111,7 @@ public class ResponseEntityExceptionHandlerTests { @Test public void noSuchRequestHandlingMethod() { - Exception ex = new NoSuchRequestHandlingMethodException("GET", TestController.class); + Exception ex = new NoSuchRequestHandlingMethodException("GET", getClass()); testException(ex); } @@ -117,7 +122,6 @@ public class ResponseEntityExceptionHandlerTests { ResponseEntity responseEntity = testException(ex); assertEquals(EnumSet.of(HttpMethod.POST, HttpMethod.DELETE), responseEntity.getHeaders().getAllow()); - } @Test @@ -213,54 +217,131 @@ public class ResponseEntityExceptionHandlerTests { @Test public void controllerAdvice() throws Exception { - StaticWebApplicationContext cxt = new StaticWebApplicationContext(); - cxt.registerSingleton("exceptionHandler", ApplicationExceptionHandler.class); - cxt.refresh(); + StaticWebApplicationContext ctx = new StaticWebApplicationContext(); + ctx.registerSingleton("exceptionHandler", ApplicationExceptionHandler.class); + ctx.refresh(); ExceptionHandlerExceptionResolver resolver = new ExceptionHandlerExceptionResolver(); - resolver.setApplicationContext(cxt); + resolver.setApplicationContext(ctx); resolver.afterPropertiesSet(); ServletRequestBindingException ex = new ServletRequestBindingException("message"); - resolver.resolveException(this.servletRequest, this.servletResponse, null, ex); + assertNotNull(resolver.resolveException(this.servletRequest, this.servletResponse, null, ex)); assertEquals(400, this.servletResponse.getStatus()); assertEquals("error content", this.servletResponse.getContentAsString()); assertEquals("someHeaderValue", this.servletResponse.getHeader("someHeader")); } + @Test + public void controllerAdviceWithNestedException() { + StaticWebApplicationContext ctx = new StaticWebApplicationContext(); + ctx.registerSingleton("exceptionHandler", ApplicationExceptionHandler.class); + ctx.refresh(); + + ExceptionHandlerExceptionResolver resolver = new ExceptionHandlerExceptionResolver(); + resolver.setApplicationContext(ctx); + resolver.afterPropertiesSet(); + + IllegalStateException ex = new IllegalStateException(new ServletRequestBindingException("message")); + assertNull(resolver.resolveException(this.servletRequest, this.servletResponse, null, ex)); + } + + @Test + public void controllerAdviceWithinDispatcherServlet() throws Exception { + StaticWebApplicationContext ctx = new StaticWebApplicationContext(); + ctx.registerSingleton("controller", ExceptionThrowingController.class); + ctx.registerSingleton("exceptionHandler", ApplicationExceptionHandler.class); + ctx.registerSingleton("exceptionResolver", ExceptionHandlerExceptionResolver.class); + ctx.registerSingleton("handlerMapping", RequestMappingHandlerMapping.class); + ctx.registerSingleton("handlerAdapter", RequestMappingHandlerAdapter.class); + ctx.refresh(); + + DispatcherServlet servlet = new DispatcherServlet(ctx); + servlet.init(new MockServletConfig()); + servlet.service(this.servletRequest, this.servletResponse); + + assertEquals(400, this.servletResponse.getStatus()); + assertEquals("error content", this.servletResponse.getContentAsString()); + assertEquals("someHeaderValue", this.servletResponse.getHeader("someHeader")); + } + + @Test + public void controllerAdviceWithNestedExceptionWithinDispatcherServlet() throws Exception { + StaticWebApplicationContext ctx = new StaticWebApplicationContext(); + ctx.registerSingleton("controller", NestedExceptionThrowingController.class); + ctx.registerSingleton("exceptionHandler", ApplicationExceptionHandler.class); + ctx.registerSingleton("exceptionResolver", ExceptionHandlerExceptionResolver.class); + ctx.registerSingleton("handlerMapping", RequestMappingHandlerMapping.class); + ctx.registerSingleton("handlerAdapter", RequestMappingHandlerAdapter.class); + ctx.refresh(); + + DispatcherServlet servlet = new DispatcherServlet(ctx); + servlet.init(new MockServletConfig()); + try { + servlet.service(this.servletRequest, this.servletResponse); + } + catch (ServletException ex) { + assertTrue(ex.getCause() instanceof IllegalStateException); + assertTrue(ex.getCause().getCause() instanceof ServletRequestBindingException); + } + } + private ResponseEntity testException(Exception ex) { - ResponseEntity responseEntity = this.exceptionHandlerSupport.handleException(ex, this.request); + try { + ResponseEntity responseEntity = this.exceptionHandlerSupport.handleException(ex, this.request); + + // SPR-9653 + if (HttpStatus.INTERNAL_SERVER_ERROR.equals(responseEntity.getStatusCode())) { + assertSame(ex, this.servletRequest.getAttribute("javax.servlet.error.exception")); + } + + this.defaultExceptionResolver.resolveException(this.servletRequest, this.servletResponse, null, ex); - // SPR-9653 - if (HttpStatus.INTERNAL_SERVER_ERROR.equals(responseEntity.getStatusCode())) { - assertSame(ex, this.servletRequest.getAttribute("javax.servlet.error.exception")); + assertEquals(this.servletResponse.getStatus(), responseEntity.getStatusCode().value()); + + return responseEntity; + } + catch (Exception ex2) { + throw new IllegalStateException("handleException threw exception", ex2); } + } - this.defaultExceptionResolver.resolveException(this.servletRequest, this.servletResponse, null, ex); - assertEquals(this.servletResponse.getStatus(), responseEntity.getStatusCode().value()); + @Controller + private static class ExceptionThrowingController { - return responseEntity; + @RequestMapping("/") + public void handleRequest() throws Exception { + throw new ServletRequestBindingException("message"); + } } - private static class TestController { + @Controller + private static class NestedExceptionThrowingController { + + @RequestMapping("/") + public void handleRequest() throws Exception { + throw new IllegalStateException(new ServletRequestBindingException("message")); + } } + @ControllerAdvice private static class ApplicationExceptionHandler extends ResponseEntityExceptionHandler { @Override - protected ResponseEntity handleServletRequestBindingException(ServletRequestBindingException ex, - HttpHeaders headers, HttpStatus status, WebRequest request) { + protected ResponseEntity handleServletRequestBindingException( + ServletRequestBindingException ex, HttpHeaders headers, HttpStatus status, WebRequest request) { headers.set("someHeader", "someHeaderValue"); return handleExceptionInternal(ex, "error content", headers, status, request); } } + @SuppressWarnings("unused") void handle(String arg) { }