From 71c2c69c9224c8998c69e0d75e6285b586c24b12 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 27 Mar 2014 17:22:04 +0000 Subject: [PATCH] Return actual status code not 200 to machine client Machine clients are much more fussy than browsers and we should take care to preserve the HTTP status for them. Fixes gh-596 --- .../boot/actuate/web/BasicErrorController.java | 18 ++++++++++++++---- .../BasicErrorControllerIntegrationTests.java | 2 +- ...opertiesSampleActuatorApplicationTests.java | 2 +- .../SampleActuatorApplicationTests.java | 2 +- 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java index add4a26105b..ff7f1f80159 100644 --- a/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java +++ b/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/web/BasicErrorController.java @@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.annotation.Value; import org.springframework.boot.context.embedded.AbstractEmbeddedServletContainerFactory; import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.bind.annotation.ResponseBody; @@ -63,17 +64,26 @@ public class BasicErrorController implements ErrorController { @RequestMapping(value = "${error.path:/error}", produces = "text/html") public ModelAndView errorHtml(HttpServletRequest request) { - Map map = error(request); + Map map = extract(new ServletRequestAttributes(request), false, + false); return new ModelAndView(ERROR_KEY, map); } @RequestMapping(value = "${error.path:/error}") @ResponseBody - public Map error(HttpServletRequest request) { + public ResponseEntity> error(HttpServletRequest request) { ServletRequestAttributes attributes = new ServletRequestAttributes(request); String trace = request.getParameter("trace"); - return extract(attributes, trace != null && !"false".equals(trace.toLowerCase()), - true); + Map extracted = extract(attributes, + trace != null && !"false".equals(trace.toLowerCase()), true); + HttpStatus statusCode; + try { + statusCode = HttpStatus.valueOf((Integer) extracted.get("status")); + } + catch (Exception e) { + statusCode = HttpStatus.INTERNAL_SERVER_ERROR; + } + return new ResponseEntity>(extracted, statusCode); } @Override diff --git a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/web/BasicErrorControllerIntegrationTests.java b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/web/BasicErrorControllerIntegrationTests.java index b1d1b9a8ad7..29c8c40a8ab 100644 --- a/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/web/BasicErrorControllerIntegrationTests.java +++ b/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/web/BasicErrorControllerIntegrationTests.java @@ -69,7 +69,7 @@ public class BasicErrorControllerIntegrationTests { @Test public void testErrorForMachineClient() throws Exception { MvcResult response = this.mockMvc.perform(get("/error")) - .andExpect(status().isOk()).andReturn(); + .andExpect(status().is5xxServerError()).andReturn(); String content = response.getResponse().getContentAsString(); assertTrue("Wrong content: " + content, content.contains("999")); } diff --git a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java index 823803c7082..b0d34cb676d 100644 --- a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java +++ b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/EndpointsPropertiesSampleActuatorApplicationTests.java @@ -50,7 +50,7 @@ public class EndpointsPropertiesSampleActuatorApplicationTests { @SuppressWarnings("rawtypes") ResponseEntity entity = RestTemplates.get("user", "password").getForEntity( "http://localhost:8080/oops", Map.class); - assertEquals(HttpStatus.OK, entity.getStatusCode()); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, entity.getStatusCode()); @SuppressWarnings("unchecked") Map body = entity.getBody(); assertEquals("None", body.get("error")); diff --git a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/SampleActuatorApplicationTests.java b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/SampleActuatorApplicationTests.java index 34bb9cc1f98..9ccc06cbbcb 100644 --- a/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/SampleActuatorApplicationTests.java +++ b/spring-boot-samples/spring-boot-sample-actuator/src/test/java/sample/actuator/SampleActuatorApplicationTests.java @@ -158,7 +158,7 @@ public class SampleActuatorApplicationTests { @SuppressWarnings("rawtypes") ResponseEntity entity = RestTemplates.get().getForEntity( "http://localhost:8080/error", Map.class); - assertEquals(HttpStatus.OK, entity.getStatusCode()); + assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, entity.getStatusCode()); @SuppressWarnings("unchecked") Map body = entity.getBody(); assertEquals("None", body.get("error"));