diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/CompositeHandlerExceptionResolver.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/CompositeHandlerExceptionResolver.java index 6d7b7121e35..3fc31ffc596 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/CompositeHandlerExceptionResolver.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/main/java/org/springframework/boot/actuate/autoconfigure/web/servlet/CompositeHandlerExceptionResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -19,6 +19,7 @@ package org.springframework.boot.actuate.autoconfigure.web.servlet; import java.util.ArrayList; import java.util.List; import java.util.Objects; +import java.util.Optional; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -36,6 +37,7 @@ import org.springframework.web.servlet.mvc.support.DefaultHandlerExceptionResolv * @author Andy Wilkinson * @author Stephane Nicoll * @author Phillip Webb + * @author Scott Frederick */ class CompositeHandlerExceptionResolver implements HandlerExceptionResolver { @@ -50,8 +52,15 @@ class CompositeHandlerExceptionResolver implements HandlerExceptionResolver { if (this.resolvers == null) { this.resolvers = extractResolvers(); } - return this.resolvers.stream().map((resolver) -> resolver.resolveException(request, response, handler, ex)) - .filter(Objects::nonNull).findFirst().orElse(null); + Optional modelAndView = this.resolvers.stream() + .map((resolver) -> resolver.resolveException(request, response, handler, ex)).filter(Objects::nonNull) + .findFirst(); + modelAndView.ifPresent((mav) -> { + if (mav.isEmpty()) { + request.setAttribute("javax.servlet.error.exception", ex); + } + }); + return modelAndView.orElse(null); } private List extractResolvers() { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/CompositeHandlerExceptionResolverTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/CompositeHandlerExceptionResolverTests.java index 4fdc3f0e8a4..129d7dd462c 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/CompositeHandlerExceptionResolverTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/CompositeHandlerExceptionResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2019 the original author or authors. + * Copyright 2012-2020 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. @@ -38,6 +38,7 @@ import static org.assertj.core.api.Assertions.assertThat; * Tests for {@link CompositeHandlerExceptionResolver}. * * @author Madhura Bhave + * @author Scott Frederick */ class CompositeHandlerExceptionResolverTests { @@ -62,9 +63,11 @@ class CompositeHandlerExceptionResolverTests { load(BaseConfiguration.class); CompositeHandlerExceptionResolver resolver = (CompositeHandlerExceptionResolver) this.context .getBean(DispatcherServlet.HANDLER_EXCEPTION_RESOLVER_BEAN_NAME); - ModelAndView resolved = resolver.resolveException(this.request, this.response, null, - new HttpRequestMethodNotSupportedException("POST")); + HttpRequestMethodNotSupportedException exception = new HttpRequestMethodNotSupportedException("POST"); + ModelAndView resolved = resolver.resolveException(this.request, this.response, null, exception); assertThat(resolved).isNotNull(); + assertThat(resolved.isEmpty()).isTrue(); + assertThat(this.request.getAttribute("javax.servlet.error.exception")).isSameAs(exception); } private void load(Class... configs) { diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/WebMvcEndpointChildContextConfigurationIntegrationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/WebMvcEndpointChildContextConfigurationIntegrationTests.java index d0517a20aac..3fffcd87bab 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/WebMvcEndpointChildContextConfigurationIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/web/servlet/WebMvcEndpointChildContextConfigurationIntegrationTests.java @@ -16,7 +16,12 @@ package org.springframework.boot.actuate.autoconfigure.web.servlet; +import java.util.Collections; import java.util.Map; +import java.util.function.Consumer; + +import javax.validation.Valid; +import javax.validation.constraints.NotEmpty; import org.junit.jupiter.api.Test; @@ -25,15 +30,21 @@ import org.springframework.boot.actuate.autoconfigure.endpoint.web.WebEndpointAu import org.springframework.boot.actuate.autoconfigure.web.server.ManagementContextAutoConfiguration; import org.springframework.boot.actuate.endpoint.annotation.Endpoint; import org.springframework.boot.actuate.endpoint.annotation.ReadOperation; +import org.springframework.boot.actuate.endpoint.web.annotation.RestControllerEndpoint; import org.springframework.boot.autoconfigure.AutoConfigurations; import org.springframework.boot.autoconfigure.web.servlet.DispatcherServletAutoConfiguration; import org.springframework.boot.autoconfigure.web.servlet.ServletWebServerFactoryAutoConfiguration; import org.springframework.boot.autoconfigure.web.servlet.error.ErrorMvcAutoConfiguration; +import org.springframework.boot.test.context.assertj.AssertableWebApplicationContext; +import org.springframework.boot.test.context.runner.ContextConsumer; import org.springframework.boot.test.context.runner.WebApplicationContextRunner; import org.springframework.boot.web.context.ServerPortInfoApplicationContextInitializer; import org.springframework.boot.web.servlet.context.AnnotationConfigServletWebServerApplicationContext; import org.springframework.http.MediaType; -import org.springframework.stereotype.Component; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestBody; +import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.reactive.function.client.ClientResponse; import org.springframework.web.reactive.function.client.WebClient; @@ -54,40 +65,80 @@ class WebMvcEndpointChildContextConfigurationIntegrationTests { ServletManagementContextAutoConfiguration.class, WebEndpointAutoConfiguration.class, EndpointAutoConfiguration.class, DispatcherServletAutoConfiguration.class, ErrorMvcAutoConfiguration.class)) - .withUserConfiguration(FailingEndpoint.class) - .withInitializer(new ServerPortInfoApplicationContextInitializer()).withPropertyValues( - "server.port=0", "management.server.port=0", "management.endpoints.web.exposure.include=*"); + .withUserConfiguration(FailingEndpoint.class, FailingControllerEndpoint.class) + .withInitializer(new ServerPortInfoApplicationContextInitializer()) + .withPropertyValues("server.port=0", "management.server.port=0", + "management.endpoints.web.exposure.include=*", "server.error.include-exception=true", + "server.error.include-message=always", "server.error.include-binding-errors=always"); @Test // gh-17938 - @SuppressWarnings("unchecked") - void errorPageAndErrorControllerAreUsed() { - this.runner.run((context) -> { - String port = context.getEnvironment().getProperty("local.management.port"); - WebClient client = WebClient.create("http://localhost:" + port); + void errorEndpointIsUsedWithEndpoint() { + this.runner.run(withWebTestClient((client) -> { ClientResponse response = client.get().uri("actuator/fail").accept(MediaType.APPLICATION_JSON).exchange() .block(); - Map body = response.bodyToMono(Map.class).block(); - assertThat(body).containsEntry("message", ""); - assertThat(body).doesNotContainKey("trace"); - }); + Map body = getResponseBody(response); + assertThat(body).hasEntrySatisfying("exception", + (value) -> assertThat(value).asString().contains("IllegalStateException")); + assertThat(body).hasEntrySatisfying("message", + (value) -> assertThat(value).asString().contains("Epic Fail")); + })); } @Test void errorPageAndErrorControllerIncludeDetails() { this.runner.withPropertyValues("server.error.include-stacktrace=always", "server.error.include-message=always") - .run((context) -> { - String port = context.getEnvironment().getProperty("local.management.port"); - WebClient client = WebClient.create("http://localhost:" + port); + .run(withWebTestClient((client) -> { ClientResponse response = client.get().uri("actuator/fail").accept(MediaType.APPLICATION_JSON) .exchange().block(); - Map body = response.bodyToMono(Map.class).block(); - assertThat(body).containsEntry("message", "Epic Fail"); + Map body = getResponseBody(response); + assertThat(body).hasEntrySatisfying("message", + (value) -> assertThat(value).asString().contains("Epic Fail")); assertThat(body).hasEntrySatisfying("trace", (value) -> assertThat(value).asString() .contains("java.lang.IllegalStateException: Epic Fail")); - }); + })); + } + + @Test + void errorEndpointIsUsedWithRestControllerEndpoint() { + this.runner.run(withWebTestClient((client) -> { + ClientResponse response = client.get().uri("actuator/failController").accept(MediaType.APPLICATION_JSON) + .exchange().block(); + Map body = getResponseBody(response); + assertThat(body).hasEntrySatisfying("exception", + (value) -> assertThat(value).asString().contains("IllegalStateException")); + assertThat(body).hasEntrySatisfying("message", + (value) -> assertThat(value).asString().contains("Epic Fail")); + })); + } + + @Test + void errorEndpointIsUsedWithRestControllerEndpointOnBindingError() { + this.runner.run(withWebTestClient((client) -> { + ClientResponse response = client.post().uri("actuator/failController") + .bodyValue(Collections.singletonMap("content", "")).accept(MediaType.APPLICATION_JSON).exchange() + .block(); + Map body = getResponseBody(response); + assertThat(body).hasEntrySatisfying("exception", + (value) -> assertThat(value).asString().contains("MethodArgumentNotValidException")); + assertThat(body).hasEntrySatisfying("message", + (value) -> assertThat(value).asString().contains("Validation failed")); + assertThat(body).hasEntrySatisfying("errors", (value) -> assertThat(value).asList().isNotEmpty()); + })); + } + + private ContextConsumer withWebTestClient(Consumer webClient) { + return (context) -> { + String port = context.getEnvironment().getProperty("local.management.port"); + WebClient client = WebClient.create("http://localhost:" + port); + webClient.accept(client); + }; + } + + @SuppressWarnings("unchecked") + private Map getResponseBody(ClientResponse response) { + return (Map) response.bodyToMono(Map.class).block(); } - @Component @Endpoint(id = "fail") static class FailingEndpoint { @@ -98,4 +149,35 @@ class WebMvcEndpointChildContextConfigurationIntegrationTests { } + @RestControllerEndpoint(id = "failController") + static class FailingControllerEndpoint { + + @GetMapping + String fail() { + throw new IllegalStateException("Epic Fail"); + } + + @PostMapping(produces = "application/json") + @ResponseBody + String bodyValidation(@Valid @RequestBody TestBody body) { + return body.getContent(); + } + + } + + public static class TestBody { + + @NotEmpty + private String content; + + public String getContent() { + return this.content; + } + + public void setContent(String content) { + this.content = content; + } + + } + }