Skip to content

SPR-16819 - Improve handling of unknown status codes by WebClient #1829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,22 @@ public interface ClientResponse {

/**
* Return the status code of this response.
* @return the status as an HttpStatus enum value
* @throws IllegalArgumentException in case of an unknown HTTP status code
* @see HttpStatus#valueOf(int)
*/
HttpStatus statusCode();

/**
* Return the status code (potentially non-standard and not resolvable
* through the {@link HttpStatus} enum) as an integer.
* @return the status as an integer
* @since 5.0.7
* @see #statusCode()
* @see HttpStatus#resolve(int)
*/
int rawStatusCode();

/**
* Return the headers of this response.
*/
Expand Down Expand Up @@ -172,6 +185,17 @@ static Builder create(HttpStatus statusCode) {
return create(statusCode, ExchangeStrategies.withDefaults());
}

/**
* Create a response builder with the given status code and using default strategies for
* reading the body.
* @param statusCode the status code
* @return the created builder
* @since 5.0.7
*/
static Builder create(int statusCode) {
return create(statusCode, ExchangeStrategies.withDefaults());
}

/**
* Create a response builder with the given status code and strategies for reading the body.
* @param statusCode the status code
Expand All @@ -182,6 +206,17 @@ static Builder create(HttpStatus statusCode, ExchangeStrategies strategies) {
return new DefaultClientResponseBuilder(strategies).statusCode(statusCode);
}

/**
* Create a response builder with the given status code and strategies for reading the body.
* @param statusCode the status code
* @param strategies the strategies
* @return the created builder
* @since 5.0.7
*/
static Builder create(int statusCode, ExchangeStrategies strategies) {
return new DefaultClientResponseBuilder(strategies).statusCode(statusCode);
}

/**
* Create a response builder with the given status code and message body readers.
* @param statusCode the status code
Expand All @@ -202,6 +237,27 @@ public List<HttpMessageWriter<?>> messageWriters() {
});
}

/**
* Create a response builder with the given status code and message body readers.
* @param statusCode the status code
* @param messageReaders the message readers
* @return the created builder
* @since 5.0.7
*/
static Builder create(int statusCode, List<HttpMessageReader<?>> messageReaders) {
return create(statusCode, new ExchangeStrategies() {
@Override
public List<HttpMessageReader<?>> messageReaders() {
return messageReaders;
}
@Override
public List<HttpMessageWriter<?>> messageWriters() {
// not used in the response
return Collections.emptyList();
}
});
}


/**
* Represents the headers of the HTTP response.
Expand Down Expand Up @@ -247,6 +303,14 @@ interface Builder {
*/
Builder statusCode(HttpStatus statusCode);

/**
* Set the status code of the response.
* @param statusCode the new status code.
* @return this builder
* @since 5.0.7
*/
Builder statusCode(int statusCode);

/**
* Add the given header value(s) under the given name.
* @param headerName the header name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ public HttpStatus statusCode() {
return this.response.getStatusCode();
}

@Override
public int rawStatusCode() {
return this.response.getRawStatusCode();
}

@Override
public Headers headers() {
return this.headers;
Expand Down Expand Up @@ -173,11 +178,11 @@ public <T> Mono<ResponseEntity<T>> toEntity(ParameterizedTypeReference<T> typeRe

private <T> Mono<ResponseEntity<T>> toEntityInternal(Mono<T> bodyMono) {
HttpHeaders headers = headers().asHttpHeaders();
HttpStatus statusCode = statusCode();
int status = rawStatusCode();
return bodyMono
.map(body -> new ResponseEntity<>(body, headers, statusCode))
.map(body -> createEntity(body, headers, status))
.switchIfEmpty(Mono.defer(
() -> Mono.just(new ResponseEntity<>(headers, statusCode))));
() -> Mono.just(createEntity(headers, status))));
}

@Override
Expand All @@ -192,10 +197,24 @@ public <T> Mono<ResponseEntity<List<T>>> toEntityList(ParameterizedTypeReference

private <T> Mono<ResponseEntity<List<T>>> toEntityListInternal(Flux<T> bodyFlux) {
HttpHeaders headers = headers().asHttpHeaders();
HttpStatus statusCode = statusCode();
int status = rawStatusCode();
return bodyFlux
.collectList()
.map(body -> new ResponseEntity<>(body, headers, statusCode));
.map(body -> createEntity(body, headers, status));
}

private <T> ResponseEntity<T> createEntity(HttpHeaders headers, int status) {
HttpStatus resolvedStatus = HttpStatus.resolve(status);
return resolvedStatus != null
? new ResponseEntity<>(headers, resolvedStatus)
: ResponseEntity.status(status).headers(headers).build();
}

private <T> ResponseEntity<T> createEntity(T body, HttpHeaders headers, int status) {
HttpStatus resolvedStatus = HttpStatus.resolve(status);
return resolvedStatus != null
? new ResponseEntity<>(body, headers, resolvedStatus)
: ResponseEntity.status(status).headers(headers).body(body);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder {

private ExchangeStrategies strategies;

private HttpStatus statusCode = HttpStatus.OK;
private int statusCode = HttpStatus.OK.value();

private final HttpHeaders headers = new HttpHeaders();

Expand All @@ -61,7 +61,7 @@ public DefaultClientResponseBuilder(ExchangeStrategies strategies) {
public DefaultClientResponseBuilder(ClientResponse other) {
Assert.notNull(other, "ClientResponse must not be null");
this.strategies = other.strategies();
statusCode(other.statusCode());
statusCode(other.rawStatusCode());
headers(headers -> headers.addAll(other.headers().asHttpHeaders()));
cookies(cookies -> cookies.addAll(other.cookies()));
}
Expand All @@ -70,6 +70,12 @@ public DefaultClientResponseBuilder(ClientResponse other) {
@Override
public DefaultClientResponseBuilder statusCode(HttpStatus statusCode) {
Assert.notNull(statusCode, "HttpStatus must not be null");
this.statusCode = statusCode.value();
return this;
}

@Override
public DefaultClientResponseBuilder statusCode(int statusCode) {
this.statusCode = statusCode;
return this;
}
Expand Down Expand Up @@ -137,15 +143,15 @@ public ClientResponse build() {

private static class BuiltClientHttpResponse implements ClientHttpResponse {

private final HttpStatus statusCode;
private final int statusCode;

private final HttpHeaders headers;

private final MultiValueMap<String, ResponseCookie> cookies;

private final Flux<DataBuffer> body;

public BuiltClientHttpResponse(HttpStatus statusCode, HttpHeaders headers,
public BuiltClientHttpResponse(int statusCode, HttpHeaders headers,
MultiValueMap<String, ResponseCookie> cookies, Flux<DataBuffer> body) {

this.statusCode = statusCode;
Expand All @@ -156,12 +162,12 @@ public BuiltClientHttpResponse(HttpStatus statusCode, HttpHeaders headers,

@Override
public HttpStatus getStatusCode() {
return this.statusCode;
return HttpStatus.valueOf(this.statusCode);
}

@Override
public int getRawStatusCode() {
return this.statusCode.value();
return this.statusCode;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
* Default implementation of {@link WebClient}.
*
* @author Rossen Stoyanchev
* @author Denys Ivano
* @since 5.0
*/
class DefaultWebClient implements WebClient {
Expand Down Expand Up @@ -369,11 +370,12 @@ public ResponseSpec retrieve() {
private static class DefaultResponseSpec implements ResponseSpec {

private static final StatusHandler DEFAULT_STATUS_HANDLER =
new StatusHandler(HttpStatus::isError, DefaultResponseSpec::createResponseException);
new StatusHandler(StatusCodePredicates.isError(),
DefaultResponseSpec::createResponseException);

private final Mono<ClientResponse> responseMono;

private List<StatusHandler> statusHandlers = new ArrayList<>(1);
private final List<StatusHandler> statusHandlers = new ArrayList<>(1);

DefaultResponseSpec(Mono<ClientResponse> responseMono) {
this.responseMono = responseMono;
Expand All @@ -384,12 +386,26 @@ private static class DefaultResponseSpec implements ResponseSpec {
public ResponseSpec onStatus(Predicate<HttpStatus> statusPredicate,
Function<ClientResponse, Mono<? extends Throwable>> exceptionFunction) {

removeDefaultStatusHandler();
this.statusHandlers.add(new HttpStatusHandler(statusPredicate, exceptionFunction));

return this;
}

@Override
public ResponseSpec onStatusCode(Predicate<Integer> statusCodePredicate,
Function<ClientResponse, Mono<? extends Throwable>> exceptionFunction) {

removeDefaultStatusHandler();
this.statusHandlers.add(new StatusHandler(statusCodePredicate, exceptionFunction));

return this;
}

private void removeDefaultStatusHandler() {
if (this.statusHandlers.size() == 1 && this.statusHandlers.get(0) == DEFAULT_STATUS_HANDLER) {
this.statusHandlers.clear();
}
this.statusHandlers.add(new StatusHandler(statusPredicate, exceptionFunction));

return this;
}

@Override
Expand Down Expand Up @@ -435,7 +451,7 @@ private <T extends Publisher<?>> T bodyToPublisher(ClientResponse response,
T bodyPublisher, Function<Mono<? extends Throwable>, T> errorFunction) {

return this.statusHandlers.stream()
.filter(statusHandler -> statusHandler.test(response.statusCode()))
.filter(statusHandler -> statusHandler.test(response.rawStatusCode()))
.findFirst()
.map(statusHandler -> statusHandler.apply(response))
.map(errorFunction::apply)
Expand All @@ -453,14 +469,16 @@ private static Mono<WebClientResponseException> createResponseException(ClientRe
})
.defaultIfEmpty(new byte[0])
.map(bodyBytes -> {
String msg = String.format("ClientResponse has erroneous status code: %d %s", response.statusCode().value(),
response.statusCode().getReasonPhrase());
int status = response.rawStatusCode();
HttpStatus resolvedStatus = HttpStatus.resolve(status);
String msg = "ClientResponse has erroneous status code: " + status +
(resolvedStatus != null ? " " + resolvedStatus.getReasonPhrase() : "");
Charset charset = response.headers().contentType()
.map(MimeType::getCharset)
.orElse(StandardCharsets.ISO_8859_1);
return new WebClientResponseException(msg,
response.statusCode().value(),
response.statusCode().getReasonPhrase(),
status,
resolvedStatus != null ? resolvedStatus.getReasonPhrase() : null,
response.headers().asHttpHeaders(),
bodyBytes,
charset
Expand All @@ -471,11 +489,11 @@ private static Mono<WebClientResponseException> createResponseException(ClientRe

private static class StatusHandler {

private final Predicate<HttpStatus> predicate;
private final Predicate<Integer> predicate;

private final Function<ClientResponse, Mono<? extends Throwable>> exceptionFunction;

public StatusHandler(Predicate<HttpStatus> predicate,
public StatusHandler(Predicate<Integer> predicate,
Function<ClientResponse, Mono<? extends Throwable>> exceptionFunction) {

Assert.notNull(predicate, "Predicate must not be null");
Expand All @@ -484,14 +502,30 @@ public StatusHandler(Predicate<HttpStatus> predicate,
this.exceptionFunction = exceptionFunction;
}

public boolean test(HttpStatus status) {
public boolean test(int status) {
return this.predicate.test(status);
}

public Mono<? extends Throwable> apply(ClientResponse response) {
return this.exceptionFunction.apply(response);
}
}

private static class HttpStatusHandler extends StatusHandler {

public HttpStatusHandler(Predicate<HttpStatus> predicate,
Function<ClientResponse, Mono<? extends Throwable>> exceptionFunction) {

super(statusCodePredicate(predicate), exceptionFunction);
}

private static Predicate<Integer> statusCodePredicate(Predicate<HttpStatus> predicate) {
return status -> {
HttpStatus resolvedStatus = HttpStatus.resolve(status);
return resolvedStatus != null && predicate.test(resolvedStatus);
};
}
}
}

}
Loading