diff --git a/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc b/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc index 0457c264469..91396fa1843 100644 --- a/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc +++ b/spring-boot-docs/src/main/asciidoc/spring-boot-features.adoc @@ -952,8 +952,8 @@ Spring Boot includes auto-configuration support for the following templating eng * http://www.thymeleaf.org[Thymeleaf] * http://velocity.apache.org[Velocity] -When you're using one of these templating engines with the default configuration, your templates -will be picked up automatically from `src/main/resources/templates`. +When you're using one of these templating engines with the default configuration, your +templates will be picked up automatically from `src/main/resources/templates`. TIP: JSPs should be avoided if possible, there are several <> when using them with embedded @@ -988,36 +988,45 @@ support a uniform Java DSL for customizing the error handling. For example: @Override public void customize(ConfigurableEmbeddedServletContainer container) { - container.addErrorPages(new ErrorPage(HttpStatus.BAD_REQUEST, "/400")); + container.addErrorPages(new ErrorPage(HttpStatus.BAD_REQUEST, "/400")); } } ---- -You can also use regular Spring MVC features like http://docs.spring.io/spring/docs/current/spring-framework-reference/htmlsingle/#mvc-exception-handlers[`@ExceptionHandler` -methods] and http://docs.spring.io/spring/docs/current/spring-framework-reference/htmlsingle/#mvc-ann-controller-advice[`@ControllerAdvice`]. -The `ErrorController` will then pick up any unhandled exceptions. +You can also use regular Spring MVC features like +{spring-reference}/#mvc-exceptionhandlers[`@ExceptionHandler` methods] and +{spring-reference}/#mvc-ann-controller-advice[`@ControllerAdvice`]. The `ErrorController` +will then pick up any unhandled exceptions. N.B. if you register an `ErrorPage` with a path that will end up being handled by a -`Filter` (e.g. as is common with some non-Spring web frameworks, like Jersey and Wicket), +`Filter` (e.g. as is common with some non-Spring web frameworks, like Jersey and Wicket), then the `Filter` has to be explicitly registered as an `ERROR` dispatcher, e.g. [source,java,indent=0,subs="verbatim,quotes,attributes"] ---- @Bean public FilterRegistrationBean myFilter() { - - FilterRegistrationBean registration = new FilterRegistrationBean(); - registration.setFilter(new MyFilter()); - ... - registration.setDispatcherTypes(EnumSet.allOf(DispatcherType.class)); - return registration; - + FilterRegistrationBean registration = new FilterRegistrationBean(); + registration.setFilter(new MyFilter()); + ... + registration.setDispatcherTypes(EnumSet.allOf(DispatcherType.class)); + return registration; } ---- (the default `FilterRegistrationBean` does not include the `ERROR` dispatcher type). +[[boot-features-error-handling-websphere]] +===== Error Handling on WebSphere Application Server +When deployed to a servlet container, a Spring Boot uses its error page filter to +forward a request with an error status to the appropriate error page. The request can +only be forwarded to the correct error page if the response has not already been +committed. By default, WebSphere Application Server 8.0 and later commits the response +upon successful completion of a servlet's service method. You should disable this +behaviour by setting `com.ibm.ws.webcontainer.invokeFlushAfterService` to `false` + + [[boot-features-embedded-container]] diff --git a/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java b/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java index 69ebc5a9b3c..ab1e2533093 100644 --- a/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java +++ b/spring-boot/src/main/java/org/springframework/boot/context/web/ErrorPageFilter.java @@ -52,6 +52,7 @@ import org.springframework.web.filter.OncePerRequestFilter; * * @author Dave Syer * @author Phillip Webb + * @author Andy Wilkinson */ @Component @Order(Ordered.HIGHEST_PRECEDENCE) @@ -124,6 +125,12 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple private void handleErrorStatus(HttpServletRequest request, HttpServletResponse response, int status, String message) throws ServletException, IOException { + + if (response.isCommitted()) { + handleCommittedResponse(request, null); + return; + } + String errorPath = getErrorPath(this.statuses, status); if (errorPath == null) { response.sendError(status, message); @@ -132,6 +139,7 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple response.setStatus(status); setErrorAttributes(request, status, message); request.getRequestDispatcher(errorPath).forward(request, response); + } private void handleException(HttpServletRequest request, @@ -143,33 +151,49 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple rethrow(ex); return; } + if (response.isCommitted()) { + handleCommittedResponse(request, ex); + return; + } + + forwardToErrorPage(errorPath, request, wrapped, ex); + } + + private void forwardToErrorPage(String path, HttpServletRequest request, + HttpServletResponse response, Throwable ex) throws ServletException, + IOException { + if (logger.isErrorEnabled()) { String message = "Forwarding to error page from request [" + request.getServletPath() + request.getPathInfo() + "] due to exception [" + ex.getMessage() + "]"; logger.error(message, ex); } + setErrorAttributes(request, 500, ex.getMessage()); request.setAttribute(ERROR_EXCEPTION, ex); - request.setAttribute(ERROR_EXCEPTION_TYPE, type.getName()); - forwardToErrorPage(errorPath, request, wrapped, ex); + request.setAttribute(ERROR_EXCEPTION_TYPE, ex.getClass().getName()); + + response.reset(); + response.sendError(500, ex.getMessage()); + request.getRequestDispatcher(path).forward(request, response); } - private void forwardToErrorPage(String path, HttpServletRequest request, - HttpServletResponse response, Throwable ex) throws ServletException, - IOException { - if (response.isCommitted()) { - String message = "Cannot forward to error page for" + request.getRequestURI() - + " (response is committed), so this response may have " - + "the wrong status code"; + private void handleCommittedResponse(HttpServletRequest request, Throwable ex) { + String message = "Cannot forward to error page for request to " + + request.getRequestURI() + " as the response has already been" + + " committed. As a result, the response may have the wrong status" + + " code. If your application is running on WebSphere Application" + + " Server you may be able to resolve this problem by setting " + + " com.ibm.ws.webcontainer.invokeFlushAfterService to false"; + if (ex == null) { + logger.error(message); + } + else { // User might see the error page without all the data here but throwing the // exception isn't going to help anyone (we'll log it to be on the safe side) logger.error(message, ex); - return; } - response.reset(); - response.sendError(500, ex.getMessage()); - request.getRequestDispatcher(path).forward(request, response); } private String getErrorPath(Map map, Integer status) { @@ -243,6 +267,8 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple private String message; + private boolean errorToSend; + public ErrorWrapperResponse(HttpServletResponse response) { super(response); this.status = response.getStatus(); @@ -257,6 +283,8 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple public void sendError(int status, String message) throws IOException { this.status = status; this.message = message; + + this.errorToSend = true; } @Override @@ -264,6 +292,15 @@ class ErrorPageFilter extends AbstractConfigurableEmbeddedServletContainer imple return this.status; } + @Override + public void flushBuffer() throws IOException { + if (this.errorToSend && !isCommitted()) { + ((HttpServletResponse) getResponse()) + .sendError(this.status, this.message); + } + super.flushBuffer(); + } + public String getMessage() { return this.message; } diff --git a/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java b/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java index 968df7f39ff..c6dee4244d2 100644 --- a/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/context/web/ErrorPageFilterTests.java @@ -34,6 +34,8 @@ import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; @@ -43,6 +45,7 @@ import static org.junit.Assert.assertTrue; * Tests for {@link ErrorPageFilter}. * * @author Dave Syer + * @author Andy Wilkinson */ public class ErrorPageFilterTests { @@ -61,6 +64,7 @@ public class ErrorPageFilterTests { assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getResponse(), equalTo((ServletResponse) this.response)); assertTrue(this.response.isCommitted()); + assertThat(this.response.getForwardedUrl(), is(nullValue())); } @Test @@ -83,6 +87,7 @@ public class ErrorPageFilterTests { assertThat(wrapper.getStatus(), equalTo(401)); // The real response has to be 401 as well... assertThat(this.response.getStatus(), equalTo(401)); + assertThat(this.response.getForwardedUrl(), equalTo("/error")); } @Test @@ -103,11 +108,12 @@ public class ErrorPageFilterTests { equalTo((ServletResponse) this.response)); assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(), equalTo(400)); + assertThat(this.response.getForwardedUrl(), is(nullValue())); assertTrue(this.response.isCommitted()); } @Test - public void responseUncommitted() throws Exception { + public void responseUncommittedWithoutErrorPage() throws Exception { this.chain = new MockFilterChain() { @Override public void doFilter(ServletRequest request, ServletResponse response) @@ -122,6 +128,7 @@ public class ErrorPageFilterTests { equalTo((ServletResponse) this.response)); assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(), equalTo(400)); + assertThat(this.response.getForwardedUrl(), is(nullValue())); assertTrue(this.response.isCommitted()); } @@ -159,6 +166,7 @@ public class ErrorPageFilterTests { assertThat(this.request.getAttribute(RequestDispatcher.ERROR_MESSAGE), equalTo((Object) "BAD")); assertTrue(this.response.isCommitted()); + assertThat(this.response.getForwardedUrl(), equalTo("/error")); } @Test @@ -180,6 +188,26 @@ public class ErrorPageFilterTests { assertThat(this.request.getAttribute(RequestDispatcher.ERROR_MESSAGE), equalTo((Object) "BAD")); assertTrue(this.response.isCommitted()); + assertThat(this.response.getForwardedUrl(), equalTo("/400")); + } + + @Test + public void statusErrorWithCommittedResponse() throws Exception { + this.filter.addErrorPages(new ErrorPage(HttpStatus.BAD_REQUEST, "/400")); + this.chain = new MockFilterChain() { + @Override + public void doFilter(ServletRequest request, ServletResponse response) + throws IOException, ServletException { + ((HttpServletResponse) response).sendError(400, "BAD"); + response.flushBuffer(); + super.doFilter(request, response); + } + }; + this.filter.doFilter(this.request, this.response, this.chain); + assertThat(((HttpServletResponseWrapper) this.chain.getResponse()).getStatus(), + equalTo(400)); + assertTrue(this.response.isCommitted()); + assertThat(this.response.getForwardedUrl(), is(nullValue())); } @Test @@ -203,6 +231,23 @@ public class ErrorPageFilterTests { assertThat(this.request.getAttribute(RequestDispatcher.ERROR_EXCEPTION_TYPE), equalTo((Object) RuntimeException.class.getName())); assertTrue(this.response.isCommitted()); + assertThat(this.response.getForwardedUrl(), equalTo("/500")); + } + + @Test + public void exceptionErrorWithCommittedResponse() throws Exception { + this.filter.addErrorPages(new ErrorPage(RuntimeException.class, "/500")); + this.chain = new MockFilterChain() { + @Override + public void doFilter(ServletRequest request, ServletResponse response) + throws IOException, ServletException { + super.doFilter(request, response); + response.flushBuffer(); + throw new RuntimeException("BAD"); + } + }; + this.filter.doFilter(this.request, this.response, this.chain); + assertThat(this.response.getForwardedUrl(), is(nullValue())); } @Test