From 382faef5e5cd5cd07b7efd2a6fe33ec4d527bbf0 Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Wed, 16 Jul 2025 09:58:11 +0200 Subject: [PATCH] HTTPCLIENT-2371: Improved request re-execution strategy --- .../async/TestHttp1RequestReExecution.java | 6 +- .../async/StandardTestClientBuilder.java | 6 +- .../async/TestAsyncClientBuilder.java | 4 +- .../sync/StandardTestClientBuilder.java | 6 +- .../extension/sync/TestClientBuilder.java | 4 +- .../sync/TestClientRequestExecution.java | 43 +-- .../sync/TestClientRequestReExecution.java | 7 +- .../client5/testing/sync/TestRedirects.java | 4 +- .../http/HttpRequestRetryStrategy.java | 37 +++ .../http/RequestReExecutionStrategy.java | 78 ++++++ .../impl/DefaultHttpRequestRetryStrategy.java | 8 +- .../DefaultRequestReExecutionStrategy.java | 256 ++++++++++++++++++ .../impl/async/AsyncHttpRequestRetryExec.java | 79 +++--- .../http/impl/async/H2AsyncClientBuilder.java | 39 ++- .../impl/async/HttpAsyncClientBuilder.java | 39 ++- .../http/impl/classic/HttpClientBuilder.java | 39 ++- .../impl/classic/HttpRequestRetryExec.java | 21 +- ...estDefaultRequestReExecutionStrategy.java} | 88 ++++-- .../classic/TestHttpRequestRetryExec.java | 52 ++-- 19 files changed, 639 insertions(+), 177 deletions(-) create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/RequestReExecutionStrategy.java create mode 100644 httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRequestReExecutionStrategy.java rename httpclient5/src/test/java/org/apache/hc/client5/http/impl/{TestDefaultHttpRequestRetryStrategy.java => TestDefaultRequestReExecutionStrategy.java} (57%) diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1RequestReExecution.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1RequestReExecution.java index 4b00ee0202..2046b6dd10 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1RequestReExecution.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/async/TestHttp1RequestReExecution.java @@ -33,7 +33,7 @@ import org.apache.hc.client5.http.async.methods.SimpleHttpResponse; import org.apache.hc.client5.http.async.methods.SimpleRequestBuilder; -import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy; +import org.apache.hc.client5.http.impl.DefaultRequestReExecutionStrategy; import org.apache.hc.client5.testing.extension.async.ClientProtocolLevel; import org.apache.hc.client5.testing.extension.async.ServerProtocolLevel; import org.apache.hc.client5.testing.extension.async.TestAsyncClient; @@ -81,7 +81,7 @@ void testGiveUpAfterOneRetry() throws Exception { final HttpHost target = startServer(); configureClient(builder -> builder - .setRetryStrategy(new DefaultHttpRequestRetryStrategy(1, TimeValue.ofSeconds(1)))); + .setReExecutionStrategy(new DefaultRequestReExecutionStrategy(1, TimeValue.ofSeconds(1)))); final TestAsyncClient client = startClient(); final Future future = client.execute( @@ -100,7 +100,7 @@ void testDoNotGiveUpEasily() throws Exception { final HttpHost target = startServer(); configureClient(builder -> builder - .setRetryStrategy(new DefaultHttpRequestRetryStrategy(5, TimeValue.ofSeconds(1)))); + .setReExecutionStrategy(new DefaultRequestReExecutionStrategy(5, TimeValue.ofSeconds(1)))); final TestAsyncClient client = startClient(); final Future future = client.execute( diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/StandardTestClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/StandardTestClientBuilder.java index efaa6d1081..c44b43c5da 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/StandardTestClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/async/StandardTestClientBuilder.java @@ -31,7 +31,7 @@ import java.util.Collection; import org.apache.hc.client5.http.AuthenticationStrategy; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.RequestReExecutionStrategy; import org.apache.hc.client5.http.UserTokenHandler; import org.apache.hc.client5.http.auth.AuthSchemeFactory; import org.apache.hc.client5.http.auth.CredentialsProvider; @@ -146,8 +146,8 @@ public TestAsyncClientBuilder setDefaultHeaders(final Collection de throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); } - default TestAsyncClientBuilder setRetryStrategy(HttpRequestRetryStrategy retryStrategy) { + default TestAsyncClientBuilder setReExecutionStrategy(RequestReExecutionStrategy reExecutionStrategy) { throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/StandardTestClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/StandardTestClientBuilder.java index 44bae7ab15..693d38104d 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/StandardTestClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/StandardTestClientBuilder.java @@ -31,7 +31,7 @@ import java.util.Collection; import org.apache.hc.client5.http.AuthenticationStrategy; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.RequestReExecutionStrategy; import org.apache.hc.client5.http.UserTokenHandler; import org.apache.hc.client5.http.auth.AuthSchemeFactory; import org.apache.hc.client5.http.auth.CredentialsProvider; @@ -119,8 +119,8 @@ public TestClientBuilder setDefaultHeaders(final Collection de } @Override - public TestClientBuilder setRetryStrategy(final HttpRequestRetryStrategy retryStrategy) { - this.clientBuilder.setRetryStrategy(retryStrategy); + public TestClientBuilder setReExecutionStrategy(final RequestReExecutionStrategy reExecutionStrategy) { + this.clientBuilder.setRequestReExecutionStrategy(reExecutionStrategy); return this; } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java index f84d15de35..f57f013695 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/extension/sync/TestClientBuilder.java @@ -31,7 +31,7 @@ import java.util.Collection; import org.apache.hc.client5.http.AuthenticationStrategy; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.RequestReExecutionStrategy; import org.apache.hc.client5.http.UserTokenHandler; import org.apache.hc.client5.http.auth.AuthSchemeFactory; import org.apache.hc.client5.http.auth.CredentialsProvider; @@ -77,7 +77,7 @@ default TestClientBuilder setDefaultHeaders(Collection default throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); } - default TestClientBuilder setRetryStrategy(HttpRequestRetryStrategy retryStrategy) { + default TestClientBuilder setReExecutionStrategy(RequestReExecutionStrategy reExecutionStrategy) { throw new UnsupportedOperationException("Operation not supported by " + getProtocolLevel()); } diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java index 17173a4253..0f2b244369 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestExecution.java @@ -29,12 +29,13 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.net.URI; +import java.util.Optional; import java.util.Random; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.RequestReExecutionStrategy; import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager; @@ -141,31 +142,23 @@ void testAutoGeneratedHeaders() throws Exception { final HttpRequestInterceptor interceptor = (request, entityDetails, context) -> request.addHeader("my-header", "stuff"); - final HttpRequestRetryStrategy requestRetryStrategy = new HttpRequestRetryStrategy() { + final RequestReExecutionStrategy requestRetryStrategy = new RequestReExecutionStrategy() { @Override - public boolean retryRequest( + public Optional reExecute( final HttpRequest request, final IOException exception, final int executionCount, final HttpContext context) { - return true; + return Optional.of(TimeValue.ZERO_MILLISECONDS); } @Override - public boolean retryRequest( + public Optional reExecute( final HttpResponse response, final int executionCount, final HttpContext context) { - return false; - } - - @Override - public TimeValue getRetryInterval( - final HttpResponse response, - final int executionCount, - final HttpContext context) { - return TimeValue.ofSeconds(1L); + return Optional.empty(); } }; @@ -173,7 +166,7 @@ public TimeValue getRetryInterval( configureClient(builder -> builder .addRequestInterceptorFirst(interceptor) .setRequestExecutor(new FaultyHttpRequestExecutor("Oppsie")) - .setRetryStrategy(requestRetryStrategy) + .setReExecutionStrategy(requestRetryStrategy) ); final TestClient client = client(); @@ -199,38 +192,30 @@ void testNonRepeatableEntity() throws Exception { configureServer(bootstrap -> bootstrap.register("*", new SimpleService())); final HttpHost target = startServer(); - final HttpRequestRetryStrategy requestRetryStrategy = new HttpRequestRetryStrategy() { + final RequestReExecutionStrategy requestRetryStrategy = new RequestReExecutionStrategy() { @Override - public boolean retryRequest( + public Optional reExecute( final HttpRequest request, final IOException exception, final int executionCount, final HttpContext context) { - return true; - } - - @Override - public boolean retryRequest( - final HttpResponse response, - final int executionCount, - final HttpContext context) { - return false; + return Optional.of(TimeValue.ZERO_MILLISECONDS); } @Override - public TimeValue getRetryInterval( + public Optional reExecute( final HttpResponse response, final int executionCount, final HttpContext context) { - return TimeValue.ofSeconds(1L); + return Optional.empty(); } }; configureClient(builder -> builder .setRequestExecutor(new FaultyHttpRequestExecutor("a message showing that this failed")) - .setRetryStrategy(requestRetryStrategy) + .setReExecutionStrategy(requestRetryStrategy) ); final TestClient client = client(); diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestReExecution.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestReExecution.java index 30b36b82d4..5714d9b0e7 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestReExecution.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestClientRequestReExecution.java @@ -29,7 +29,8 @@ import static org.hamcrest.MatcherAssert.assertThat; import java.util.concurrent.atomic.AtomicInteger; -import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy; + +import org.apache.hc.client5.http.impl.DefaultRequestReExecutionStrategy; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.client5.testing.classic.RandomHandler; import org.apache.hc.client5.testing.classic.ServiceUnavailableDecorator; @@ -79,7 +80,7 @@ void testGiveUpAfterOneRetry() throws Exception { final HttpHost target = startServer(); configureClient(builder -> builder - .setRetryStrategy(new DefaultHttpRequestRetryStrategy(1, TimeValue.ofSeconds(1)))); + .setReExecutionStrategy(new DefaultRequestReExecutionStrategy(1, TimeValue.ofSeconds(1)))); final TestClient client = client(); final HttpClientContext context = HttpClientContext.create(); @@ -100,7 +101,7 @@ void testDoNotGiveUpEasily() throws Exception { final HttpHost target = startServer(); configureClient(builder -> builder - .setRetryStrategy(new DefaultHttpRequestRetryStrategy(5, TimeValue.ofSeconds(1)))); + .setReExecutionStrategy(new DefaultRequestReExecutionStrategy(5, TimeValue.ofSeconds(1)))); final TestClient client = client(); final HttpClientContext context = HttpClientContext.create(); diff --git a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java index 1371dd8d0b..9226e01be2 100644 --- a/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java +++ b/httpclient5-testing/src/test/java/org/apache/hc/client5/testing/sync/TestRedirects.java @@ -49,7 +49,7 @@ import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.cookie.BasicCookieStore; import org.apache.hc.client5.http.cookie.CookieStore; -import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy; +import org.apache.hc.client5.http.impl.DefaultRequestReExecutionStrategy; import org.apache.hc.client5.http.impl.LaxRedirectStrategy; import org.apache.hc.client5.http.impl.cookie.BasicClientCookie; import org.apache.hc.client5.http.protocol.HttpClientContext; @@ -655,7 +655,7 @@ public void handle(final ClassicHttpRequest request, @Test void testRetryUponRedirect() throws Exception { configureClient(builder -> builder - .setRetryStrategy(new DefaultHttpRequestRetryStrategy( + .setReExecutionStrategy(new DefaultRequestReExecutionStrategy( 3, TimeValue.ofSeconds(1), Arrays.asList( diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/HttpRequestRetryStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/HttpRequestRetryStrategy.java index 3fcd9912c5..6cffb3b249 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/HttpRequestRetryStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/HttpRequestRetryStrategy.java @@ -28,6 +28,7 @@ package org.apache.hc.client5.http; import java.io.IOException; +import java.util.Optional; import org.apache.hc.core5.annotation.Contract; import org.apache.hc.core5.annotation.ThreadingBehavior; @@ -42,8 +43,11 @@ * it should be done and so on. * * @since 5.0 + * + * @deprecated Use {@link RequestReExecutionStrategy}. */ @Contract(threading = ThreadingBehavior.STATELESS) +@Deprecated public interface HttpRequestRetryStrategy { /** @@ -103,4 +107,37 @@ default TimeValue getRetryInterval(HttpRequest request, IOException exception, i */ TimeValue getRetryInterval(HttpResponse response, int execCount, HttpContext context); + static RequestReExecutionStrategy adaptor(final HttpRequestRetryStrategy retryStrategy) { + + return retryStrategy != null ? new RequestReExecutionStrategy() { + + @Override + public Optional reExecute( + final HttpRequest request, + final IOException exception, + final int execCount, + final HttpContext context) { + if (retryStrategy.retryRequest(request, exception, execCount, context)) { + return Optional.ofNullable(retryStrategy.getRetryInterval(request, exception, execCount, context)); + } else { + return Optional.empty(); + } + } + + @Override + public Optional reExecute( + final HttpResponse response, + final int execCount, + final HttpContext context) { + if (retryStrategy.retryRequest(response, execCount, context)) { + return Optional.ofNullable(retryStrategy.getRetryInterval(response, execCount, context)); + } else { + return Optional.empty(); + } + } + + } : null; + + } + } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/RequestReExecutionStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/RequestReExecutionStrategy.java new file mode 100644 index 0000000000..bedf81ade8 --- /dev/null +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/RequestReExecutionStrategy.java @@ -0,0 +1,78 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ + +package org.apache.hc.client5.http; + +import java.io.IOException; +import java.util.Optional; + +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.TimeValue; + +/** + * Strategy interface to determine if the request should automatically reexecuted + * in case of an I/O error or a recoverable response status code. + * + * @since 5.6 + */ +@Contract(threading = ThreadingBehavior.STATELESS) +public interface RequestReExecutionStrategy { + + /** + * Determines if a method should be re-executed after an I/O exception + * occurred during execution. + * + * @param request the request failed due to an I/O exception + * @param exception the exception that occurred + * @param execCount the number of times this method has been + * unsuccessfully executed + * @param context the context for the request execution + * + * @return a time interval to pause before the request re-execution + * or {@link Optional#empty()} if the request is not to be re-executed + */ + Optional reExecute(HttpRequest request, IOException exception, int execCount, HttpContext context); + + /** + * Determines if a method should be re-executed given the response from + * the target server. + * + * @param response the response from the target server + * @param execCount the number of times this method has been + * unsuccessfully executed + * @param context the context for the request execution + * + * @return a time interval to pause before the request re-execution + * or {@link Optional#empty()} if the request is not to be re-executed + */ + Optional reExecute(HttpResponse response, int execCount, HttpContext context); + +} diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultHttpRequestRetryStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultHttpRequestRetryStrategy.java index 47c9bab567..0052c1cdb4 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultHttpRequestRetryStrategy.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultHttpRequestRetryStrategy.java @@ -40,7 +40,6 @@ import javax.net.ssl.SSLException; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.client5.http.utils.DateUtils; @@ -60,12 +59,15 @@ import org.apache.hc.core5.util.Timeout; /** - * Default implementation of the {@link HttpRequestRetryStrategy} interface. + * Default implementation of the {@link org.apache.hc.client5.http.HttpRequestRetryStrategy} interface. * * @since 5.0 + * + * @deprecated Use {@link org.apache.hc.client5.http.RequestReExecutionStrategy} */ +@Deprecated @Contract(threading = ThreadingBehavior.STATELESS) -public class DefaultHttpRequestRetryStrategy implements HttpRequestRetryStrategy { +public class DefaultHttpRequestRetryStrategy implements org.apache.hc.client5.http.HttpRequestRetryStrategy { /** * Default instance of {@link DefaultHttpRequestRetryStrategy}. diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRequestReExecutionStrategy.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRequestReExecutionStrategy.java new file mode 100644 index 0000000000..d1115d0a77 --- /dev/null +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/DefaultRequestReExecutionStrategy.java @@ -0,0 +1,256 @@ +/* + * ==================================================================== + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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 + * + * http://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. + * ==================================================================== + * + * This software consists of voluntary contributions made by many + * individuals on behalf of the Apache Software Foundation. For more + * information on the Apache Software Foundation, please see + * . + * + */ + +package org.apache.hc.client5.http.impl; + +import java.io.IOException; +import java.io.InterruptedIOException; +import java.net.ConnectException; +import java.net.NoRouteToHostException; +import java.net.UnknownHostException; +import java.time.Instant; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.Optional; +import java.util.Set; + +import javax.net.ssl.SSLException; + +import org.apache.hc.client5.http.RequestReExecutionStrategy; +import org.apache.hc.client5.http.config.RequestConfig; +import org.apache.hc.client5.http.protocol.HttpClientContext; +import org.apache.hc.client5.http.utils.DateUtils; +import org.apache.hc.core5.annotation.Contract; +import org.apache.hc.core5.annotation.ThreadingBehavior; +import org.apache.hc.core5.concurrent.CancellableDependency; +import org.apache.hc.core5.http.ConnectionClosedException; +import org.apache.hc.core5.http.Header; +import org.apache.hc.core5.http.HttpHeaders; +import org.apache.hc.core5.http.HttpRequest; +import org.apache.hc.core5.http.HttpResponse; +import org.apache.hc.core5.http.HttpStatus; +import org.apache.hc.core5.http.Method; +import org.apache.hc.core5.http.protocol.HttpContext; +import org.apache.hc.core5.util.Args; +import org.apache.hc.core5.util.TimeValue; +import org.apache.hc.core5.util.Timeout; + +/** + * Default implementation of the {@link RequestReExecutionStrategy} interface. + * + * @since 5.6 + */ +@Contract(threading = ThreadingBehavior.STATELESS) +public class DefaultRequestReExecutionStrategy implements RequestReExecutionStrategy { + + /** + * Default instance of {@link DefaultRequestReExecutionStrategy}. + */ + public static final DefaultRequestReExecutionStrategy INSTANCE = new DefaultRequestReExecutionStrategy(); + + /** + * Maximum number of allowed retries + */ + private final int maxRetries; + + /** + * Retry interval between subsequent retries + */ + private final TimeValue defaultRetryInterval; + + /** + * Derived {@code IOExceptions} which shall not be retried + */ + private final Set> nonRetriableIOExceptionClasses; + + /** + * HTTP status codes which shall be retried + */ + private final Set retriableCodes; + + protected DefaultRequestReExecutionStrategy( + final int maxRetries, + final TimeValue defaultRetryInterval, + final Collection> clazzes, + final Collection codes) { + Args.notNegative(maxRetries, "maxRetries"); + Args.notNull(defaultRetryInterval, "defaultRetryInterval"); + Args.check(TimeValue.isNonNegative(defaultRetryInterval), "Default retry interval is negative"); + this.maxRetries = maxRetries; + this.defaultRetryInterval = defaultRetryInterval; + this.nonRetriableIOExceptionClasses = new HashSet<>(clazzes); + this.retriableCodes = new HashSet<>(codes); + } + + /** + * Create the HTTP request retry strategy using the following list of + * non-retriable I/O exception classes:
+ *
    + *
  • InterruptedIOException
    • + *
  • UnknownHostException
    • + *
  • ConnectException
    • + *
  • ConnectionClosedException
    • + *
  • NoRouteToHostException
    • + *
  • SSLException
    • + *
+ * + * and retriable HTTP status codes:
+ *
    + *
  • SC_TOO_MANY_REQUESTS (429)
    • + *
  • SC_SERVICE_UNAVAILABLE (503)
    • + *
+ * + * @param maxRetries how many times to retry; 0 means no retries + * @param defaultRetryInterval the default retry interval between + * subsequent retries if the {@code Retry-After} header is not set + * or invalid. + */ + public DefaultRequestReExecutionStrategy( + final int maxRetries, + final TimeValue defaultRetryInterval) { + this(maxRetries, defaultRetryInterval, + Arrays.asList( + InterruptedIOException.class, + UnknownHostException.class, + ConnectException.class, + ConnectionClosedException.class, + NoRouteToHostException.class, + SSLException.class), + Arrays.asList( + HttpStatus.SC_TOO_MANY_REQUESTS, + HttpStatus.SC_SERVICE_UNAVAILABLE)); + } + + /** + * Create the HTTP request retry strategy with a max retry count of 1, + * default retry interval of 1 second, and using the following list of + * non-retriable I/O exception classes:
+ *
    + *
  • InterruptedIOException
    • + *
  • UnknownHostException
    • + *
  • ConnectException
    • + *
  • ConnectionClosedException
    • + *
  • SSLException
    • + *
+ * + * and retriable HTTP status codes:
+ *
    + *
  • SC_TOO_MANY_REQUESTS (429)
    • + *
  • SC_SERVICE_UNAVAILABLE (503)
    • + *
+ */ + public DefaultRequestReExecutionStrategy() { + this(1, TimeValue.ofSeconds(1L)); + } + + @Override + public Optional reExecute( + final HttpRequest request, + final IOException exception, + final int execCount, + final HttpContext context) { + Args.notNull(request, "request"); + Args.notNull(exception, "exception"); + + if (execCount > this.maxRetries) { + // Do not retry if over max retries + return Optional.empty(); + } + if (this.nonRetriableIOExceptionClasses.contains(exception.getClass())) { + return Optional.empty(); + } + for (final Class rejectException : this.nonRetriableIOExceptionClasses) { + if (rejectException.isInstance(exception)) { + return Optional.empty(); + } + } + if (request instanceof CancellableDependency && ((CancellableDependency) request).isCancelled()) { + return Optional.empty(); + } + + // Retry if the request is considered idempotent + if (handleAsIdempotent(request)) { + return Optional.of(getRetryInterval(request, execCount, context)); + } else { + return Optional.empty(); + } + } + + @Override + public Optional reExecute( + final HttpResponse response, + final int execCount, + final HttpContext context) { + Args.notNull(response, "response"); + if (execCount <= this.maxRetries && retriableCodes.contains(response.getCode())) { + final TimeValue retryInterval = getRetryInterval(response, execCount, context); + final HttpClientContext clientContext = HttpClientContext.cast(context); + final RequestConfig requestConfig = clientContext != null ? clientContext.getRequestConfig() : null; + final Timeout responseTimeout = requestConfig != null ? requestConfig.getResponseTimeout() : null; + if (responseTimeout == null || responseTimeout.compareTo(retryInterval) >= 0) { + return Optional.of(retryInterval); + } + } + return Optional.empty(); + } + + protected TimeValue getRetryInterval(final HttpResponse response, + final int execCount, + final HttpContext context) { + final Header header = response.getFirstHeader(HttpHeaders.RETRY_AFTER); + TimeValue retryAfter = null; + if (header != null) { + final String value = header.getValue(); + try { + retryAfter = TimeValue.ofSeconds(Long.parseLong(value)); + } catch (final NumberFormatException ignore) { + final Instant retryAfterDate = DateUtils.parseStandardDate(value); + if (retryAfterDate != null) { + retryAfter = + TimeValue.ofMilliseconds(retryAfterDate.toEpochMilli() - System.currentTimeMillis()); + } + } + + if (TimeValue.isPositive(retryAfter)) { + return retryAfter; + } + } + return this.defaultRetryInterval; + } + + protected boolean handleAsIdempotent(final HttpRequest request) { + return Method.isIdempotent(request.getMethod()); + } + + protected TimeValue getRetryInterval(final HttpRequest request, + final int execCount, + final HttpContext context) { + return TimeValue.ZERO_MILLISECONDS; + } + +} diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java index 325567cd6e..0252a4daa9 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncHttpRequestRetryExec.java @@ -27,8 +27,9 @@ package org.apache.hc.client5.http.impl.async; import java.io.IOException; +import java.util.Optional; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.RequestReExecutionStrategy; import org.apache.hc.client5.http.async.AsyncExecCallback; import org.apache.hc.client5.http.async.AsyncExecChain; import org.apache.hc.client5.http.async.AsyncExecChainHandler; @@ -86,11 +87,11 @@ public final class AsyncHttpRequestRetryExec implements AsyncExecChainHandler { private static final Logger LOG = LoggerFactory.getLogger(AsyncHttpRequestRetryExec.class); - private final HttpRequestRetryStrategy retryStrategy; + private final RequestReExecutionStrategy reExecutionStrategy; - public AsyncHttpRequestRetryExec(final HttpRequestRetryStrategy retryStrategy) { - Args.notNull(retryStrategy, "retryStrategy"); - this.retryStrategy = retryStrategy; + public AsyncHttpRequestRetryExec(final RequestReExecutionStrategy reExecutionStrategy) { + Args.notNull(reExecutionStrategy, "reExecutionStrategy"); + this.reExecutionStrategy = reExecutionStrategy; } private static class State { @@ -99,6 +100,12 @@ private static class State { volatile int status; volatile TimeValue delay; + void reset() { + retrying = false; + status = 0; + delay = null; + } + } private void internalExecute( @@ -124,11 +131,14 @@ public AsyncDataConsumer handleResponse( } return asyncExecCallback.handleResponse(response, entityDetails); } - state.retrying = retryStrategy.retryRequest(response, scope.execCount.get(), clientContext); - if (state.retrying) { + final Optional reExecInterval = reExecutionStrategy.reExecute(response, scope.execCount.get(), clientContext); + if (reExecInterval.isPresent()) { + state.retrying = true; state.status = response.getCode(); - state.delay = retryStrategy.getRetryInterval(response, scope.execCount.get(), clientContext); + state.delay = reExecInterval.get(); return new DiscardingEntityConsumer<>(); + } else { + state.reset(); } return asyncExecCallback.handleResponse(response, entityDetails); } @@ -173,31 +183,36 @@ public void failed(final Exception cause) { if (LOG.isDebugEnabled()) { LOG.debug("{} cannot retry non-repeatable request", exchangeId); } - } else if (retryStrategy.retryRequest(request, (IOException) cause, scope.execCount.get(), clientContext)) { - if (LOG.isDebugEnabled()) { - LOG.debug("{} {}", exchangeId, cause.getMessage(), cause); - } - scope.execRuntime.discardEndpoint(); - if (entityProducer != null) { - entityProducer.releaseResources(); - } - state.retrying = true; - final int execCount = scope.execCount.incrementAndGet(); - state.delay = retryStrategy.getRetryInterval(request, (IOException) cause, execCount - 1, clientContext); - final TimeValue delay = TimeValue.isPositive(state.delay) ? state.delay : TimeValue.ZERO_MILLISECONDS; - if (LOG.isInfoEnabled()) { - LOG.info("{} recoverable I/O exception ({}) caught when sending request to {};" + - "request will be automatically re-executed in {} (exec count {})", - exchangeId, cause.getClass().getName(), target, delay, execCount); + } else { + final Optional reExecInterval = reExecutionStrategy.reExecute(request, (IOException) cause, scope.execCount.get(), clientContext); + if (reExecInterval.isPresent()) { + if (LOG.isDebugEnabled()) { + LOG.debug("{} {}", exchangeId, cause.getMessage(), cause); + } + scope.execRuntime.discardEndpoint(); + if (entityProducer != null) { + entityProducer.releaseResources(); + } + state.retrying = true; + final int execCount = scope.execCount.incrementAndGet(); + state.delay = reExecInterval.get(); + final TimeValue delay = TimeValue.isPositive(state.delay) ? state.delay : TimeValue.ZERO_MILLISECONDS; + if (LOG.isInfoEnabled()) { + LOG.info("{} recoverable I/O exception ({}) caught when sending request to {};" + + "request will be automatically re-executed in {} (exec count {})", + exchangeId, cause.getClass().getName(), target, delay, execCount); + } + scope.scheduler.scheduleExecution( + request, + entityProducer, + scope, + (r, e, s, c) -> execute(r, e, s, chain, c), + asyncExecCallback, + delay); + return; + } else { + state.reset(); } - scope.scheduler.scheduleExecution( - request, - entityProducer, - scope, - (r, e, s, c) -> execute(r, e, s, chain, c), - asyncExecCallback, - delay); - return; } } asyncExecCallback.failed(cause); diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java index 37174b457b..a3215a5fcd 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/H2AsyncClientBuilder.java @@ -39,7 +39,7 @@ import org.apache.hc.client5.http.AuthenticationStrategy; import org.apache.hc.client5.http.DnsResolver; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.RequestReExecutionStrategy; import org.apache.hc.client5.http.SchemePortResolver; import org.apache.hc.client5.http.async.AsyncExecChainHandler; import org.apache.hc.client5.http.auth.AuthSchemeFactory; @@ -53,8 +53,8 @@ import org.apache.hc.client5.http.impl.ChainElement; import org.apache.hc.client5.http.impl.CookieSpecSupport; import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy; -import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy; import org.apache.hc.client5.http.impl.DefaultRedirectStrategy; +import org.apache.hc.client5.http.impl.DefaultRequestReExecutionStrategy; import org.apache.hc.client5.http.impl.DefaultSchemePortResolver; import org.apache.hc.client5.http.impl.auth.BasicCredentialsProvider; import org.apache.hc.client5.http.impl.auth.BasicSchemeFactory; @@ -183,7 +183,7 @@ private ExecInterceptorEntry( private HttpRoutePlanner routePlanner; private RedirectStrategy redirectStrategy; - private HttpRequestRetryStrategy retryStrategy; + private RequestReExecutionStrategy reExecutionStrategy; private Lookup authSchemeRegistry; private Lookup cookieSpecRegistry; @@ -445,16 +445,35 @@ public final H2AsyncClientBuilder addRequestInterceptorLast(final HttpRequestInt } /** - * Sets {@link HttpRequestRetryStrategy} instance. + * Sets {@link org.apache.hc.client5.http.HttpRequestRetryStrategy} instance. *

* Please note this value can be overridden by the {@link #disableAutomaticRetries()} * method. *

* * @return this instance. + * + * @deprecated Use {@link #setRequestReExecutionStrategy(RequestReExecutionStrategy)} + */ + @Deprecated + public final H2AsyncClientBuilder setRetryStrategy(final org.apache.hc.client5.http.HttpRequestRetryStrategy retryStrategy) { + this.reExecutionStrategy = org.apache.hc.client5.http.HttpRequestRetryStrategy.adaptor(retryStrategy); + return this; + } + + /** + * Sets {@link RequestReExecutionStrategy} instance. + *

+ * Please note this value can be overridden by the {@link #disableAutomaticRetries()} + * method. + *

+ * + * @return this instance. + * + * @since 5.6 */ - public final H2AsyncClientBuilder setRetryStrategy(final HttpRequestRetryStrategy retryStrategy) { - this.retryStrategy = retryStrategy; + public final H2AsyncClientBuilder setRequestReExecutionStrategy(final RequestReExecutionStrategy reExecutionStrategy) { + this.reExecutionStrategy = reExecutionStrategy; return this; } @@ -814,12 +833,12 @@ public CloseableHttpAsyncClient build() { // Add request retry executor, if not disabled if (!automaticRetriesDisabled) { - HttpRequestRetryStrategy retryStrategyCopy = this.retryStrategy; - if (retryStrategyCopy == null) { - retryStrategyCopy = DefaultHttpRequestRetryStrategy.INSTANCE; + RequestReExecutionStrategy reExecutionStrategyCopy = this.reExecutionStrategy; + if (reExecutionStrategyCopy == null) { + reExecutionStrategyCopy = DefaultRequestReExecutionStrategy.INSTANCE; } execChainDefinition.addFirst( - new AsyncHttpRequestRetryExec(retryStrategyCopy), + new AsyncHttpRequestRetryExec(reExecutionStrategyCopy), ChainElement.RETRY.name()); } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java index bb73238e45..168a3e208c 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/HttpAsyncClientBuilder.java @@ -40,7 +40,7 @@ import org.apache.hc.client5.http.AuthenticationStrategy; import org.apache.hc.client5.http.ConnectionKeepAliveStrategy; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.RequestReExecutionStrategy; import org.apache.hc.client5.http.SchemePortResolver; import org.apache.hc.client5.http.UserTokenHandler; import org.apache.hc.client5.http.async.AsyncExecChainHandler; @@ -57,8 +57,8 @@ import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy; import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy; import org.apache.hc.client5.http.impl.DefaultConnectionKeepAliveStrategy; -import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy; import org.apache.hc.client5.http.impl.DefaultRedirectStrategy; +import org.apache.hc.client5.http.impl.DefaultRequestReExecutionStrategy; import org.apache.hc.client5.http.impl.DefaultSchemePortResolver; import org.apache.hc.client5.http.impl.DefaultUserTokenHandler; import org.apache.hc.client5.http.impl.IdleConnectionEvictor; @@ -229,7 +229,7 @@ private ExecInterceptorEntry( private HttpRoutePlanner routePlanner; private RedirectStrategy redirectStrategy; - private HttpRequestRetryStrategy retryStrategy; + private RequestReExecutionStrategy reExecutionStrategy; private ConnectionReuseStrategy reuseStrategy; @@ -576,16 +576,35 @@ public final HttpAsyncClientBuilder addRequestInterceptorLast(final HttpRequestI } /** - * Sets {@link HttpRequestRetryStrategy} instance. + * Sets {@link org.apache.hc.client5.http.HttpRequestRetryStrategy} instance. *

* Please note this value can be overridden by the {@link #disableAutomaticRetries()} * method. *

* * @return this instance. + * + * @deprecated Use {@link #setRequestReExecutionStrategy(RequestReExecutionStrategy)} + */ + @Deprecated + public final HttpAsyncClientBuilder setRetryStrategy(final org.apache.hc.client5.http.HttpRequestRetryStrategy retryStrategy) { + this.reExecutionStrategy = org.apache.hc.client5.http.HttpRequestRetryStrategy.adaptor(retryStrategy); + return this; + } + + /** + * Sets {@link RequestReExecutionStrategy} instance. + *

+ * Please note this value can be overridden by the {@link #disableAutomaticRetries()} + * method. + *

+ * + * @return this instance. + * + * @since 5.6 */ - public final HttpAsyncClientBuilder setRetryStrategy(final HttpRequestRetryStrategy retryStrategy) { - this.retryStrategy = retryStrategy; + public final HttpAsyncClientBuilder setRequestReExecutionStrategy(final RequestReExecutionStrategy reExecutionStrategy) { + this.reExecutionStrategy = reExecutionStrategy; return this; } @@ -992,12 +1011,12 @@ public CloseableHttpAsyncClient build() { // Add request retry executor, if not disabled if (!automaticRetriesDisabled) { - HttpRequestRetryStrategy retryStrategyCopy = this.retryStrategy; - if (retryStrategyCopy == null) { - retryStrategyCopy = DefaultHttpRequestRetryStrategy.INSTANCE; + RequestReExecutionStrategy reExecutionStrategyCopy = this.reExecutionStrategy; + if (reExecutionStrategyCopy == null) { + reExecutionStrategyCopy = DefaultRequestReExecutionStrategy.INSTANCE; } execChainDefinition.addFirst( - new AsyncHttpRequestRetryExec(retryStrategyCopy), + new AsyncHttpRequestRetryExec(reExecutionStrategyCopy), ChainElement.RETRY.name()); } diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java index a9483d6132..a55a4f3851 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/HttpClientBuilder.java @@ -42,7 +42,7 @@ import org.apache.hc.client5.http.AuthenticationStrategy; import org.apache.hc.client5.http.ConnectionKeepAliveStrategy; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; +import org.apache.hc.client5.http.RequestReExecutionStrategy; import org.apache.hc.client5.http.SchemePortResolver; import org.apache.hc.client5.http.UserTokenHandler; import org.apache.hc.client5.http.auth.AuthSchemeFactory; @@ -62,8 +62,8 @@ import org.apache.hc.client5.http.impl.DefaultAuthenticationStrategy; import org.apache.hc.client5.http.impl.DefaultClientConnectionReuseStrategy; import org.apache.hc.client5.http.impl.DefaultConnectionKeepAliveStrategy; -import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy; import org.apache.hc.client5.http.impl.DefaultRedirectStrategy; +import org.apache.hc.client5.http.impl.DefaultRequestReExecutionStrategy; import org.apache.hc.client5.http.impl.DefaultSchemePortResolver; import org.apache.hc.client5.http.impl.DefaultUserTokenHandler; import org.apache.hc.client5.http.impl.IdleConnectionEvictor; @@ -207,7 +207,7 @@ private ExecInterceptorEntry( private LinkedList responseInterceptors; private LinkedList execInterceptors; - private HttpRequestRetryStrategy retryStrategy; + private RequestReExecutionStrategy reExecutionStrategy; private HttpRoutePlanner routePlanner; private RedirectStrategy redirectStrategy; private ConnectionBackoffStrategy connectionBackoffStrategy; @@ -552,16 +552,35 @@ public final HttpClientBuilder disableAuthCaching() { } /** - * Sets {@link HttpRequestRetryStrategy} instance. + * Sets {@link org.apache.hc.client5.http.HttpRequestRetryStrategy} instance. *

* Please note this value can be overridden by the {@link #disableAutomaticRetries()} * method. *

* * @return this instance. + * + * @deprecated Use #RequestReExecutionStrategy + */ + @Deprecated + public final HttpClientBuilder setRetryStrategy(final org.apache.hc.client5.http.HttpRequestRetryStrategy retryStrategy) { + this.reExecutionStrategy = org.apache.hc.client5.http.HttpRequestRetryStrategy.adaptor(retryStrategy); + return this; + } + + /** + * Sets {@link RequestReExecutionStrategy} instance. + *

+ * Please note this value can be overridden by the {@link #disableAutomaticRetries()} + * method. + *

+ * + * @return this instance. + * + * @since 5.6 */ - public final HttpClientBuilder setRetryStrategy(final HttpRequestRetryStrategy retryStrategy) { - this.retryStrategy = retryStrategy; + public final HttpClientBuilder setRequestReExecutionStrategy(final RequestReExecutionStrategy reExecutionStrategy) { + this.reExecutionStrategy = reExecutionStrategy; return this; } @@ -994,12 +1013,12 @@ public CloseableHttpClient build() { // Add request retry executor, if not disabled if (!automaticRetriesDisabled) { - HttpRequestRetryStrategy retryStrategyCopy = this.retryStrategy; - if (retryStrategyCopy == null) { - retryStrategyCopy = DefaultHttpRequestRetryStrategy.INSTANCE; + RequestReExecutionStrategy reExecutionStrategyCopy = this.reExecutionStrategy; + if (reExecutionStrategyCopy == null) { + reExecutionStrategyCopy = DefaultRequestReExecutionStrategy.INSTANCE; } execChainDefinition.addFirst( - new HttpRequestRetryExec(retryStrategyCopy), + new HttpRequestRetryExec(reExecutionStrategyCopy), ChainElement.RETRY.name()); } 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 3dc801dd36..edb04edb80 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 @@ -28,9 +28,10 @@ import java.io.IOException; import java.io.InterruptedIOException; +import java.util.Optional; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; import org.apache.hc.client5.http.HttpRoute; +import org.apache.hc.client5.http.RequestReExecutionStrategy; 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; @@ -86,12 +87,12 @@ public class HttpRequestRetryExec implements ExecChainHandler { private static final Logger LOG = LoggerFactory.getLogger(HttpRequestRetryExec.class); - private final HttpRequestRetryStrategy retryStrategy; + private final RequestReExecutionStrategy reExecutionStrategy; public HttpRequestRetryExec( - final HttpRequestRetryStrategy retryStrategy) { - Args.notNull(retryStrategy, "retryStrategy"); - this.retryStrategy = retryStrategy; + final RequestReExecutionStrategy reExecutionStrategy) { + Args.notNull(reExecutionStrategy, "reExecutionStrategy"); + this.reExecutionStrategy = reExecutionStrategy; } @Override @@ -118,9 +119,10 @@ public ClassicHttpResponse execute( } return response; } - if (retryStrategy.retryRequest(response, execCount, context)) { + final Optional reExecInterval = reExecutionStrategy.reExecute(response, execCount, context); + if (reExecInterval.isPresent()) { response.close(); - final TimeValue delay = retryStrategy.getRetryInterval(response, execCount, context); + final TimeValue delay = reExecInterval.get(); if (LOG.isInfoEnabled()) { LOG.info("{} {} responded with status {}; " + "request will be automatically re-executed in {} (exec count {})", @@ -146,11 +148,12 @@ public ClassicHttpResponse execute( } throw ex; } - if (retryStrategy.retryRequest(request, ex, execCount, context)) { + final Optional reExecInterval = reExecutionStrategy.reExecute(request, ex, execCount, context); + if (reExecInterval.isPresent()) { if (LOG.isDebugEnabled()) { LOG.debug("{} {}", exchangeId, ex.getMessage(), ex); } - final TimeValue delay = retryStrategy.getRetryInterval(request, ex, execCount, context); + final TimeValue delay = reExecInterval.get(); if (LOG.isInfoEnabled()) { LOG.info("{} recoverable I/O exception ({}) caught when sending request to {};" + "request will be automatically re-executed in {} (exec count {})", diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryStrategy.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRequestReExecutionStrategy.java similarity index 57% rename from httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryStrategy.java rename to httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRequestReExecutionStrategy.java index 4c2649b524..66348df6ca 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultHttpRequestRetryStrategy.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/TestDefaultRequestReExecutionStrategy.java @@ -33,7 +33,10 @@ import java.net.UnknownHostException; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.Optional; + import javax.net.ssl.SSLException; + import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.protocol.HttpClientContext; @@ -48,31 +51,42 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -class TestDefaultHttpRequestRetryStrategy { +class TestDefaultRequestReExecutionStrategy { - private DefaultHttpRequestRetryStrategy retryStrategy; + private DefaultRequestReExecutionStrategy reExecutionStrategy; @BeforeEach void setup() { - this.retryStrategy = new DefaultHttpRequestRetryStrategy(3, TimeValue.ofMilliseconds(1234L)); + reExecutionStrategy = new DefaultRequestReExecutionStrategy(3, TimeValue.ofMilliseconds(1234L)); } @Test void testBasics() { final HttpResponse response1 = new BasicHttpResponse(503, "Oopsie"); - Assertions.assertTrue(this.retryStrategy.retryRequest(response1, 1, null)); - Assertions.assertTrue(this.retryStrategy.retryRequest(response1, 2, null)); - Assertions.assertTrue(this.retryStrategy.retryRequest(response1, 3, null)); - Assertions.assertFalse(this.retryStrategy.retryRequest(response1, 4, null)); + Assertions.assertEquals(Optional.of(TimeValue.ofMilliseconds(1234L)), + reExecutionStrategy.reExecute(response1, 1, null)); + Assertions.assertEquals(Optional.of(TimeValue.ofMilliseconds(1234L)), + reExecutionStrategy.reExecute(response1, 2, null)); + Assertions.assertEquals(Optional.of(TimeValue.ofMilliseconds(1234L)), + reExecutionStrategy.reExecute(response1, 3, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(response1, 4, null)); + final HttpResponse response2 = new BasicHttpResponse(500, "Big Time Oopsie"); - Assertions.assertFalse(this.retryStrategy.retryRequest(response2, 1, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(response2, 1, null)); final HttpResponse response3 = new BasicHttpResponse(429, "Oopsie"); - Assertions.assertTrue(this.retryStrategy.retryRequest(response3, 1, null)); - Assertions.assertTrue(this.retryStrategy.retryRequest(response3, 2, null)); - Assertions.assertTrue(this.retryStrategy.retryRequest(response3, 3, null)); - Assertions.assertFalse(this.retryStrategy.retryRequest(response3, 4, null)); - - Assertions.assertEquals(TimeValue.ofMilliseconds(1234L), this.retryStrategy.getRetryInterval(response1, 1, null)); + Assertions.assertEquals(Optional.of(TimeValue.ofMilliseconds(1234L)), + reExecutionStrategy.reExecute(response3, 1, null)); + Assertions.assertEquals(Optional.of(TimeValue.ofMilliseconds(1234L)), + reExecutionStrategy.reExecute(response3, 2, null)); + Assertions.assertEquals(Optional.of(TimeValue.ofMilliseconds(1234L)), + reExecutionStrategy.reExecute(response3, 3, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(response3, 4, null)); + + Assertions.assertEquals(TimeValue.ofMilliseconds(1234L), + reExecutionStrategy.getRetryInterval(response1, 1, null)); } @Test @@ -83,19 +97,22 @@ void testRetryRequestWithResponseTimeout() { context.setRequestConfig(RequestConfig.custom() .build()); - Assertions.assertTrue(retryStrategy.retryRequest(response, 1, context)); + Assertions.assertEquals(Optional.of(TimeValue.ofMilliseconds(1234L)), + reExecutionStrategy.reExecute(response, 1, context)); context.setRequestConfig(RequestConfig.custom() .setResponseTimeout(Timeout.ofMilliseconds(1234L)) .build()); - Assertions.assertTrue(retryStrategy.retryRequest(response, 1, context)); + Assertions.assertEquals(Optional.of(TimeValue.ofMilliseconds(1234L)), + reExecutionStrategy.reExecute(response, 1, context)); context.setRequestConfig(RequestConfig.custom() .setResponseTimeout(Timeout.ofMilliseconds(1233L)) .build()); - Assertions.assertFalse(retryStrategy.retryRequest(response, 1, context)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(response, 1, context)); } @Test @@ -103,16 +120,17 @@ void testRetryAfterHeaderAsLong() { final HttpResponse response = new BasicHttpResponse(503, "Oopsie"); response.setHeader(HttpHeaders.RETRY_AFTER, "321"); - Assertions.assertEquals(TimeValue.ofSeconds(321L), this.retryStrategy.getRetryInterval(response, 3, null)); + Assertions.assertEquals(TimeValue.ofSeconds(321L), + reExecutionStrategy.getRetryInterval(response, 3, null)); } @Test void testRetryAfterHeaderAsDate() { - this.retryStrategy = new DefaultHttpRequestRetryStrategy(3, TimeValue.ZERO_MILLISECONDS); + reExecutionStrategy = new DefaultRequestReExecutionStrategy(3, TimeValue.ZERO_MILLISECONDS); final HttpResponse response = new BasicHttpResponse(503, "Oopsie"); response.setHeader(HttpHeaders.RETRY_AFTER, DateUtils.formatStandardDate(Instant.now().plus(100, ChronoUnit.SECONDS))); - Assertions.assertTrue(this.retryStrategy.getRetryInterval(response, 3, null).compareTo(TimeValue.ZERO_MILLISECONDS) > 0); + Assertions.assertTrue(reExecutionStrategy.getRetryInterval(response, 3, null).compareTo(TimeValue.ZERO_MILLISECONDS) > 0); } @Test @@ -120,7 +138,8 @@ void testRetryAfterHeaderAsPastDate() { final HttpResponse response = new BasicHttpResponse(503, "Oopsie"); response.setHeader(HttpHeaders.RETRY_AFTER, DateUtils.formatStandardDate(Instant.now().minus(100, ChronoUnit.SECONDS))); - Assertions.assertEquals(TimeValue.ofMilliseconds(1234L), this.retryStrategy.getRetryInterval(response, 3, null)); + Assertions.assertEquals(TimeValue.ofMilliseconds(1234L), + reExecutionStrategy.getRetryInterval(response, 3, null)); } @Test @@ -128,49 +147,56 @@ void testInvalidRetryAfterHeader() { final HttpResponse response = new BasicHttpResponse(503, "Oopsie"); response.setHeader(HttpHeaders.RETRY_AFTER, "Stuff"); - Assertions.assertEquals(TimeValue.ofMilliseconds(1234L), retryStrategy.getRetryInterval(response, 3, null)); + Assertions.assertEquals(TimeValue.ofMilliseconds(1234L), + reExecutionStrategy.getRetryInterval(response, 3, null)); } @Test void noRetryOnConnectTimeout() { final HttpGet request = new HttpGet("/"); - Assertions.assertFalse(retryStrategy.retryRequest(request, new SocketTimeoutException(), 1, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(request, new SocketTimeoutException(), 1, null)); } @Test void noRetryOnConnect() { final HttpGet request = new HttpGet("/"); - Assertions.assertFalse(retryStrategy.retryRequest(request, new ConnectException(), 1, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(request, new ConnectException(), 1, null)); } @Test void noRetryOnConnectionClosed() { final HttpGet request = new HttpGet("/"); - Assertions.assertFalse(retryStrategy.retryRequest(request, new ConnectionClosedException(), 1, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(request, new ConnectionClosedException(), 1, null)); } @Test void noRetryForNoRouteToHostException() { final HttpGet request = new HttpGet("/"); - Assertions.assertFalse(retryStrategy.retryRequest(request, new NoRouteToHostException(), 1, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(request, new NoRouteToHostException(), 1, null)); } @Test void noRetryOnSSLFailure() { final HttpGet request = new HttpGet("/"); - Assertions.assertFalse(retryStrategy.retryRequest(request, new SSLException("encryption failed"), 1, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(request, new SSLException("encryption failed"), 1, null)); } @Test void noRetryOnUnknownHost() { final HttpGet request = new HttpGet("/"); - Assertions.assertFalse(retryStrategy.retryRequest(request, new UnknownHostException(), 1, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(request, new UnknownHostException(), 1, null)); } @Test @@ -178,14 +204,16 @@ void noRetryOnAbortedRequests() { final HttpGet request = new HttpGet("/"); request.cancel(); - Assertions.assertFalse(retryStrategy.retryRequest(request, new IOException(), 1, null)); + Assertions.assertEquals(Optional.empty(), + reExecutionStrategy.reExecute(request, new IOException(), 1, null)); } @Test void retryOnNonAbortedRequests() { final HttpGet request = new HttpGet("/"); - Assertions.assertTrue(retryStrategy.retryRequest(request, new IOException(), 1, null)); + Assertions.assertEquals(Optional.of(TimeValue.ZERO_MILLISECONDS), + reExecutionStrategy.reExecute(request, new IOException(), 1, null)); } } 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 006ef9a506..85761cad2f 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 @@ -29,9 +29,10 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.util.Optional; -import org.apache.hc.client5.http.HttpRequestRetryStrategy; import org.apache.hc.client5.http.HttpRoute; +import org.apache.hc.client5.http.RequestReExecutionStrategy; 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; @@ -56,7 +57,7 @@ class TestHttpRequestRetryExec { @Mock - private HttpRequestRetryStrategy retryStrategy; + private RequestReExecutionStrategy reExecutionStrategy; @Mock private ExecChain chain; @Mock @@ -70,11 +71,10 @@ class TestHttpRequestRetryExec { @BeforeEach void setup() { MockitoAnnotations.openMocks(this); - retryExec = new HttpRequestRetryExec(retryStrategy); + retryExec = new HttpRequestRetryExec(reExecutionStrategy); target = new HttpHost("localhost", 80); } - @Test void testFundamentals1() throws Exception { final HttpRoute route = new HttpRoute(target); @@ -86,14 +86,14 @@ void testFundamentals1() throws Exception { Mockito.when(chain.proceed( Mockito.same(request), Mockito.any())).thenReturn(response); - Mockito.when(retryStrategy.retryRequest( + Mockito.when(reExecutionStrategy.reExecute( Mockito.any(), - Mockito.anyInt(), - Mockito.any())).thenReturn(Boolean.TRUE, Boolean.FALSE); - Mockito.when(retryStrategy.getRetryInterval( + Mockito.eq(1), + Mockito.any())).thenReturn(Optional.of(TimeValue.ZERO_MILLISECONDS)); + Mockito.when(reExecutionStrategy.reExecute( Mockito.any(), - Mockito.anyInt(), - Mockito.any())).thenReturn(TimeValue.ZERO_MILLISECONDS); + Mockito.intThat(i -> i > 1), + Mockito.any())).thenReturn(Optional.empty()); final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context); retryExec.execute(request, scope, chain); @@ -115,16 +115,16 @@ void testRetrySleepOnIOException() throws Exception { Mockito.when(chain.proceed( Mockito.same(request), Mockito.any())).thenThrow(new IOException("Ka-boom")); - Mockito.when(retryStrategy.retryRequest( + Mockito.when(reExecutionStrategy.reExecute( Mockito.any(), Mockito.any(), - Mockito.anyInt(), - Mockito.any())).thenReturn(Boolean.TRUE, Boolean.FALSE); - Mockito.when(retryStrategy.getRetryInterval( + Mockito.eq(1), + Mockito.any())).thenReturn(Optional.of(nextInterval)); + Mockito.when(reExecutionStrategy.reExecute( Mockito.any(), Mockito.any(), - Mockito.anyInt(), - Mockito.any())).thenReturn(nextInterval); + Mockito.intThat(i -> i > 1), + Mockito.any())).thenReturn(Optional.empty()); Mockito.when(nextInterval.getDuration()).thenReturn(100L); Mockito.when(nextInterval.compareTo(Mockito.any())).thenReturn(-1); @@ -151,14 +151,14 @@ void testRetryIntervalResponseTimeoutNull() throws Exception { Mockito.when(chain.proceed( Mockito.same(request), Mockito.any())).thenReturn(response); - Mockito.when(retryStrategy.retryRequest( + Mockito.when(reExecutionStrategy.reExecute( Mockito.any(), - Mockito.anyInt(), - Mockito.any())).thenReturn(Boolean.TRUE, Boolean.FALSE); - Mockito.when(retryStrategy.getRetryInterval( + Mockito.eq(1), + Mockito.any())).thenReturn(Optional.of(TimeValue.ofSeconds(1))); + Mockito.when(reExecutionStrategy.reExecute( Mockito.any(), - Mockito.anyInt(), - Mockito.any())).thenReturn(TimeValue.ofSeconds(1)); + Mockito.intThat(i -> i > 1), + Mockito.any())).thenReturn(Optional.empty()); final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, endpoint, context); retryExec.execute(request, scope, chain); @@ -179,7 +179,7 @@ void testStrategyRuntimeException() throws Exception { Mockito.when(chain.proceed( Mockito.any(), Mockito.any())).thenReturn(response); - Mockito.doThrow(new RuntimeException("Ooopsie")).when(retryStrategy).retryRequest( + Mockito.doThrow(new RuntimeException("Ooopsie")).when(reExecutionStrategy).reExecute( Mockito.any(), Mockito.anyInt(), Mockito.any()); @@ -231,11 +231,11 @@ void testFundamentals2() throws Exception { wrapper.addHeader("Cookie", "monster"); throw new IOException("Ka-boom"); }); - Mockito.when(retryStrategy.retryRequest( + Mockito.when(reExecutionStrategy.reExecute( Mockito.any(), Mockito.any(), Mockito.eq(1), - Mockito.any())).thenReturn(Boolean.TRUE); + Mockito.any())).thenReturn(Optional.of(TimeValue.ZERO_MILLISECONDS)); final ExecChain.Scope scope = new ExecChain.Scope("test", route, originalRequest, endpoint, context); final ClassicHttpRequest request = ClassicRequestBuilder.copy(originalRequest).build(); Assertions.assertThrows(IOException.class, () -> @@ -264,7 +264,7 @@ void testAbortedRequest() throws Exception { Mockito.verify(chain, Mockito.times(1)).proceed( Mockito.same(request), Mockito.same(scope)); - Mockito.verify(retryStrategy, Mockito.never()).retryRequest( + Mockito.verify(reExecutionStrategy, Mockito.never()).reExecute( Mockito.any(), Mockito.any(), Mockito.anyInt(),