From 6b8d08a6e3f32d55a30a059595823c64aab8e8dd Mon Sep 17 00:00:00 2001 From: Scott Frederick Date: Tue, 5 May 2020 15:49:13 -0500 Subject: [PATCH] Handle exceptions in management context Prior to this commit, details about an exception would get dropped when the management context was separate from the application context and an actuator endpoint threw a binding exception. This commit adds some logic to capture the exception so the management context error handlers can add the appropriate attributes to the error response. Fixes gh-21036 --- .../CompositeHandlerExceptionResolver.java | 15 +- ...ompositeHandlerExceptionResolverTests.java | 9 +- ...dContextConfigurationIntegrationTests.java | 128 +++++++++++++++--- 3 files changed, 127 insertions(+), 25 deletions(-) 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 6d7b7121e3..3fc31ffc59 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 4fdc3f0e8a..129d7dd462 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 d82ea973b7..5ed26b0f28 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 @@ -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. @@ -16,6 +16,13 @@ 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; import org.springframework.boot.actuate.autoconfigure.endpoint.EndpointAutoConfiguration; @@ -23,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; @@ -41,29 +54,75 @@ import static org.assertj.core.api.Assertions.assertThat; * Integration tests for {@link WebMvcEndpointChildContextConfiguration}. * * @author Phillip Webb + * @author Scott Frederick */ class WebMvcEndpointChildContextConfigurationIntegrationTests { + final WebApplicationContextRunner contextRunner = new WebApplicationContextRunner( + AnnotationConfigServletWebServerApplicationContext::new) + .withConfiguration(AutoConfigurations.of(ServletWebServerFactoryAutoConfiguration.class, + ManagementContextAutoConfiguration.class, ServletManagementContextAutoConfiguration.class, + WebEndpointAutoConfiguration.class, EndpointAutoConfiguration.class, + DispatcherServletAutoConfiguration.class, ErrorMvcAutoConfiguration.class)) + .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"); + @Test // gh-17938 - void errorPageAndErrorControllerAreUsed() { - new WebApplicationContextRunner(AnnotationConfigServletWebServerApplicationContext::new) - .withConfiguration(AutoConfigurations.of(ManagementContextAutoConfiguration.class, - ServletWebServerFactoryAutoConfiguration.class, 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=*") - .run((context) -> { - String port = context.getEnvironment().getProperty("local.management.port"); - WebClient client = WebClient.create("http://localhost:" + port); - ClientResponse response = client.get().uri("actuator/fail").accept(MediaType.APPLICATION_JSON) - .exchange().block(); - assertThat(response.bodyToMono(String.class).block()).contains("message\":\"Epic Fail"); - }); + void errorEndpointIsUsedWithEndpoint() { + this.contextRunner.run(withWebTestClient((client) -> { + ClientResponse response = client.get().uri("actuator/fail").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 errorEndpointIsUsedWithRestControllerEndpoint() { + this.contextRunner.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.contextRunner.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 { @@ -74,4 +133,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; + } + + } + }