From 4b4dc28a869e2f8b988f6ac6ea8a31c274477da5 Mon Sep 17 00:00:00 2001 From: Madhura Bhave Date: Mon, 5 Aug 2019 15:05:00 -0700 Subject: [PATCH] Support non-standard error codes with AbstractErrorWebExceptionHandler Fixes gh-16691 --- .../AbstractErrorWebExceptionHandler.java | 4 +- .../DefaultErrorWebExceptionHandler.java | 22 ++++++--- .../DefaultErrorWebExceptionHandlerTests.java | 49 ++++++++++++++++++- 3 files changed, 67 insertions(+), 8 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java index b36b7384f5d..b67ae96b372 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/AbstractErrorWebExceptionHandler.java @@ -62,6 +62,7 @@ public abstract class AbstractErrorWebExceptionHandler implements ErrorWebExcept * Currently duplicated from Spring WebFlux HttpWebHandlerAdapter. */ private static final Set DISCONNECTED_CLIENT_EXCEPTIONS; + static { Set exceptions = new HashSet<>(); exceptions.add("AbortedException"); @@ -276,7 +277,8 @@ public abstract class AbstractErrorWebExceptionHandler implements ErrorWebExcept if (logger.isDebugEnabled()) { logger.debug(request.exchange().getLogPrefix() + formatError(throwable, request)); } - if (response.statusCode().equals(HttpStatus.INTERNAL_SERVER_ERROR)) { + if (HttpStatus.resolve(response.rawStatusCode()) != null + && response.statusCode().equals(HttpStatus.INTERNAL_SERVER_ERROR)) { logger.error(request.exchange().getLogPrefix() + "500 Server Error for " + formatRequest(request), throwable); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java index 385f8485e7c..1d76099703b 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandler.java @@ -17,6 +17,7 @@ package org.springframework.boot.autoconfigure.web.reactive.error; import java.nio.charset.StandardCharsets; +import java.util.ArrayList; import java.util.Collections; import java.util.EnumMap; import java.util.List; @@ -113,16 +114,26 @@ public class DefaultErrorWebExceptionHandler extends AbstractErrorWebExceptionHa protected Mono renderErrorView(ServerRequest request) { boolean includeStackTrace = isIncludeStackTrace(request, MediaType.TEXT_HTML); Map error = getErrorAttributes(request, includeStackTrace); - HttpStatus errorStatus = getHttpStatus(error); + int errorStatus = getHttpStatus(error); ServerResponse.BodyBuilder responseBody = ServerResponse.status(errorStatus).contentType(TEXT_HTML_UTF8); - return Flux - .just("error/" + errorStatus.value(), "error/" + SERIES_VIEWS.get(errorStatus.series()), "error/error") + return Flux.just(getData(errorStatus).toArray(new String[] {})) .flatMap((viewName) -> renderErrorView(viewName, responseBody, error)) .switchIfEmpty(this.errorProperties.getWhitelabel().isEnabled() ? renderDefaultErrorView(responseBody, error) : Mono.error(getError(request))) .next(); } + private List getData(int errorStatus) { + HttpStatus errorHttpStatus = HttpStatus.resolve(errorStatus); + List data = new ArrayList<>(); + data.add("error/" + errorStatus); + if (errorHttpStatus != null) { + data.add("error/" + SERIES_VIEWS.get(errorHttpStatus.series())); + } + data.add("error/error"); + return data; + } + /** * Render the error information as a JSON payload. * @param request the current request @@ -157,9 +168,8 @@ public class DefaultErrorWebExceptionHandler extends AbstractErrorWebExceptionHa * @param errorAttributes the current error information * @return the error HTTP status */ - protected HttpStatus getHttpStatus(Map errorAttributes) { - int statusCode = (int) errorAttributes.get("status"); - return HttpStatus.valueOf(statusCode); + protected int getHttpStatus(Map errorAttributes) { + return (int) errorAttributes.get("status"); } /** diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerTests.java index 329ed9388b3..ed682a3f8d0 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/reactive/error/DefaultErrorWebExceptionHandlerTests.java @@ -16,17 +16,37 @@ package org.springframework.boot.autoconfigure.web.reactive.error; -import org.junit.jupiter.api.Test; +import java.util.Collections; +import java.util.Map; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Mono; + +import org.springframework.boot.autoconfigure.web.ErrorProperties; +import org.springframework.boot.autoconfigure.web.ResourceProperties; +import org.springframework.boot.web.reactive.context.AnnotationConfigReactiveWebApplicationContext; +import org.springframework.boot.web.reactive.error.ErrorAttributes; +import org.springframework.context.ApplicationContext; +import org.springframework.http.MediaType; +import org.springframework.mock.http.server.reactive.MockServerHttpRequest; +import org.springframework.mock.web.server.MockServerWebExchange; import org.springframework.test.util.ReflectionTestUtils; +import org.springframework.web.reactive.result.view.View; +import org.springframework.web.reactive.result.view.ViewResolver; +import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.adapter.HttpWebHandlerAdapter; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; /** * Tests for {@link AbstractErrorWebExceptionHandler}. * * @author Phillip Webb + * @author Madhura Bhave */ class DefaultErrorWebExceptionHandlerTests { @@ -39,4 +59,31 @@ class DefaultErrorWebExceptionHandlerTests { assertThat(errorHandlers).isNotNull().isEqualTo(webHandlers); } + @Test + void nonStandardErrorStatusCodeShouldNotFail() { + ErrorAttributes errorAttributes = mock(ErrorAttributes.class); + ResourceProperties resourceProperties = new ResourceProperties(); + ErrorProperties errorProperties = new ErrorProperties(); + ApplicationContext context = new AnnotationConfigReactiveWebApplicationContext(); + given(errorAttributes.getErrorAttributes(any(), anyBoolean())).willReturn(getErrorAttributes()); + DefaultErrorWebExceptionHandler exceptionHandler = new DefaultErrorWebExceptionHandler(errorAttributes, + resourceProperties, errorProperties, context); + setupViewResolver(exceptionHandler); + ServerWebExchange exchange = MockServerWebExchange + .from(MockServerHttpRequest.get("/some-other-path").accept(MediaType.TEXT_HTML)); + exceptionHandler.handle(exchange, new RuntimeException()).block(); + } + + private Map getErrorAttributes() { + return Collections.singletonMap("status", 498); + } + + private void setupViewResolver(DefaultErrorWebExceptionHandler exceptionHandler) { + View view = mock(View.class); + given(view.render(any(), any(), any())).willReturn(Mono.empty()); + ViewResolver viewResolver = mock(ViewResolver.class); + given(viewResolver.resolveViewName(any(), any())).willReturn(Mono.just(view)); + exceptionHandler.setViewResolvers(Collections.singletonList(viewResolver)); + } + }