diff --git a/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java b/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java index d760c8676ca..5093a28be8c 100644 --- a/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java +++ b/spring-web/src/main/java/org/springframework/web/server/handler/ResponseStatusExceptionHandler.java @@ -20,22 +20,15 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import reactor.core.publisher.Mono; -import org.springframework.core.annotation.AnnotatedElementUtils; import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.lang.Nullable; -import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.server.ResponseStatusException; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebExceptionHandler; /** - * Handle instances of {@link ResponseStatusException}, or of exceptions annotated - * with {@link ResponseStatus @ResponseStatus}, by extracting the - * {@code HttpStatus} from them and updating the status of the response. - * - *

If the response is already committed, the error remains unresolved and is - * propagated. + * Handle {@link ResponseStatusException} by setting the response status. * * @author Rossen Stoyanchev * @author Sebastien Deleuze @@ -48,7 +41,7 @@ public class ResponseStatusExceptionHandler implements WebExceptionHandler { @Override public Mono handle(ServerWebExchange exchange, Throwable ex) { - HttpStatus status = resolveHttpStatus(ex); + HttpStatus status = resolveStatus(ex); if (status != null && exchange.getResponse().setStatusCode(status)) { if (status.is5xxServerError()) { logger.error(buildMessage(exchange.getRequest(), ex)); @@ -64,24 +57,34 @@ public class ResponseStatusExceptionHandler implements WebExceptionHandler { return Mono.error(ex); } + private String buildMessage(ServerHttpRequest request, Throwable ex) { + return "Failed to handle request [" + request.getMethod() + " " + request.getURI() + "]: " + ex.getMessage(); + } + @Nullable - private HttpStatus resolveHttpStatus(Throwable ex) { + private HttpStatus resolveStatus(Throwable ex) { + HttpStatus status = determineStatus(ex); + if (status == null) { + Throwable cause = ex.getCause(); + if (cause != null) { + status = resolveStatus(cause); + } + } + return status; + } + + /** + * Determine the HTTP status implied by the given exception. + * @param ex the exception to introspect + * @return the associated HTTP status, if any + * @since 5.0.5 + */ + @Nullable + protected HttpStatus determineStatus(Throwable ex) { if (ex instanceof ResponseStatusException) { return ((ResponseStatusException) ex).getStatus(); } - ResponseStatus status = AnnotatedElementUtils.findMergedAnnotation(ex.getClass(), ResponseStatus.class); - if (status != null) { - return status.code(); - } - if (ex.getCause() != null) { - return resolveHttpStatus(ex.getCause()); - } return null; } - private String buildMessage(ServerHttpRequest request, Throwable ex) { - return "Failed to handle request [" + request.getMethod() + " " - + request.getURI() + "]: " + ex.getMessage(); - } - -} \ No newline at end of file +} diff --git a/spring-web/src/test/java/org/springframework/web/server/handler/ResponseStatusExceptionHandlerTests.java b/spring-web/src/test/java/org/springframework/web/server/handler/ResponseStatusExceptionHandlerTests.java index d81f1cb138e..3224cefc434 100644 --- a/spring-web/src/test/java/org/springframework/web/server/handler/ResponseStatusExceptionHandlerTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/handler/ResponseStatusExceptionHandlerTests.java @@ -18,6 +18,7 @@ package org.springframework.web.server.handler; import java.time.Duration; +import org.junit.Before; import org.junit.Test; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -25,21 +26,31 @@ import reactor.test.StepVerifier; import org.springframework.http.HttpStatus; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.web.test.server.MockServerWebExchange; -import org.springframework.web.bind.annotation.ResponseStatus; import org.springframework.web.server.ResponseStatusException; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertSame; +import static org.junit.Assert.*; /** * Unit tests for {@link ResponseStatusExceptionHandler}. + * * @author Rossen Stoyanchev + * @author Juergen Hoeller */ public class ResponseStatusExceptionHandlerTests { - private final ResponseStatusExceptionHandler handler = new ResponseStatusExceptionHandler(); + protected final MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); - private final MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/")); + protected ResponseStatusExceptionHandler handler; + + + @Before + public void setup() { + this.handler = createResponseStatusExceptionHandler(); + } + + protected ResponseStatusExceptionHandler createResponseStatusExceptionHandler() { + return new ResponseStatusExceptionHandler(); + } @Test @@ -49,13 +60,6 @@ public class ResponseStatusExceptionHandlerTests { assertEquals(HttpStatus.BAD_REQUEST, this.exchange.getResponse().getStatusCode()); } - @Test - public void handleAnnotatedException() { - Throwable ex = new CustomException(); - this.handler.handle(this.exchange, ex).block(Duration.ofSeconds(5)); - assertEquals(HttpStatus.I_AM_A_TEAPOT, this.exchange.getResponse().getStatusCode()); - } - @Test public void handleNestedResponseStatusException() { Throwable ex = new Exception(new ResponseStatusException(HttpStatus.BAD_REQUEST, "")); @@ -63,13 +67,6 @@ public class ResponseStatusExceptionHandlerTests { assertEquals(HttpStatus.BAD_REQUEST, this.exchange.getResponse().getStatusCode()); } - @Test - public void handleNestedAnnotatedException() { - Throwable ex = new Exception(new CustomException()); - this.handler.handle(this.exchange, ex).block(Duration.ofSeconds(5)); - assertEquals(HttpStatus.I_AM_A_TEAPOT, this.exchange.getResponse().getStatusCode()); - } - @Test public void unresolvedException() { Throwable expected = new IllegalStateException(); @@ -77,7 +74,7 @@ public class ResponseStatusExceptionHandlerTests { StepVerifier.create(mono).consumeErrorWith(actual -> assertSame(expected, actual)).verify(); } - @Test // SPR-16231 + @Test // SPR-16231 public void responseCommitted() { Throwable ex = new ResponseStatusException(HttpStatus.INTERNAL_SERVER_ERROR, "Oops"); this.exchange.getResponse().setStatusCode(HttpStatus.CREATED); @@ -86,9 +83,4 @@ public class ResponseStatusExceptionHandlerTests { StepVerifier.create(mono).consumeErrorWith(actual -> assertSame(ex, actual)).verify(); } - - @ResponseStatus(HttpStatus.I_AM_A_TEAPOT) - private static class CustomException extends Exception { - } - } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java index 5715552b376..c636d929a28 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/config/WebFluxConfigurationSupport.java @@ -52,6 +52,7 @@ import org.springframework.web.reactive.function.server.support.HandlerFunctionA import org.springframework.web.reactive.function.server.support.RouterFunctionMapping; import org.springframework.web.reactive.function.server.support.ServerResponseResultHandler; import org.springframework.web.reactive.handler.AbstractHandlerMapping; +import org.springframework.web.reactive.handler.WebFluxResponseStatusExceptionHandler; import org.springframework.web.reactive.result.SimpleHandlerAdapter; import org.springframework.web.reactive.result.method.annotation.ArgumentResolverConfigurer; import org.springframework.web.reactive.result.method.annotation.RequestMappingHandlerAdapter; @@ -62,7 +63,6 @@ import org.springframework.web.reactive.result.view.ViewResolutionResultHandler; import org.springframework.web.reactive.result.view.ViewResolver; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebExceptionHandler; -import org.springframework.web.server.handler.ResponseStatusExceptionHandler; import org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver; import org.springframework.web.server.i18n.LocaleContextResolver; @@ -108,7 +108,7 @@ public class WebFluxConfigurationSupport implements ApplicationContextAware { @Bean @Order(0) public WebExceptionHandler responseStatusExceptionHandler() { - return new ResponseStatusExceptionHandler(); + return new WebFluxResponseStatusExceptionHandler(); } @Bean diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultHandlerStrategiesBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultHandlerStrategiesBuilder.java index 697425cb4c6..824ad3855ab 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultHandlerStrategiesBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/server/DefaultHandlerStrategiesBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -25,10 +25,10 @@ import org.springframework.http.codec.HttpMessageReader; import org.springframework.http.codec.HttpMessageWriter; import org.springframework.http.codec.ServerCodecConfigurer; import org.springframework.util.Assert; +import org.springframework.web.reactive.handler.WebFluxResponseStatusExceptionHandler; import org.springframework.web.reactive.result.view.ViewResolver; import org.springframework.web.server.WebExceptionHandler; import org.springframework.web.server.WebFilter; -import org.springframework.web.server.handler.ResponseStatusExceptionHandler; import org.springframework.web.server.i18n.AcceptHeaderLocaleContextResolver; import org.springframework.web.server.i18n.LocaleContextResolver; @@ -58,7 +58,7 @@ class DefaultHandlerStrategiesBuilder implements HandlerStrategies.Builder { public void defaultConfiguration() { this.codecConfigurer.registerDefaults(true); - this.exceptionHandlers.add(new ResponseStatusExceptionHandler()); + this.exceptionHandlers.add(new WebFluxResponseStatusExceptionHandler()); this.localeContextResolver = new AcceptHeaderLocaleContextResolver(); } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/handler/WebFluxResponseStatusExceptionHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/WebFluxResponseStatusExceptionHandler.java new file mode 100644 index 00000000000..2bc38d9fd89 --- /dev/null +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/handler/WebFluxResponseStatusExceptionHandler.java @@ -0,0 +1,54 @@ +/* + * Copyright 2002-2018 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 + * + * http://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.web.reactive.handler; + +import org.springframework.core.annotation.AnnotatedElementUtils; +import org.springframework.http.HttpStatus; +import org.springframework.lang.Nullable; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.server.handler.ResponseStatusExceptionHandler; + +/** + * Common WebFlux exception handler that detects instances of + * {@link org.springframework.web.server.ResponseStatusException} + * (inherited from the base class) as well as exceptions annotated with + * {@link ResponseStatus @ResponseStatus} by determining the HTTP status + * for them and updating the status of the response accordingly. + * + *

If the response is already committed, the error remains unresolved + * and is propagated. + * + * @author Juergen Hoeller + * @author Rossen Stoyanchev + * @since 5.0.5 + */ +public class WebFluxResponseStatusExceptionHandler extends ResponseStatusExceptionHandler { + + @Override + @Nullable + protected HttpStatus determineStatus(Throwable ex) { + HttpStatus status = super.determineStatus(ex); + if (status == null) { + ResponseStatus ann = AnnotatedElementUtils.findMergedAnnotation(ex.getClass(), ResponseStatus.class); + if (ann != null) { + status = ann.code(); + } + } + return status; + } + +} diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/handler/WebFluxResponseStatusExceptionHandlerTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/WebFluxResponseStatusExceptionHandlerTests.java new file mode 100644 index 00000000000..025b7715d12 --- /dev/null +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/handler/WebFluxResponseStatusExceptionHandlerTests.java @@ -0,0 +1,65 @@ +/* + * Copyright 2002-2018 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 + * + * http://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.web.reactive.handler; + +import java.time.Duration; + +import org.junit.Test; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; +import org.springframework.web.reactive.handler.WebFluxResponseStatusExceptionHandler; +import org.springframework.web.server.handler.ResponseStatusExceptionHandler; +import org.springframework.web.server.handler.ResponseStatusExceptionHandlerTests; + +import static org.junit.Assert.*; + +/** + * Unit tests for {@link WebFluxResponseStatusExceptionHandler}. + * + * @author Juergen Hoeller + * @author Rossen Stoyanchev + */ +public class WebFluxResponseStatusExceptionHandlerTests extends ResponseStatusExceptionHandlerTests { + + @Override + protected ResponseStatusExceptionHandler createResponseStatusExceptionHandler() { + return new WebFluxResponseStatusExceptionHandler(); + } + + + @Test + public void handleAnnotatedException() { + Throwable ex = new CustomException(); + this.handler.handle(this.exchange, ex).block(Duration.ofSeconds(5)); + assertEquals(HttpStatus.I_AM_A_TEAPOT, this.exchange.getResponse().getStatusCode()); + } + + @Test + public void handleNestedAnnotatedException() { + Throwable ex = new Exception(new CustomException()); + this.handler.handle(this.exchange, ex).block(Duration.ofSeconds(5)); + assertEquals(HttpStatus.I_AM_A_TEAPOT, this.exchange.getResponse().getStatusCode()); + } + + + @SuppressWarnings("serial") + @ResponseStatus(HttpStatus.I_AM_A_TEAPOT) + private static class CustomException extends Exception { + } + +}