You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2021/03/26 07:14:51 UTC
[httpcomponents-client] 03/03: HTTPCLIENT-2141: HttpClient to not
retry requests if the retry interval exceeds the response timeout
This is an automated email from the ASF dual-hosted git repository.
olegk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/httpcomponents-client.git
commit 18e39656971ecf2c0d99e0587c26688175dc7c21
Author: Oleg Kalnichevski <ol...@apache.org>
AuthorDate: Wed Mar 24 19:35:55 2021 +0100
HTTPCLIENT-2141: HttpClient to not retry requests if the retry interval exceeds the response timeout
---
.../http/impl/classic/HttpRequestRetryExec.java | 13 ++++-
.../impl/classic/TestHttpRequestRetryExec.java | 66 ++++++++++++++++++++++
2 files changed, 77 insertions(+), 2 deletions(-)
diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java
index ffed450..71e2221 100644
--- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java
+++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpRequestRetryExec.java
@@ -34,6 +34,7 @@ import org.apache.hc.client5.http.HttpRoute;
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.NoHttpResponseException;
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 class HttpRequestRetryExec implements ExecChainHandler {
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()) {
diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpRequestRetryExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpRequestRetryExec.java
index 83a1b83..2c3e919 100644
--- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpRequestRetryExec.java
+++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestHttpRequestRetryExec.java
@@ -36,6 +36,7 @@ import org.apache.hc.client5.http.classic.ExecChain;
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.HttpResponse;
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 class TestHttpRequestRetryExec {
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);