From e44e0c8f1ed10e0cfcd839fa382ff0483831fff4 Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Thu, 14 Dec 2023 11:17:12 +0100 Subject: [PATCH] Remove ErrorAttributes.ERROR_ATTRIBUTE This commit removes the now defunkt `ErrorAttributes.ERROR_ATTRIBUTE` that was introduce to register handled errors as metrics. This has been replaced since 3.0 by a direct support in Spring Framework and had no effect whatsoever since that release. This also updates the documentation to point to the Framework mechanism that replaced it. Fixes gh-33731 --- .../src/docs/asciidoc/web/reactive.adoc | 6 +-- .../src/docs/asciidoc/web/servlet.adoc | 6 +-- .../MyExceptionHandlingController.java | 41 ------------------ .../springmvc/errorhandling/MyController.java | 34 --------------- .../MyExceptionHandlingController.kt | 42 ------------------- .../springmvc/errorhandling/MyController.kt | 33 --------------- .../error/DefaultErrorAttributes.java | 1 - .../web/reactive/error/ErrorAttributes.java | 9 +--- .../servlet/error/DefaultErrorAttributes.java | 3 -- .../web/servlet/error/ErrorAttributes.java | 10 +---- .../error/DefaultErrorAttributesTests.java | 3 -- .../error/DefaultErrorAttributesTests.java | 10 ----- 12 files changed, 6 insertions(+), 192 deletions(-) delete mode 100644 spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/web/reactive/webflux/errorhandling/MyExceptionHandlingController.java delete mode 100644 spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/web/servlet/springmvc/errorhandling/MyController.java delete mode 100644 spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/web/reactive/webflux/errorhandling/MyExceptionHandlingController.kt delete mode 100644 spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/web/servlet/springmvc/errorhandling/MyController.kt diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/reactive.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/reactive.adoc index 82292dbe64aa..6e5a2d1b60bc 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/reactive.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/reactive.adoc @@ -189,10 +189,8 @@ include::code:MyErrorWebExceptionHandler[] For a more complete picture, you can also subclass `DefaultErrorWebExceptionHandler` directly and override specific methods. -In some cases, errors handled at the controller or handler function level are not recorded by the <>. -Applications can ensure that such exceptions are recorded with the request metrics by setting the handled exception as a request attribute: - -include::code:MyExceptionHandlingController[] +In some cases, errors handled at the controller level are not recorded by web observations or the <>. +Applications can ensure that such exceptions are recorded with the observations by {spring-framework-docs}/integration/observability.html#observability.http-server.reactive[setting the handled exception on the observation context]. diff --git a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/servlet.adoc b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/servlet.adoc index 19503eab0796..fbc330fd0ce9 100644 --- a/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/servlet.adoc +++ b/spring-boot-project/spring-boot-docs/src/docs/asciidoc/web/servlet.adoc @@ -355,10 +355,8 @@ include::code:MyControllerAdvice[] In the preceding example, if `MyException` is thrown by a controller defined in the same package as `SomeController`, a JSON representation of the `MyErrorBody` POJO is used instead of the `ErrorAttributes` representation. -In some cases, errors handled at the controller level are not recorded by the <>. -Applications can ensure that such exceptions are recorded with the request metrics by setting the handled exception as a request attribute: - -include::code:MyController[] +In some cases, errors handled at the controller level are not recorded by web observations or the <>. +Applications can ensure that such exceptions are recorded with the observations by {spring-framework-docs}/integration/observability.html#observability.http-server.servlet[setting the handled exception on the observation context]. diff --git a/spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/web/reactive/webflux/errorhandling/MyExceptionHandlingController.java b/spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/web/reactive/webflux/errorhandling/MyExceptionHandlingController.java deleted file mode 100644 index b1913940a84e..000000000000 --- a/spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/web/reactive/webflux/errorhandling/MyExceptionHandlingController.java +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright 2012-2021 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.docs.web.reactive.webflux.errorhandling; - -import org.springframework.boot.web.reactive.error.ErrorAttributes; -import org.springframework.stereotype.Controller; -import org.springframework.web.bind.annotation.ExceptionHandler; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.reactive.result.view.Rendering; -import org.springframework.web.server.ServerWebExchange; - -@Controller -public class MyExceptionHandlingController { - - @GetMapping("/profile") - public Rendering userProfile() { - // ... - throw new IllegalStateException(); - } - - @ExceptionHandler(IllegalStateException.class) - public Rendering handleIllegalState(ServerWebExchange exchange, IllegalStateException exc) { - exchange.getAttributes().putIfAbsent(ErrorAttributes.ERROR_ATTRIBUTE, exc); - return Rendering.view("errorView").modelAttribute("message", exc.getMessage()).build(); - } - -} diff --git a/spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/web/servlet/springmvc/errorhandling/MyController.java b/spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/web/servlet/springmvc/errorhandling/MyController.java deleted file mode 100644 index e93f0ba92b43..000000000000 --- a/spring-boot-project/spring-boot-docs/src/main/java/org/springframework/boot/docs/web/servlet/springmvc/errorhandling/MyController.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2012-2021 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.docs.web.servlet.springmvc.errorhandling; - -import jakarta.servlet.http.HttpServletRequest; - -import org.springframework.boot.web.servlet.error.ErrorAttributes; -import org.springframework.stereotype.Controller; -import org.springframework.web.bind.annotation.ExceptionHandler; - -@Controller -public class MyController { - - @ExceptionHandler(CustomException.class) - String handleCustomException(HttpServletRequest request, CustomException ex) { - request.setAttribute(ErrorAttributes.ERROR_ATTRIBUTE, ex); - return "errorView"; - } - -} diff --git a/spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/web/reactive/webflux/errorhandling/MyExceptionHandlingController.kt b/spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/web/reactive/webflux/errorhandling/MyExceptionHandlingController.kt deleted file mode 100644 index 26dfa251d465..000000000000 --- a/spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/web/reactive/webflux/errorhandling/MyExceptionHandlingController.kt +++ /dev/null @@ -1,42 +0,0 @@ -/* - * Copyright 2012-2022 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.docs.web.reactive.webflux.errorhandling - -import org.springframework.boot.web.reactive.error.ErrorAttributes -import org.springframework.stereotype.Controller -import org.springframework.web.bind.annotation.ExceptionHandler -import org.springframework.web.bind.annotation.GetMapping -import org.springframework.web.reactive.result.view.Rendering -import org.springframework.web.server.ServerWebExchange - -@Suppress("UNUSED_PARAMETER") -@Controller -class MyExceptionHandlingController { - - @GetMapping("/profile") - fun userProfile(): Rendering { - // ... - throw IllegalStateException() - } - - @ExceptionHandler(IllegalStateException::class) - fun handleIllegalState(exchange: ServerWebExchange, exc: IllegalStateException): Rendering { - exchange.attributes.putIfAbsent(ErrorAttributes.ERROR_ATTRIBUTE, exc) - return Rendering.view("errorView").modelAttribute("message", exc.message ?: "").build() - } - -} diff --git a/spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/web/servlet/springmvc/errorhandling/MyController.kt b/spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/web/servlet/springmvc/errorhandling/MyController.kt deleted file mode 100644 index deead1e60da0..000000000000 --- a/spring-boot-project/spring-boot-docs/src/main/kotlin/org/springframework/boot/docs/web/servlet/springmvc/errorhandling/MyController.kt +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright 2012-2022 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. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.boot.docs.web.servlet.springmvc.errorhandling - -import jakarta.servlet.http.HttpServletRequest -import org.springframework.boot.web.servlet.error.ErrorAttributes -import org.springframework.stereotype.Controller -import org.springframework.web.bind.annotation.ExceptionHandler - -@Controller -class MyController { - - @ExceptionHandler(CustomException::class) - fun handleCustomException(request: HttpServletRequest, ex: CustomException?): String { - request.setAttribute(ErrorAttributes.ERROR_ATTRIBUTE, ex) - return "errorView" - } - -} diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java index d55366b53c4a..354b46988a8f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributes.java @@ -152,7 +152,6 @@ private void handleException(Map errorAttributes, Throwable erro @Override public Throwable getError(ServerRequest request) { Optional error = request.attribute(ERROR_INTERNAL_ATTRIBUTE); - error.ifPresent((value) -> request.attributes().putIfAbsent(ErrorAttributes.ERROR_ATTRIBUTE, value)); return (Throwable) error .orElseThrow(() -> new IllegalStateException("Missing exception attribute in ServerWebExchange")); } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java index 3c9a1d5567ba..aab94b4e31ec 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/reactive/error/ErrorAttributes.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2023 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. @@ -34,13 +34,6 @@ */ public interface ErrorAttributes { - /** - * Name of the {@link ServerRequest#attribute(String) request attribute} holding the - * error resolved by the {@code ErrorAttributes} implementation. - * @since 2.5.0 - */ - String ERROR_ATTRIBUTE = ErrorAttributes.class.getName() + ".error"; - /** * Return a {@link Map} of the error attributes. The map can be used as the model of * an error page, or returned as a {@link ServerResponse} body. diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java index 21e6bd068fe0..ef02be96709b 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributes.java @@ -216,9 +216,6 @@ public Throwable getError(WebRequest webRequest) { if (exception == null) { exception = getAttribute(webRequest, RequestDispatcher.ERROR_EXCEPTION); } - // store the exception in a well-known attribute to make it available to metrics - // instrumentation. - webRequest.setAttribute(ErrorAttributes.ERROR_ATTRIBUTE, exception, WebRequest.SCOPE_REQUEST); return exception; } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java index 898363885eac..3f2fc64d1e12 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/error/ErrorAttributes.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2023 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. @@ -34,14 +34,6 @@ */ public interface ErrorAttributes { - /** - * Name of the {@link jakarta.servlet.http.HttpServletRequest#getAttribute(String) - * request attribute} holding the error resolved by the {@code ErrorAttributes} - * implementation. - * @since 2.5.0 - */ - String ERROR_ATTRIBUTE = ErrorAttributes.class.getName() + ".error"; - /** * Returns a {@link Map} of the error attributes. The map can be used as the model of * an error page {@link ModelAndView}, or returned as a diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java index 06633045c35b..9b3d63375c5f 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/reactive/error/DefaultErrorAttributesTests.java @@ -160,7 +160,6 @@ void includeException() { Map attributes = this.errorAttributes.getErrorAttributes(serverRequest, ErrorAttributeOptions.of(Include.EXCEPTION, Include.MESSAGE)); assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error); - assertThat(serverRequest.attribute(ErrorAttributes.ERROR_ATTRIBUTE)).containsSame(error); assertThat(attributes).containsEntry("exception", RuntimeException.class.getName()); assertThat(attributes).containsEntry("message", "Test"); } @@ -178,7 +177,6 @@ void processResponseStatusException() { assertThat(attributes).containsEntry("message", "invalid request"); assertThat(attributes).containsEntry("exception", RuntimeException.class.getName()); assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error); - assertThat(serverRequest.attribute(ErrorAttributes.ERROR_ATTRIBUTE)).containsSame(error); } @Test @@ -194,7 +192,6 @@ void processResponseStatusExceptionWithNoNestedCause() { assertThat(attributes).containsEntry("message", "could not process request"); assertThat(attributes).containsEntry("exception", ResponseStatusException.class.getName()); assertThat(this.errorAttributes.getError(serverRequest)).isSameAs(error); - assertThat(serverRequest.attribute(ErrorAttributes.ERROR_ATTRIBUTE)).containsSame(error); } @Test diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java index 71405232b66e..fe6848d0cb3f 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/error/DefaultErrorAttributesTests.java @@ -88,8 +88,6 @@ void mvcError() { Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(ex); - assertThat(this.webRequest.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE, WebRequest.SCOPE_REQUEST)) - .isSameAs(ex); assertThat(modelAndView).isNull(); assertThat(attributes).doesNotContainKey("exception"); assertThat(attributes).containsEntry("message", "Test"); @@ -102,8 +100,6 @@ void servletErrorWithMessage() { Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(ex); - assertThat(this.webRequest.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE, WebRequest.SCOPE_REQUEST)) - .isSameAs(ex); assertThat(attributes).doesNotContainKey("exception"); assertThat(attributes).containsEntry("message", "Test"); } @@ -115,8 +111,6 @@ void servletErrorWithoutMessage() { Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, ErrorAttributeOptions.defaults()); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(ex); - assertThat(this.webRequest.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE, WebRequest.SCOPE_REQUEST)) - .isSameAs(ex); assertThat(attributes).doesNotContainKey("exception"); assertThat(attributes).doesNotContainKey("message"); } @@ -166,8 +160,6 @@ void unwrapServletException() { Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(wrapped); - assertThat(this.webRequest.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE, WebRequest.SCOPE_REQUEST)) - .isSameAs(wrapped); assertThat(attributes).doesNotContainKey("exception"); assertThat(attributes).containsEntry("message", "Test"); } @@ -179,8 +171,6 @@ void getError() { Map attributes = this.errorAttributes.getErrorAttributes(this.webRequest, ErrorAttributeOptions.of(Include.MESSAGE)); assertThat(this.errorAttributes.getError(this.webRequest)).isSameAs(error); - assertThat(this.webRequest.getAttribute(ErrorAttributes.ERROR_ATTRIBUTE, WebRequest.SCOPE_REQUEST)) - .isSameAs(error); assertThat(attributes).doesNotContainKey("exception"); assertThat(attributes).containsEntry("message", "Test error"); }