diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatchExceptionHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatchExceptionHandler.java new file mode 100644 index 000000000000..017e8bfda042 --- /dev/null +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatchExceptionHandler.java @@ -0,0 +1,40 @@ +/* + * Copyright 2002-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.web.reactive; + +import reactor.core.publisher.Mono; + +import org.springframework.web.server.ServerWebExchange; + +/** + * Contract to map a {@link Throwable} to a {@link HandlerResult}. + * + * @author Rossen Stoyanchev + * @since 6.0 + */ +public interface DispatchExceptionHandler { + + /** + * Handler the given exception and resolve it to {@link HandlerResult} that + * can be used for rendering an HTTP response. + * @param exchange the current exchange + * @param ex the exception to handle + * @return a {@code Mono} that emits a {@code HandlerResult} or the original exception + */ + Mono handleError(ServerWebExchange exchange, Throwable ex); + +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java index ee2f4603f8d3..f7e6ecfcf21e 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java @@ -150,8 +150,8 @@ public Mono handle(ServerWebExchange exchange) { .concatMap(mapping -> mapping.getHandler(exchange)) .next() .switchIfEmpty(createNotFoundError()) - .flatMap(handler -> invokeHandler(exchange, handler)) - .flatMap(result -> handleResult(exchange, result)); + .onErrorResume(ex -> handleDispatchError(exchange, ex)) + .flatMap(handler -> handleRequestWith(exchange, handler)); } private Mono createNotFoundError() { @@ -161,14 +161,27 @@ private Mono createNotFoundError() { }); } - private Mono invokeHandler(ServerWebExchange exchange, Object handler) { + private Mono handleDispatchError(ServerWebExchange exchange, Throwable ex) { + Mono resultMono = Mono.error(ex); + if (this.handlerAdapters != null) { + for (HandlerAdapter adapter : this.handlerAdapters) { + if (adapter instanceof DispatchExceptionHandler exceptionHandler) { + resultMono = resultMono.onErrorResume(ex2 -> exceptionHandler.handleError(exchange, ex2)); + } + } + } + return resultMono.flatMap(result -> handleResult(exchange, result)); + } + + private Mono handleRequestWith(ServerWebExchange exchange, Object handler) { if (ObjectUtils.nullSafeEquals(exchange.getResponse().getStatusCode(), HttpStatus.FORBIDDEN)) { return Mono.empty(); // CORS rejection } if (this.handlerAdapters != null) { - for (HandlerAdapter handlerAdapter : this.handlerAdapters) { - if (handlerAdapter.supports(handler)) { - return handlerAdapter.handle(exchange, handler); + for (HandlerAdapter adapter : this.handlerAdapters) { + if (adapter.supports(handler)) { + return adapter.handle(exchange, handler) + .flatMap(result -> handleResult(exchange, result)); } } } @@ -179,11 +192,10 @@ private Mono handleResult(ServerWebExchange exchange, HandlerResult result return getResultHandler(result).handleResult(exchange, result) .checkpoint("Handler " + result.getHandler() + " [DispatcherHandler]") .onErrorResume(ex -> - result.applyExceptionHandler(ex).flatMap(exResult -> { - String text = "Exception handler " + exResult.getHandler() + - ", error=\"" + ex.getMessage() + "\" [DispatcherHandler]"; - return getResultHandler(exResult).handleResult(exchange, exResult).checkpoint(text); - })); + result.applyExceptionHandler(ex).flatMap(exResult -> + getResultHandler(exResult).handleResult(exchange, exResult) + .checkpoint("Exception handler " + exResult.getHandler() + ", " + + "error=\"" + ex.getMessage() + "\" [DispatcherHandler]"))); } private HandlerResultHandler getResultHandler(HandlerResult handlerResult) { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/HandlerAdapter.java b/spring-webflux/src/main/java/org/springframework/web/reactive/HandlerAdapter.java index 7361b54d9053..7504b9851405 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/HandlerAdapter.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/HandlerAdapter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-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. @@ -26,6 +26,14 @@ * Contract that decouples the {@link DispatcherHandler} from the details of * invoking a handler and makes it possible to support any handler type. * + *

A {@code HandlerAdapter} can implement {@link DispatchExceptionHandler} + * if it wants to handle an exception that occured before the request is mapped + * to a handler. This allows the {@code HandlerAdapter} to expose a consistent + * exception handling mechanism for any request handling error. + * In Reactive Streams terms, {@link #handle} processes the onNext, while + * {@link DispatchExceptionHandler#handleError} processes the onError signal + * from the upstream. + * * @author Rossen Stoyanchev * @author Sebastien Deleuze * @since 5.0 diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java index e8ff100c455d..50a73f1a2dbf 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ControllerMethodResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-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. @@ -330,38 +330,47 @@ private InvocableHandlerMethod createAttributeMethod(Object bean, Method method) } /** - * Find an {@code @ExceptionHandler} method in {@code @ControllerAdvice} - * components or in the controller of the given {@code @RequestMapping} method. + * Look for an {@code @ExceptionHandler} method within the class of the given + * controller method, and also within {@code @ControllerAdvice} classes that + * are applicable to the class of the given controller method. + * @param ex the exception to find a handler for + * @param handlerMethod the controller method that raised the exception, or + * if {@code null}, check only {@code @ControllerAdvice} classes. */ @Nullable - public InvocableHandlerMethod getExceptionHandlerMethod(Throwable ex, HandlerMethod handlerMethod) { - Class handlerType = handlerMethod.getBeanType(); - - // Controller-local first... - Object targetBean = handlerMethod.getBean(); - Method targetMethod = this.exceptionHandlerCache - .computeIfAbsent(handlerType, ExceptionHandlerMethodResolver::new) - .resolveMethodByThrowable(ex); + public InvocableHandlerMethod getExceptionHandlerMethod(Throwable ex, @Nullable HandlerMethod handlerMethod) { + + Class handlerType = (handlerMethod != null ? handlerMethod.getBeanType() : null); + Object exceptionHandlerObject = null; + Method exceptionHandlerMethod = null; + + if (handlerType != null) { + // Controller-local first... + exceptionHandlerObject = handlerMethod.getBean(); + exceptionHandlerMethod = this.exceptionHandlerCache + .computeIfAbsent(handlerType, ExceptionHandlerMethodResolver::new) + .resolveMethodByThrowable(ex); + } - if (targetMethod == null) { + if (exceptionHandlerMethod == null) { // Global exception handlers... for (Map.Entry entry : this.exceptionHandlerAdviceCache.entrySet()) { ControllerAdviceBean advice = entry.getKey(); if (advice.isApplicableToBeanType(handlerType)) { - targetBean = advice.resolveBean(); - targetMethod = entry.getValue().resolveMethodByThrowable(ex); - if (targetMethod != null) { + exceptionHandlerMethod = entry.getValue().resolveMethodByThrowable(ex); + if (exceptionHandlerMethod != null) { + exceptionHandlerObject = advice.resolveBean(); break; } } } } - if (targetMethod == null) { + if (exceptionHandlerObject == null || exceptionHandlerMethod == null) { return null; } - InvocableHandlerMethod invocable = new InvocableHandlerMethod(targetBean, targetMethod); + InvocableHandlerMethod invocable = new InvocableHandlerMethod(exceptionHandlerObject, exceptionHandlerMethod); invocable.setArgumentResolvers(this.exceptionHandlerResolvers); return invocable; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java index ceb8085ed61b..0bbbb7146f41 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/RequestMappingHandlerAdapter.java @@ -38,6 +38,7 @@ import org.springframework.web.bind.support.WebBindingInitializer; import org.springframework.web.method.HandlerMethod; import org.springframework.web.reactive.BindingContext; +import org.springframework.web.reactive.DispatchExceptionHandler; import org.springframework.web.reactive.HandlerAdapter; import org.springframework.web.reactive.HandlerMapping; import org.springframework.web.reactive.HandlerResult; @@ -52,7 +53,8 @@ * @author Rossen Stoyanchev * @since 5.0 */ -public class RequestMappingHandlerAdapter implements HandlerAdapter, ApplicationContextAware, InitializingBean { +public class RequestMappingHandlerAdapter + implements HandlerAdapter, DispatchExceptionHandler, ApplicationContextAware, InitializingBean { private static final Log logger = LogFactory.getLog(RequestMappingHandlerAdapter.class); @@ -193,7 +195,7 @@ public Mono handle(ServerWebExchange exchange, Object handler) { InvocableHandlerMethod invocableMethod = this.methodResolver.getRequestMappingMethod(handlerMethod); Function> exceptionHandler = - ex -> handleException(ex, handlerMethod, bindingContext, exchange); + ex -> handleException(exchange, ex, handlerMethod, bindingContext); return this.modelInitializer .initModel(handlerMethod, bindingContext, exchange) @@ -203,8 +205,9 @@ public Mono handle(ServerWebExchange exchange, Object handler) { .onErrorResume(exceptionHandler); } - private Mono handleException(Throwable exception, HandlerMethod handlerMethod, - BindingContext bindingContext, ServerWebExchange exchange) { + private Mono handleException( + ServerWebExchange exchange, Throwable exception, + @Nullable HandlerMethod handlerMethod, @Nullable BindingContext bindingContext) { Assert.state(this.methodResolver != null, "Not initialized"); @@ -212,14 +215,21 @@ private Mono handleException(Throwable exception, HandlerMethod h exchange.getAttributes().remove(HandlerMapping.PRODUCIBLE_MEDIA_TYPES_ATTRIBUTE); exchange.getResponse().getHeaders().clearContentHeaders(); - InvocableHandlerMethod invocable = this.methodResolver.getExceptionHandlerMethod(exception, handlerMethod); + InvocableHandlerMethod invocable = + this.methodResolver.getExceptionHandlerMethod(exception, handlerMethod); + if (invocable != null) { ArrayList exceptions = new ArrayList<>(); try { if (logger.isDebugEnabled()) { logger.debug(exchange.getLogPrefix() + "Using @ExceptionHandler " + invocable); } - bindingContext.getModel().asMap().clear(); + if (bindingContext != null) { + bindingContext.getModel().asMap().clear(); + } + else { + bindingContext = new BindingContext(); + } // Expose causes as provided arguments as well Throwable exToExpose = exception; @@ -245,4 +255,9 @@ private Mono handleException(Throwable exception, HandlerMethod h return Mono.error(exception); } + @Override + public Mono handleError(ServerWebExchange exchange, Throwable ex) { + return handleException(exchange, ex, null, null); + } + } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingExceptionHandlingIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingExceptionHandlingIntegrationTests.java index a090f3c07f34..e7df37b4b916 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingExceptionHandlingIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingExceptionHandlingIntegrationTests.java @@ -20,6 +20,7 @@ import java.util.Collections; import java.util.Map; +import org.junit.jupiter.api.Test; import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -31,12 +32,15 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.annotation.ControllerAdvice; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.reactive.config.EnableWebFlux; import org.springframework.web.testfixture.http.server.reactive.bootstrap.HttpServer; +import org.springframework.web.testfixture.http.server.reactive.bootstrap.ReactorHttpServer; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -47,7 +51,7 @@ * @author Rossen Stoyanchev * @author Juergen Hoeller */ -class RequestMappingExceptionHandlingIntegrationTests extends AbstractRequestMappingIntegrationTests { +public class RequestMappingExceptionHandlingIntegrationTests extends AbstractRequestMappingIntegrationTests { @Override protected ApplicationContext initApplicationContext() { @@ -61,38 +65,38 @@ protected ApplicationContext initApplicationContext() { @ParameterizedHttpServerTest void thrownException(HttpServer httpServer) throws Exception { startServer(httpServer); - doTest("/thrown-exception", "Recovered from error: State"); } @ParameterizedHttpServerTest void thrownExceptionWithCause(HttpServer httpServer) throws Exception { startServer(httpServer); - doTest("/thrown-exception-with-cause", "Recovered from error: State"); } @ParameterizedHttpServerTest void thrownExceptionWithCauseToHandle(HttpServer httpServer) throws Exception { startServer(httpServer); - doTest("/thrown-exception-with-cause-to-handle", "Recovered from error: IO"); } @ParameterizedHttpServerTest void errorBeforeFirstItem(HttpServer httpServer) throws Exception { startServer(httpServer); - doTest("/mono-error", "Recovered from error: Argument"); } + private void doTest(String url, String expected) throws Exception { + assertThat(performGet(url, new HttpHeaders(), String.class).getBody()).isEqualTo(expected); + } + @ParameterizedHttpServerTest // SPR-16051 void exceptionAfterSeveralItems(HttpServer httpServer) throws Exception { startServer(httpServer); - assertThatExceptionOfType(Throwable.class).isThrownBy(() -> - performGet("/SPR-16051", new HttpHeaders(), String.class).getBody()) - .withMessageStartingWith("Error while extracting response"); + assertThatExceptionOfType(Throwable.class) + .isThrownBy(() -> performGet("/SPR-16051", new HttpHeaders(), String.class)) + .withMessageStartingWith("Error while extracting response"); } @ParameterizedHttpServerTest // SPR-16318 @@ -101,20 +105,50 @@ void exceptionFromMethodWithProducesCondition(HttpServer httpServer) throws Exce HttpHeaders headers = new HttpHeaders(); headers.add("Accept", "text/plain, application/problem+json"); - assertThatExceptionOfType(HttpStatusCodeException.class).isThrownBy(() -> - performGet("/SPR-16318", headers, String.class).getBody()) - .satisfies(ex -> { - assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); - assertThat(ex.getResponseHeaders().getContentType().toString()).isEqualTo("application/problem+json"); - assertThat(ex.getResponseBodyAsString()).isEqualTo("{\"reason\":\"error\"}"); - }); + assertThatExceptionOfType(HttpStatusCodeException.class) + .isThrownBy(() -> performGet("/SPR-16318", headers, String.class)) + .satisfies(ex -> { + assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR); + assertThat(ex.getResponseHeaders().getContentType().toString()).isEqualTo("application/problem+json"); + assertThat(ex.getResponseBodyAsString()).isEqualTo("{\"reason\":\"error\"}"); + }); } - private void doTest(String url, String expected) throws Exception { - assertThat(performGet(url, new HttpHeaders(), String.class).getBody()).isEqualTo(expected); + @Test + public void globalExceptionHandlerWithHandlerNotFound() throws Exception { + startServer(new ReactorHttpServer()); + + assertThatExceptionOfType(HttpStatusCodeException.class) + .isThrownBy(() -> performGet("/no-such-handler", new HttpHeaders(), String.class)) + .satisfies(ex -> { + assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); + assertThat(ex.getResponseBodyAsString()).isEqualTo("" + + "{\"type\":\"about:blank\"," + + "\"title\":\"Not Found\"," + + "\"status\":404," + + "\"instance\":\"/no-such-handler\"}"); + }); + } + + @Test + public void globalExceptionHandlerWithMissingRequestParameter() throws Exception { + startServer(new ReactorHttpServer()); + + assertThatExceptionOfType(HttpStatusCodeException.class) + .isThrownBy(() -> performGet("/missing-request-parameter", new HttpHeaders(), String.class)) + .satisfies(ex -> { + assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(ex.getResponseBodyAsString()).isEqualTo("{" + + "\"type\":\"about:blank\"," + + "\"title\":\"Bad Request\"," + + "\"status\":400," + + "\"detail\":\"Required query parameter 'q' is not present.\"," + + "\"instance\":\"/missing-request-parameter\"}"); + }); } + @Configuration @EnableWebFlux @ComponentScan(resourcePattern = "**/RequestMappingExceptionHandlingIntegrationTests$*.class") @@ -147,6 +181,11 @@ public Publisher handleWithError() { return Mono.error(new IllegalArgumentException("Argument")); } + @GetMapping(path = "/missing-request-parameter") + public String handleWithMissingParameter(@RequestParam String q) { + return "Success, q:" + q; + } + @GetMapping("/SPR-16051") public Flux errors() { return Flux.range(1, 10000) @@ -185,6 +224,11 @@ public ResponseEntity> handle(Spr16318Exception ex) { } + @ControllerAdvice + private static class GlobalExceptionHandler extends ResponseEntityExceptionHandler { + } + + @SuppressWarnings("serial") private static class Spr16318Exception extends Exception { }