Commit a1c8cfe9 by Ryan Baxter

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

parent 13155ce6
...@@ -20,6 +20,7 @@ import java.net.URI; ...@@ -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 ...@@ -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());
} }
......
...@@ -96,6 +96,7 @@ public class RetryableOkHttpLoadBalancingClient extends OkHttpLoadBalancingClien ...@@ -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());
......
...@@ -22,6 +22,7 @@ import org.apache.http.HttpResponse; ...@@ -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 { ...@@ -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 { ...@@ -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 { ...@@ -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 { ...@@ -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 { ...@@ -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 { ...@@ -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));
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment