Add generic error handling to BasicErrorController

Since Spring supports gobal error handling through
@ControllerAdvice, it is quite easy to set up more meta-data
about an exception for the BasicErrorController. You need
to be careful not to swallow Security exceptions, and probably
others (optionally) so this feature needs a bit more work.

See gh-538

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/autoconfigure/ManagementSecurityAutoConfiguration.java

@@ -22,6 +22,8 @@ import java.util.List;
 import java.util.Set;
 
 import javax.annotation.PostConstruct;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.boot.actuate.endpoint.Endpoint;
@@ -46,6 +48,7 @@ import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.core.Ordered;
 import org.springframework.core.annotation.Order;
+import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.config.annotation.web.WebSecurityConfigurer;
 import org.springframework.security.config.annotation.web.builders.HttpSecurity;
 import org.springframework.security.config.annotation.web.builders.WebSecurity;
@@ -53,9 +56,12 @@ import org.springframework.security.config.annotation.web.builders.WebSecurity.I
 import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfiguration;
 import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
+import org.springframework.security.core.AuthenticationException;
 import org.springframework.security.web.AuthenticationEntryPoint;
 import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint;
 import org.springframework.util.StringUtils;
+import org.springframework.web.bind.annotation.ControllerAdvice;
+import org.springframework.web.bind.annotation.ExceptionHandler;
 
 /**
  * {@link EnableAutoConfiguration Auto-configuration} for security of framework endpoints.
@@ -85,6 +91,19 @@ public class ManagementSecurityAutoConfiguration {
 		return new IgnoredPathsWebSecurityConfigurerAdapter();
 	}
 
+	@ConditionalOnWebApplication
+	@ControllerAdvice
+	@Order(Ordered.HIGHEST_PRECEDENCE + 10)
+	protected static class SecurityExceptionRethrowingAdvice {
+
+		@ExceptionHandler({ AccessDeniedException.class, AuthenticationException.class })
+		public void handle(HttpServletRequest request, HttpServletResponse response,
+				Exception e) throws Exception {
+			throw e;
+		}
+
+	}
+
 	@Configuration
 	protected static class ManagementSecurityPropertiesConfiguration implements
 			SecurityPrequisite {

spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java

@@ -24,19 +24,26 @@ import java.util.Map;
 
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory;
+import org.springframework.core.annotation.Order;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
 import org.springframework.stereotype.Controller;
+import org.springframework.validation.BindException;
+import org.springframework.validation.BindingResult;
+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.bind.annotation.ResponseBody;
 import org.springframework.web.context.request.RequestAttributes;
 import org.springframework.web.context.request.ServletRequestAttributes;
 import org.springframework.web.servlet.ModelAndView;
+import org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolver;
 
 /**
  * Basic global error {@link Controller}, rendering servlet container error codes and
@@ -48,12 +55,16 @@ import org.springframework.web.servlet.ModelAndView;
  * @author Dave Syer
  */
 @Controller
+@ControllerAdvice
+@Order(0)
 public class BasicErrorController implements ErrorController {
 
 	private static final String ERROR_KEY = "error";
 
 	private final Log logger = LogFactory.getLog(BasicErrorController.class);
 
+	private DefaultHandlerExceptionResolver resolver = new DefaultHandlerExceptionResolver();
+
 	@Value("${error.path:/error}")
 	private String errorPath;
 
@@ -62,6 +73,24 @@ public class BasicErrorController implements ErrorController {
 		return this.errorPath;
 	}
 
+	@ExceptionHandler(Exception.class)
+	public void handle(HttpServletRequest request, HttpServletResponse response,
+			Exception e) throws Exception {
+		this.resolver.resolveException(request, response, null, e);
+		if (response.getStatus() == HttpServletResponse.SC_OK) {
+			response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+		}
+		// There's only one exception so it's easier for the error controller to identify
+		// it this way...
+		request.setAttribute(ErrorController.class.getName(), e);
+		if (e instanceof BindException) {
+			// ... but other error pages might be looking for it here as well
+			request.setAttribute(
+					BindingResult.MODEL_KEY_PREFIX + ((BindException) e).getObjectName(),
+					e);
+		}
+	}
+
 	@RequestMapping(value = "${error.path:/error}", produces = "text/html")
 	public ModelAndView errorHtml(HttpServletRequest request) {
 		Map<String, Object> map = extract(new ServletRequestAttributes(request), false,
@@ -96,7 +125,7 @@ public class BasicErrorController implements ErrorController {
 		map.put("timestamp", new Date());
 		try {
 			Throwable error = (Throwable) attributes.getAttribute(
-					"javax.servlet.error.exception", RequestAttributes.SCOPE_REQUEST);
+					ErrorController.class.getName(), RequestAttributes.SCOPE_REQUEST);
 			Object obj = attributes.getAttribute("javax.servlet.error.status_code",
 					RequestAttributes.SCOPE_REQUEST);
 			int status = 999;
@@ -108,12 +137,16 @@ public class BasicErrorController implements ErrorController {
 				map.put(ERROR_KEY, "None");
 			}
 			map.put("status", status);
+			if (error == null) {
+				error = (Throwable) attributes.getAttribute(
+						"javax.servlet.error.exception", RequestAttributes.SCOPE_REQUEST);
+			}
 			if (error != null) {
 				while (error instanceof ServletException && error.getCause() != null) {
 					error = ((ServletException) error).getCause();
 				}
 				map.put("exception", error.getClass().getName());
-				map.put("message", error.getMessage());
+				addMessage(map, error);
 				if (trace) {
 					StringWriter stackTrace = new StringWriter();
 					error.printStackTrace(new PrintWriter(stackTrace));
@@ -129,6 +162,9 @@ public class BasicErrorController implements ErrorController {
 						RequestAttributes.SCOPE_REQUEST);
 				map.put("message", message == null ? "No message available" : message);
 			}
+			String path = (String) attributes.getAttribute(
+					"javax.servlet.error.request_uri", RequestAttributes.SCOPE_REQUEST);
+			map.put("path", path == null ? "No path available" : path);
 			return map;
 		}
 		catch (Exception ex) {
@@ -141,4 +177,22 @@ public class BasicErrorController implements ErrorController {
 		}
 	}
 
+	protected void addMessage(Map<String, Object> map, Throwable error) {
+		if (error instanceof BindingResult) {
+			BindingResult result = (BindingResult) error;
+			if (result.getErrorCount() > 0) {
+				map.put("errors", result.getAllErrors());
+				map.put("message",
+						"Validation failed for object='" + result.getObjectName()
+								+ "'. Error count: " + result.getErrorCount());
+			}
+			else {
+				map.put("message", "No errors");
+			}
+		}
+		else {
+			map.put("message", error.getMessage());
+		}
+	}
+
 }

spring-boot-actuator/src/test/java/org/springframework/boot/actuate/web/BasicErrorControllerIntegrationTests.java

@@ -18,6 +18,8 @@ package org.springframework.boot.actuate.web;
 
 import java.util.Map;
 
+import javax.servlet.DispatcherType;
+import javax.servlet.ServletContext;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -35,11 +37,16 @@ import org.springframework.boot.test.SpringApplicationConfiguration;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
 import org.springframework.http.MediaType;
+import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import org.springframework.test.context.web.WebAppConfiguration;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.RequestBuilder;
 import org.springframework.test.web.servlet.setup.MockMvcBuilders;
+import org.springframework.validation.BindException;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.context.WebApplicationContext;
 import org.springframework.web.servlet.View;
 import org.springframework.web.servlet.view.AbstractView;
@@ -67,7 +74,7 @@ public class BasicErrorControllerIntegrationTests {
 	}
 
 	@Test
-	public void testErrorForMachineClient() throws Exception {
+	public void testDirectAccessForMachineClient() throws Exception {
 		MvcResult response = this.mockMvc.perform(get("/error"))
 				.andExpect(status().is5xxServerError()).andReturn();
 		String content = response.getResponse().getContentAsString();
@@ -75,7 +82,32 @@ public class BasicErrorControllerIntegrationTests {
 	}
 
 	@Test
-	public void testErrorForBrowserClient() throws Exception {
+	public void testErrorForMachineClient() throws Exception {
+		MvcResult result = this.mockMvc.perform(get("/"))
+				.andExpect(status().is5xxServerError()).andReturn();
+		MvcResult response = this.mockMvc.perform(new ErrorDispatcher(result, "/error"))
+				.andReturn();
+		String content = response.getResponse().getContentAsString();
+		assertTrue("Wrong content: " + content, content.contains("Expected!"));
+	}
+
+	@Test
+	public void testBindingExceptionForMachineClient() throws Exception {
+		// In a real container the response is carried over into the error dispatcher, but
+		// in the mock a new one is created so we have to assert the status at this
+		// intermediate point
+		MvcResult result = this.mockMvc.perform(get("/bind"))
+				.andExpect(status().is4xxClientError()).andReturn();
+		MvcResult response = this.mockMvc.perform(new ErrorDispatcher(result, "/error"))
+				.andReturn();
+		// And the rendered status code is always wrong (but would be 400 in a real
+		// system)
+		String content = response.getResponse().getContentAsString();
+		assertTrue("Wrong content: " + content, content.contains("Error count: 1"));
+	}
+
+	@Test
+	public void testDirectAccessForBrowserClient() throws Exception {
 		MvcResult response = this.mockMvc
 				.perform(get("/error").accept(MediaType.TEXT_HTML))
 				.andExpect(status().isOk()).andReturn();
@@ -106,6 +138,46 @@ public class BasicErrorControllerIntegrationTests {
 			};
 		}
 
+		@RestController
+		protected static class Errors {
+
+			public String getFoo() {
+				return "foo";
+			}
+
+			@RequestMapping("/")
+			public String home() {
+				throw new IllegalStateException("Expected!");
+			}
+
+			@RequestMapping("/bind")
+			public String bind() throws Exception {
+				BindException error = new BindException(this, "test");
+				error.rejectValue("foo", "bar.error");
+				throw error;
+			}
+
+		}
+
+	}
+
+	private class ErrorDispatcher implements RequestBuilder {
+
+		private MvcResult result;
+		private String path;
+
+		public ErrorDispatcher(MvcResult result, String path) {
+			this.result = result;
+			this.path = path;
+		}
+
+		@Override
+		public MockHttpServletRequest buildRequest(ServletContext servletContext) {
+			MockHttpServletRequest request = this.result.getRequest();
+			request.setDispatcherType(DispatcherType.ERROR);
+			request.setRequestURI(this.path);
+			return request;
+		}
 	}
 
 }

spring-boot-samples/spring-boot-sample-actuator/pom.xml

@@ -35,6 +35,10 @@
 			<groupId>org.springframework.boot</groupId>
 			<artifactId>spring-boot-starter-remote-shell</artifactId>
 		</dependency>
+		<dependency>
+			<groupId>org.hibernate</groupId>
+			<artifactId>hibernate-validator</artifactId>
+		</dependency>
 		<dependency>
 			<groupId>org.springframework.boot</groupId>
 			<artifactId>spring-boot-starter-test</artifactId>
@@ -44,6 +48,10 @@
 			<exclusions> <exclusion> <groupId>log4j</groupId> <artifactId>log4j</artifactId>
 			</exclusion> <exclusion> <groupId>commons-logging</groupId> <artifactId>commons-logging</artifactId>
 			</exclusion> </exclusions> </dependency> -->
+		<dependency>
+			<groupId>org.hibernate</groupId>
+			<artifactId>hibernate-validator</artifactId>
+		</dependency>
 	</dependencies>
 	<build>
 		<plugins>

spring-boot-samples/spring-boot-sample-actuator/src/main/java/sample/actuator/SampleController.java

@@ -17,12 +17,17 @@
 package sample.actuator;
 
 import java.util.Collections;
+import java.util.Date;
+import java.util.LinkedHashMap;
 import java.util.Map;
 
+import org.hibernate.validator.constraints.NotBlank;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Description;
 import org.springframework.stereotype.Controller;
+import org.springframework.validation.annotation.Validated;
 import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RequestMethod;
 import org.springframework.web.bind.annotation.ResponseBody;
 
 @Controller
@@ -32,16 +37,42 @@ public class SampleController {
 	@Autowired
 	private HelloWorldService helloWorldService;
 
-	@RequestMapping("/")
+	@RequestMapping(value="/",  method=RequestMethod.GET)
 	@ResponseBody
-	public Map<String, String> helloWorld() {
+	public Map<String, String> hello() {
 		return Collections.singletonMap("message",
 				this.helloWorldService.getHelloMessage());
 	}
 
+	@RequestMapping(value="/", method=RequestMethod.POST)
+	@ResponseBody
+	public Map<String, Object> olleh(@Validated Message message) {
+		Map<String, Object> model = new LinkedHashMap<String, Object>();
+		model .put("message", message.getValue());
+		model.put("title", "Hello Home");
+		model.put("date", new Date());
+		return model;
+	}
+
 	@RequestMapping("/foo")
 	@ResponseBody
 	public String foo() {
 		throw new IllegalArgumentException("Server error");
 	}
+	
+	protected static class Message {
+		
+		@NotBlank(message="Message value cannot be empty")
+		private String value;
+
+		public String getValue() {
+			return value;
+		}
+
+		public void setValue(String value) {
+			this.value = value;
+		}
+	}
+
+
 }