Fix for RetryableFeignBlockingLoadBalancerClient closes stream (#569)
This commit is contained in:
@@ -0,0 +1,48 @@
|
||||
/*
|
||||
* Copyright 2013-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.cloud.openfeign.loadbalancer;
|
||||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.net.URI;
|
||||
|
||||
import feign.Response;
|
||||
|
||||
import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException;
|
||||
|
||||
/**
|
||||
* A {@link RetryableStatusCodeException} for {@link Response}s.
|
||||
*
|
||||
* @author Ryan Baxter
|
||||
*/
|
||||
public class LoadBalancerResponseStatusCodeException extends RetryableStatusCodeException {
|
||||
|
||||
private final Response response;
|
||||
|
||||
public LoadBalancerResponseStatusCodeException(String serviceId, Response response, byte[] body, URI uri) {
|
||||
super(serviceId, response.status(), response, uri);
|
||||
this.response = Response.builder()
|
||||
.body(new ByteArrayInputStream(body), body.length)
|
||||
.headers(response.headers()).reason(response.reason())
|
||||
.status(response.status()).request(response.request()).build();
|
||||
}
|
||||
|
||||
@Override
|
||||
public Response getResponse() {
|
||||
return this.response;
|
||||
}
|
||||
|
||||
}
|
||||
@@ -36,7 +36,6 @@ import org.springframework.cloud.client.loadbalancer.LoadBalancedRecoveryCallbac
|
||||
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext;
|
||||
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryFactory;
|
||||
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryPolicy;
|
||||
import org.springframework.cloud.client.loadbalancer.RetryableStatusCodeException;
|
||||
import org.springframework.cloud.loadbalancer.blocking.client.BlockingLoadBalancerClient;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.HttpMethod;
|
||||
@@ -46,6 +45,7 @@ import org.springframework.retry.backoff.BackOffPolicy;
|
||||
import org.springframework.retry.backoff.NoBackOffPolicy;
|
||||
import org.springframework.retry.policy.NeverRetryPolicy;
|
||||
import org.springframework.retry.support.RetryTemplate;
|
||||
import org.springframework.util.StreamUtils;
|
||||
|
||||
/**
|
||||
* A {@link Client} implementation that provides Spring Retry support for requests
|
||||
@@ -112,9 +112,11 @@ public class RetryableFeignBlockingLoadBalancerClient implements Client {
|
||||
LOG.debug(
|
||||
String.format("Retrying on status code: %d", responseStatus));
|
||||
}
|
||||
byte[] byteArray = response.body() == null ? new byte[] {}
|
||||
: StreamUtils.copyToByteArray(response.body().asInputStream());
|
||||
response.close();
|
||||
throw new RetryableStatusCodeException(serviceId, responseStatus,
|
||||
response, URI.create(request.url()));
|
||||
throw new LoadBalancerResponseStatusCodeException(serviceId, response,
|
||||
byteArray, URI.create(request.url()));
|
||||
}
|
||||
return response;
|
||||
}, new LoadBalancedRecoveryCallback<Response, Response>() {
|
||||
|
||||
@@ -16,7 +16,10 @@
|
||||
|
||||
package org.springframework.cloud.openfeign.loadbalancer;
|
||||
|
||||
import java.io.BufferedInputStream;
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.net.URI;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.Collection;
|
||||
@@ -27,6 +30,7 @@ import java.util.Map;
|
||||
import feign.Client;
|
||||
import feign.Request;
|
||||
import feign.Response;
|
||||
import org.apache.commons.io.IOUtils;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.api.extension.ExtendWith;
|
||||
@@ -113,6 +117,13 @@ class RetryableFeignBlockingLoadBalancerClientTests {
|
||||
return Response.builder().request(testRequest()).status(status).build();
|
||||
}
|
||||
|
||||
private Response testResponse(int status, String body) {
|
||||
// ByteArrayInputStream ignores close() and must be wrapped
|
||||
InputStream reallyCloseable = new BufferedInputStream(new ByteArrayInputStream(body.getBytes(StandardCharsets.UTF_8)));
|
||||
return Response.builder().request(testRequest()).status(status).body(reallyCloseable, null).build();
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldExecuteOriginalRequestIfInstanceNotFound() throws IOException {
|
||||
Request request = testRequest();
|
||||
@@ -148,6 +159,27 @@ class RetryableFeignBlockingLoadBalancerClientTests {
|
||||
verify(delegate, times(2)).execute(any(), any());
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldExposeResponseBodyOnRetry() throws IOException {
|
||||
properties.getRetryableStatusCodes().add(503);
|
||||
Request request = testRequest();
|
||||
when(delegate.execute(any(), any()))
|
||||
.thenReturn(testResponse(503, "foo"), testResponse(503, "foo"));
|
||||
when(retryFactory.createRetryPolicy(any(), eq(loadBalancerClient)))
|
||||
.thenReturn(new BlockingLoadBalancedRetryPolicy("test",
|
||||
loadBalancerClient, properties));
|
||||
when(loadBalancerClient.reconstructURI(serviceInstance,
|
||||
URI.create("http://test/path")))
|
||||
.thenReturn(URI.create("http://testhost:80/path"));
|
||||
|
||||
Response response = feignBlockingLoadBalancerClient.execute(request, new Request.Options());
|
||||
|
||||
String bodyContent = IOUtils.toString(response.body().asReader(StandardCharsets.UTF_8));
|
||||
assertThat(bodyContent).isEqualTo("foo");
|
||||
}
|
||||
|
||||
|
||||
|
||||
@Test
|
||||
void shouldPassCorrectRequestToDelegate() throws IOException {
|
||||
Request request = testRequest();
|
||||
|
||||
Reference in New Issue
Block a user