Skip to content
Permalink
Browse files
HTTPCLIENT-2141: HttpClient to not retry requests if the retry interv…
…al exceeds the response timeout
  • Loading branch information
ok2c authored and Oleg Kalnichevski committed Mar 28, 2021
1 parent bde58d6 commit 73c1530b3f99f8c06d73adb122bc9dd24279e27b
Showing 2 changed files with 77 additions and 2 deletions.
@@ -34,6 +34,7 @@
import org.apache.hc.client5.http.classic.ExecChain;
import org.apache.hc.client5.http.classic.ExecChain.Scope;
import org.apache.hc.client5.http.classic.ExecChainHandler;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.annotation.Contract;
import org.apache.hc.core5.annotation.Internal;
@@ -46,6 +47,7 @@
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
import org.apache.hc.core5.util.Args;
import org.apache.hc.core5.util.TimeValue;
import org.apache.hc.core5.util.Timeout;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@@ -133,9 +135,16 @@ public ClassicHttpResponse execute(
return response;
}
if (retryStrategy.retryRequest(response, execCount, context)) {
final TimeValue nextInterval = retryStrategy.getRetryInterval(response, execCount, context);
// Make sure the retry interval does not exceed the response timeout
if (TimeValue.isPositive(nextInterval)) {
final RequestConfig requestConfig = context.getRequestConfig();
final Timeout responseTimeout = requestConfig.getResponseTimeout();
if (responseTimeout != null && nextInterval.compareTo(responseTimeout) > 0) {
return response;
}
}
response.close();
final TimeValue nextInterval =
retryStrategy.getRetryInterval(response, execCount, context);
if (TimeValue.isPositive(nextInterval)) {
try {
if (LOG.isDebugEnabled()) {
@@ -36,6 +36,7 @@
import org.apache.hc.client5.http.classic.ExecRuntime;
import org.apache.hc.client5.http.classic.methods.HttpGet;
import org.apache.hc.client5.http.classic.methods.HttpPost;
import org.apache.hc.client5.http.config.RequestConfig;
import org.apache.hc.client5.http.entity.EntityBuilder;
import org.apache.hc.client5.http.protocol.HttpClientContext;
import org.apache.hc.core5.http.ClassicHttpRequest;
@@ -47,6 +48,7 @@
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
import org.apache.hc.core5.http.protocol.HttpContext;
import org.apache.hc.core5.util.TimeValue;
import org.apache.hc.core5.util.Timeout;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
@@ -106,6 +108,70 @@ public void testFundamentals1() throws Exception {
Mockito.verify(response, Mockito.times(1)).close();
}

@Test
public void testRetryIntervalGreaterResponseTimeout() throws Exception {
final HttpRoute route = new HttpRoute(target);
final HttpGet request = new HttpGet("/test");
final HttpClientContext context = HttpClientContext.create();
context.setRequestConfig(RequestConfig.custom()
.setResponseTimeout(Timeout.ofSeconds(3))
.build());

final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);

Mockito.when(chain.proceed(
Mockito.same(request),
Mockito.<ExecChain.Scope>any())).thenReturn(response);
Mockito.when(retryStrategy.retryRequest(
Mockito.<HttpResponse>any(),
Mockito.anyInt(),
Mockito.<HttpContext>any())).thenReturn(Boolean.TRUE, Boolean.FALSE);
Mockito.when(retryStrategy.getRetryInterval(
Mockito.<HttpResponse>any(),
Mockito.anyInt(),
Mockito.<HttpContext>any())).thenReturn(TimeValue.ofSeconds(5));

final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
retryExec.execute(request, scope, chain);

Mockito.verify(chain, Mockito.times(1)).proceed(
Mockito.<ClassicHttpRequest>any(),
Mockito.same(scope));
Mockito.verify(response, Mockito.times(0)).close();
}

@Test
public void testRetryIntervalResponseTimeoutNull() throws Exception {
final HttpRoute route = new HttpRoute(target);
final HttpGet request = new HttpGet("/test");
final HttpClientContext context = HttpClientContext.create();
context.setRequestConfig(RequestConfig.custom()
.setResponseTimeout(null)
.build());

final ClassicHttpResponse response = Mockito.mock(ClassicHttpResponse.class);

Mockito.when(chain.proceed(
Mockito.same(request),
Mockito.<ExecChain.Scope>any())).thenReturn(response);
Mockito.when(retryStrategy.retryRequest(
Mockito.<HttpResponse>any(),
Mockito.anyInt(),
Mockito.<HttpContext>any())).thenReturn(Boolean.TRUE, Boolean.FALSE);
Mockito.when(retryStrategy.getRetryInterval(
Mockito.<HttpResponse>any(),
Mockito.anyInt(),
Mockito.<HttpContext>any())).thenReturn(TimeValue.ofSeconds(1));

final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context);
retryExec.execute(request, scope, chain);

Mockito.verify(chain, Mockito.times(2)).proceed(
Mockito.<ClassicHttpRequest>any(),
Mockito.same(scope));
Mockito.verify(response, Mockito.times(1)).close();
}

@Test(expected = RuntimeException.class)
public void testStrategyRuntimeException() throws Exception {
final HttpRoute route = new HttpRoute(target);

0 comments on commit 73c1530

Please sign in to comment.