Browse Source

Close response when retry on certain status codes. Fixes #1970.

pull/6/head
Ryan Baxter 8 years ago
parent
commit
7d06c140fa
  1. 4
      spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java
  2. 1
      spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java
  3. 10
      spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java

4
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/apache/RetryableRibbonLoadBalancingHttpClient.java

@ -20,6 +20,7 @@ import java.net.URI;
import org.apache.commons.lang.BooleanUtils; import org.apache.commons.lang.BooleanUtils;
import org.apache.http.HttpResponse; import org.apache.http.HttpResponse;
import org.apache.http.client.config.RequestConfig; import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.methods.HttpUriRequest;
import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.ServiceInstance;
import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext; import org.springframework.cloud.client.loadbalancer.LoadBalancedRetryContext;
@ -91,6 +92,9 @@ public class RetryableRibbonLoadBalancingHttpClient extends RibbonLoadBalancingH
HttpUriRequest httpUriRequest = newRequest.toRequest(requestConfig); HttpUriRequest httpUriRequest = newRequest.toRequest(requestConfig);
final HttpResponse httpResponse = RetryableRibbonLoadBalancingHttpClient.this.delegate.execute(httpUriRequest); final HttpResponse httpResponse = RetryableRibbonLoadBalancingHttpClient.this.delegate.execute(httpUriRequest);
if(retryPolicy.retryableStatusCode(httpResponse.getStatusLine().getStatusCode())) { if(retryPolicy.retryableStatusCode(httpResponse.getStatusLine().getStatusCode())) {
if(CloseableHttpResponse.class.isInstance(httpResponse)) {
((CloseableHttpResponse)httpResponse).close();
}
throw new RetryableStatusCodeException(RetryableRibbonLoadBalancingHttpClient.this.clientName, throw new RetryableStatusCodeException(RetryableRibbonLoadBalancingHttpClient.this.clientName,
httpResponse.getStatusLine().getStatusCode()); httpResponse.getStatusLine().getStatusCode());
} }

1
spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/okhttp/RetryableOkHttpLoadBalancingClient.java

@ -96,6 +96,7 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien
final Request request = newRequest.toRequest(); final Request request = newRequest.toRequest();
Response response = httpClient.newCall(request).execute(); Response response = httpClient.newCall(request).execute();
if(retryPolicy.retryableStatusCode(response.code())) { if(retryPolicy.retryableStatusCode(response.code())) {
response.close();
throw new RetryableStatusCodeException(RetryableOkHttpLoadBalancingClient.this.clientName, response.code()); throw new RetryableStatusCodeException(RetryableOkHttpLoadBalancingClient.this.clientName, response.code());
} }
return new OkHttpRibbonResponse(response, newRequest.getUri()); return new OkHttpRibbonResponse(response, newRequest.getUri());

10
spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/apache/RibbonLoadBalancingHttpClientTests.java

@ -22,6 +22,7 @@ import org.apache.http.HttpResponse;
import org.apache.http.StatusLine; import org.apache.http.StatusLine;
import org.apache.http.client.HttpClient; import org.apache.http.client.HttpClient;
import org.apache.http.client.config.RequestConfig; import org.apache.http.client.config.RequestConfig;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.client.methods.HttpUriRequest;
import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
import org.junit.After; import org.junit.After;
@ -285,7 +286,7 @@ public class RibbonLoadBalancingHttpClientTests {
HttpMethod method = HttpMethod.POST; HttpMethod method = HttpMethod.POST;
URI uri = new URI("http://" + host + ":" + port); URI uri = new URI("http://" + host + ":" + port);
HttpClient delegate = mock(HttpClient.class); HttpClient delegate = mock(HttpClient.class);
final HttpResponse response = mock(HttpResponse.class); final CloseableHttpResponse response = mock(CloseableHttpResponse.class);
StatusLine statusLine = mock(StatusLine.class); StatusLine statusLine = mock(StatusLine.class);
doReturn(200).when(statusLine).getStatusCode(); doReturn(200).when(statusLine).getStatusCode();
doReturn(statusLine).when(response).getStatusLine(); doReturn(statusLine).when(response).getStatusLine();
@ -301,6 +302,7 @@ public class RibbonLoadBalancingHttpClientTests {
HttpUriRequest uriRequest = mock(HttpUriRequest.class); HttpUriRequest uriRequest = mock(HttpUriRequest.class);
doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class));
RibbonApacheHttpResponse returnedResponse = client.execute(request, null); RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(response, times(0)).close();
verify(delegate, times(3)).execute(any(HttpUriRequest.class)); verify(delegate, times(3)).execute(any(HttpUriRequest.class));
verify(lb, times(1)).chooseServer(eq(serviceName)); verify(lb, times(1)).chooseServer(eq(serviceName));
} }
@ -317,7 +319,7 @@ public class RibbonLoadBalancingHttpClientTests {
HttpMethod method = HttpMethod.POST; HttpMethod method = HttpMethod.POST;
URI uri = new URI("http://" + host + ":" + port); URI uri = new URI("http://" + host + ":" + port);
HttpClient delegate = mock(HttpClient.class); HttpClient delegate = mock(HttpClient.class);
final HttpResponse response = mock(HttpResponse.class); final CloseableHttpResponse response = mock(CloseableHttpResponse.class);
doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response). doThrow(new IOException("boom")).doThrow(new IOException("boom again")).doReturn(response).
when(delegate).execute(any(HttpUriRequest.class)); when(delegate).execute(any(HttpUriRequest.class));
ILoadBalancer lb = mock(ILoadBalancer.class); ILoadBalancer lb = mock(ILoadBalancer.class);
@ -334,6 +336,7 @@ public class RibbonLoadBalancingHttpClientTests {
client.execute(request, null); client.execute(request, null);
fail("Expected IOException"); fail("Expected IOException");
} catch(IOException e) {} finally { } catch(IOException e) {} finally {
verify(response, times(0)).close();
verify(delegate, times(1)).execute(any(HttpUriRequest.class)); verify(delegate, times(1)).execute(any(HttpUriRequest.class));
verify(lb, times(0)).chooseServer(eq(serviceName)); verify(lb, times(0)).chooseServer(eq(serviceName));
} }
@ -355,7 +358,7 @@ public class RibbonLoadBalancingHttpClientTests {
StatusLine statusLine = mock(StatusLine.class); StatusLine statusLine = mock(StatusLine.class);
doReturn(200).when(statusLine).getStatusCode(); doReturn(200).when(statusLine).getStatusCode();
doReturn(statusLine).when(response).getStatusLine(); doReturn(statusLine).when(response).getStatusLine();
final HttpResponse fourOFourResponse = mock(HttpResponse.class); final CloseableHttpResponse fourOFourResponse = mock(CloseableHttpResponse.class);
StatusLine fourOFourStatusLine = mock(StatusLine.class); StatusLine fourOFourStatusLine = mock(StatusLine.class);
doReturn(404).when(fourOFourStatusLine).getStatusCode(); doReturn(404).when(fourOFourStatusLine).getStatusCode();
doReturn(fourOFourStatusLine).when(fourOFourResponse).getStatusLine(); doReturn(fourOFourStatusLine).when(fourOFourResponse).getStatusLine();
@ -371,6 +374,7 @@ public class RibbonLoadBalancingHttpClientTests {
doReturn(uri).when(uriRequest).getURI(); doReturn(uri).when(uriRequest).getURI();
doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class)); doReturn(uriRequest).when(request).toRequest(any(RequestConfig.class));
RibbonApacheHttpResponse returnedResponse = client.execute(request, null); RibbonApacheHttpResponse returnedResponse = client.execute(request, null);
verify(fourOFourResponse, times(1)).close();
verify(delegate, times(2)).execute(any(HttpUriRequest.class)); verify(delegate, times(2)).execute(any(HttpUriRequest.class));
verify(lb, times(0)).chooseServer(eq(serviceName)); verify(lb, times(0)).chooseServer(eq(serviceName));
} }

Loading…
Cancel
Save