From 74f64c4e3b97cb419829cedcc6f703a78f9d0a5c Mon Sep 17 00:00:00 2001 From: Arjen Poutsma Date: Mon, 18 Nov 2019 11:10:33 +0100 Subject: [PATCH] Wrap exceptions in WebClient This commit makes sure that exceptions emitted by WebClient are wrapped by WebClientExceptions: - Exceptions emitted by the ClientHttpConnector are wrapped in a new WebClientRequestException. - Exceptions emitted after a response is received are wrapped in a WebClientResponseException Closes gh-23842 --- .../client/DefaultClientResponse.java | 6 +- .../function/client/DefaultWebClient.java | 18 ++++- .../function/client/ExchangeFunctions.java | 5 ++ .../client/WebClientRequestException.java | 74 +++++++++++++++++++ .../function/client/WebClientUtils.java | 7 ++ .../WebClientDataBufferAllocatingTests.java | 3 +- .../client/WebClientIntegrationTests.java | 26 ++++++- 7 files changed, 131 insertions(+), 8 deletions(-) create mode 100644 spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientRequestException.java diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java index 94eeb137f2..6d0331b9bd 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponse.java @@ -55,6 +55,9 @@ import org.springframework.web.reactive.function.BodyExtractors; */ class DefaultClientResponse implements ClientResponse { + private static final byte[] EMPTY = new byte[0]; + + private final ClientHttpResponse response; private final Headers headers; @@ -200,7 +203,8 @@ class DefaultClientResponse implements ClientResponse { DataBufferUtils.release(dataBuffer); return bytes; }) - .defaultIfEmpty(new byte[0]) + .defaultIfEmpty(EMPTY) + .onErrorReturn(IllegalStateException.class::isInstance, EMPTY) .map(bodyBytes -> { HttpRequest request = this.requestSupplier.get(); Charset charset = headers().contentType() diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java index 05850706d4..b7f7e4fb0f 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java @@ -488,11 +488,13 @@ class DefaultWebClient implements WebClient { private Mono handleBodyMono(ClientResponse response, Mono bodyPublisher) { Mono result = statusHandlers(response); + Mono wrappedExceptions = bodyPublisher.onErrorResume(WebClientUtils::shouldWrapException, + t -> wrapException(t, response)); if (result != null) { - return result.switchIfEmpty(bodyPublisher); + return result.switchIfEmpty(wrappedExceptions); } else { - return bodyPublisher; + return wrappedExceptions; } } @@ -510,11 +512,13 @@ class DefaultWebClient implements WebClient { private Publisher handleBodyFlux(ClientResponse response, Flux bodyPublisher) { Mono result = statusHandlers(response); + Flux wrappedExceptions = bodyPublisher.onErrorResume(WebClientUtils::shouldWrapException, + t -> wrapException(t, response)); if (result != null) { - return result.flux().switchIfEmpty(bodyPublisher); + return result.flux().switchIfEmpty(wrappedExceptions); } else { - return bodyPublisher; + return wrappedExceptions; } } @@ -555,6 +559,12 @@ class DefaultWebClient implements WebClient { return result.checkpoint(description); } + private Mono wrapException(Throwable throwable, ClientResponse response) { + return response.createException() + .map(responseException -> responseException.initCause(throwable)) + .flatMap(Mono::error); + } + @Override public Mono> toEntity(Class bodyClass) { return this.responseMono.flatMap(response -> diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java index b13651cc84..8566a1bad8 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/ExchangeFunctions.java @@ -104,6 +104,7 @@ public abstract class ExchangeFunctions { .connect(httpMethod, url, httpRequest -> clientRequest.writeTo(httpRequest, this.strategies)) .doOnRequest(n -> logRequest(clientRequest)) .doOnCancel(() -> logger.debug(logPrefix + "Cancel signal (to close connection)")) + .onErrorResume(WebClientUtils::shouldWrapException, t -> wrapException(t, clientRequest)) .map(httpResponse -> { logResponse(httpResponse, logPrefix); return new DefaultClientResponse( @@ -132,6 +133,10 @@ public abstract class ExchangeFunctions { return this.enableLoggingRequestDetails ? headers.toString() : headers.isEmpty() ? "{}" : "{masked}"; } + private Mono wrapException(Throwable t, ClientRequest r) { + return Mono.error(() -> new WebClientRequestException(t, r.method(), r.url(), r.headers())); + } + private HttpRequest createRequest(ClientRequest request) { return new HttpRequest() { diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientRequestException.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientRequestException.java new file mode 100644 index 0000000000..573fe6559d --- /dev/null +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientRequestException.java @@ -0,0 +1,74 @@ +/* + * Copyright 2002-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. + * 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.function.client; + +import java.net.URI; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; + +/** + * Exceptions that contain actual HTTP request data. + * + * @author Arjen Poutsma + * @since 5.3 + */ +public class WebClientRequestException extends WebClientException { + + private static final long serialVersionUID = -5139991985321385005L; + + + private final HttpMethod method; + + private final URI uri; + + private final HttpHeaders headers; + + + /** + * Constructor for throwable. + */ + public WebClientRequestException(Throwable ex, HttpMethod method, URI uri, HttpHeaders headers) { + super(ex.getMessage(), ex); + + this.method = method; + this.uri = uri; + this.headers = headers; + } + + /** + * Return the HTTP request method. + */ + public HttpMethod getMethod() { + return this.method; + } + + /** + * Return the request URI. + */ + public URI getUri() { + return this.uri; + } + + /** + * Return the HTTP request headers. + */ + public HttpHeaders getHeaders() { + return this.headers; + } + +} diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java index ac05b7b048..799ef5bf79 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java @@ -22,6 +22,7 @@ import org.reactivestreams.Publisher; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import org.springframework.core.codec.CodecException; import org.springframework.http.ResponseEntity; /** @@ -56,4 +57,10 @@ abstract class WebClientUtils { .body(list)); } + /** + * Indicates whether the given exception should be wrapped. + */ + public static boolean shouldWrapException(Throwable t) { + return !(t instanceof WebClientException) && !(t instanceof CodecException); + } } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java index 450198d08d..4b5ec631b9 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientDataBufferAllocatingTests.java @@ -40,7 +40,6 @@ import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.http.client.reactive.ReactorClientHttpConnector; import org.springframework.http.client.reactive.ReactorResourceFactory; -import org.springframework.web.reactive.function.UnsupportedMediaTypeException; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS; @@ -127,7 +126,7 @@ class WebClientDataBufferAllocatingTests extends AbstractDataBufferAllocatingTes .retrieve() .bodyToMono(new ParameterizedTypeReference>() {}); - StepVerifier.create(mono).expectError(UnsupportedMediaTypeException.class).verify(Duration.ofSeconds(3)); + StepVerifier.create(mono).expectError(WebClientResponseException.class).verify(Duration.ofSeconds(3)); assertThat(this.server.getRequestCount()).isEqualTo(1); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java index 508fa8df93..c3f07ec6a9 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/WebClientIntegrationTests.java @@ -1013,7 +1013,12 @@ class WebClientIntegrationTests { Mono responseMono = WebClient.builder().build().get().uri(uri).exchange(); StepVerifier.create(responseMono) - .expectErrorMessage("URI is not absolute: " + uri) + .expectErrorSatisfies(throwable -> { + assertThat(throwable).isInstanceOf(WebClientRequestException.class); + WebClientRequestException ex = (WebClientRequestException) throwable; + assertThat(ex.getMethod()).isEqualTo(HttpMethod.GET); + assertThat(ex.getUri()).isEqualTo(URI.create(uri)); + }) .verify(Duration.ofSeconds(5)); } @@ -1126,6 +1131,25 @@ class WebClientIntegrationTests { expectRequestCount(1); } + @ParameterizedWebClientTest + void invalidDomain(ClientHttpConnector connector) { + startServer(connector); + + String url = "http://example.invalid"; + Mono result = this.webClient.get(). + uri(url) + .exchange(); + + StepVerifier.create(result) + .expectErrorSatisfies(throwable -> { + assertThat(throwable).isInstanceOf(WebClientRequestException.class); + WebClientRequestException ex = (WebClientRequestException) throwable; + assertThat(ex.getMethod()).isEqualTo(HttpMethod.GET); + assertThat(ex.getUri()).isEqualTo(URI.create(url)); + }) + .verify(); + } + private void prepareResponse(Consumer consumer) { MockResponse response = new MockResponse();