This is an automated email from the ASF dual-hosted git repository.
kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
The following commit(s) were added to refs/heads/master by this push:
new 4ab57b9e1 Add retries for JDK HTTP client
4ab57b9e1 is described below
commit 4ab57b9e12fddb76909a9ddf0116b5b63c84078a
Author: Konrad Windszus <[email protected]>
AuthorDate: Wed Jan 7 20:18:57 2026 +0100
Add retries for JDK HTTP client
Add tests for retries after 429 ("Too Many Requests")
This closes #1732
---
.../aether/internal/test/util/http/HttpServer.java | 9 ++-
.../test/util/http/HttpTransporterTest.java | 39 ++++++++++
.../aether/transport/jdk/JdkTransporter.java | 84 +++++++++++++++++++++-
.../aether/transport/jdk/JdkTransporterTest.java | 20 +++---
.../transport/jetty/JettyTransporterTest.java | 10 +++
5 files changed, 151 insertions(+), 11 deletions(-)
diff --git
a/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpServer.java
b/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpServer.java
index 26928b071..b08270b43 100644
---
a/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpServer.java
+++
b/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpServer.java
@@ -139,6 +139,8 @@ public class HttpServer {
private final AtomicInteger serverErrorsBeforeWorks = new AtomicInteger(0);
+ private int serverErrorStatusCode;
+
private final List<LogEntry> logEntries = Collections.synchronizedList(new
ArrayList<>());
public String getHost() {
@@ -271,7 +273,12 @@ public class HttpServer {
}
public HttpServer setServerErrorsBeforeWorks(int serverErrorsBeforeWorks) {
+ return setServerErrorsBeforeWorks(serverErrorsBeforeWorks,
HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ }
+
+ public HttpServer setServerErrorsBeforeWorks(int serverErrorsBeforeWorks,
int errorStatusCode) {
this.serverErrorsBeforeWorks.set(serverErrorsBeforeWorks);
+ this.serverErrorStatusCode = errorStatusCode;
return this;
}
@@ -323,7 +330,7 @@ public class HttpServer {
public void handle(String target, Request req, HttpServletRequest
request, HttpServletResponse response)
throws IOException {
if (serverErrorsBeforeWorks.getAndDecrement() > 0) {
-
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ response.setStatus(serverErrorStatusCode);
writeResponseBodyMessage(response, "Oops, come back later!");
}
}
diff --git
a/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpTransporterTest.java
b/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpTransporterTest.java
index 920922e07..6f8e65294 100644
---
a/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpTransporterTest.java
+++
b/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/HttpTransporterTest.java
@@ -173,6 +173,9 @@ public class HttpTransporterTest {
protected static final long OLD_FILE_TIMESTAMP = 160660800000L;
+ /** HTTP status code for "Too Many Requests". */
+ private static final int SC_TOO_MANY_REQUESTS = 429;
+
@BeforeEach
protected void setUp(TestInfo testInfo) throws Exception {
System.out.println("=== " + testInfo.getDisplayName() + " ===");
@@ -242,6 +245,42 @@ public class HttpTransporterTest {
}
}
+ @Test
+ protected void testRetryHandler_tooManyRequests_explicitCount_positive()
throws Exception {
+ // set low retry count as this involves back off delays
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 1);
+ int retryIntervalMs = 500;
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL,
retryIntervalMs);
+ newTransporter(httpServer.getHttpUrl());
+ httpServer.setServerErrorsBeforeWorks(1, SC_TOO_MANY_REQUESTS);
+ long startTime = System.currentTimeMillis();
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ assertTrue(
+ System.currentTimeMillis() - startTime >= retryIntervalMs,
+ "Expected back off delay of at least " + retryIntervalMs);
+ }
+
+ @Test
+ protected void testRetryHandler_tooManyRequests_explicitCount_negative()
throws Exception {
+ // set low retry count as this involves back off delays
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 3);
+ int retryIntervalMs = 100;
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL,
retryIntervalMs);
+ newTransporter(httpServer.getHttpUrl());
+ httpServer.setServerErrorsBeforeWorks(4, SC_TOO_MANY_REQUESTS);
+ long startTime = System.currentTimeMillis();
+ try {
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ fail("Expected error");
+ } catch (Exception expected) {
+ }
+ // linear backoff: 1x + 2x + 3x
+ long expectedMinimumDuration = retryIntervalMs * (1 + 2 + 3);
+ assertTrue(
+ System.currentTimeMillis() - startTime >=
expectedMinimumDuration,
+ "Expected back off delay of at least " +
expectedMinimumDuration);
+ }
+
@Test
protected void testRetryHandler_explicitCount_positive() throws Exception {
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 10);
diff --git
a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java
b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java
index 316c073da..f5b2b29a4 100644
---
a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java
+++
b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java
@@ -20,6 +20,7 @@ package org.eclipse.aether.transport.jdk;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLEngine;
+import javax.net.ssl.SSLException;
import javax.net.ssl.SSLParameters;
import javax.net.ssl.X509ExtendedTrustManager;
import javax.net.ssl.X509TrustManager;
@@ -27,12 +28,14 @@ import javax.net.ssl.X509TrustManager;
import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InterruptedIOException;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.net.Authenticator;
import java.net.ConnectException;
import java.net.InetAddress;
import java.net.InetSocketAddress;
+import java.net.NoRouteToHostException;
import java.net.PasswordAuthentication;
import java.net.ProxySelector;
import java.net.Socket;
@@ -55,14 +58,17 @@ import java.time.format.DateTimeParseException;
import java.util.Base64;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.Semaphore;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.regex.Matcher;
import com.github.mizosoft.methanol.Methanol;
+import com.github.mizosoft.methanol.RetryInterceptor;
import org.eclipse.aether.ConfigurationProperties;
import org.eclipse.aether.RepositorySystemSession;
import org.eclipse.aether.repository.AuthenticationContext;
@@ -104,7 +110,7 @@ import static
org.eclipse.aether.transport.jdk.JdkTransporterConfigurationKeys.D
* <p>
* Known issues:
* <ul>
- * <li>Does not support {@link ConfigurationProperties#REQUEST_TIMEOUT},
see <a href="https://bugs.openjdk.org/browse/JDK-8258397">JDK-8258397</a></li>
+ * <li>Does not properly support {@link
ConfigurationProperties#REQUEST_TIMEOUT} prior Java 26, see <a
href="https://bugs.openjdk.org/browse/JDK-8208693">JDK-8208693</a></li>
* </ul>
* <p>
* Related: <a
href="https://dev.to/kdrakon/httpclient-can-t-connect-to-a-tls-proxy-118a">No
TLS proxy supported</a>.
@@ -120,6 +126,18 @@ final class JdkTransporter extends AbstractTransporter
implements HttpTransporte
private static final long MODIFICATION_THRESHOLD = 60L * 1000L;
+ /**
+ * Classes of IOExceptions that should not be retried (because they are
permanent failures).
+ * Same as in <a
href="https://github.com/apache/httpcomponents-client/blob/54900db4653d7f207477e6ee40135b88e9bcf832/httpclient/src/main/java/org/apache/http/impl/client/DefaultHttpRequestRetryHandler.java#L102">
+ * Apache HttpClient's DefaultHttpRequestRetryHandler</a>.
+ */
+ private static final Set<Class<? extends IOException>>
NON_RETRIABLE_IO_EXCEPTIONS = Set.of(
+ InterruptedIOException.class,
+ UnknownHostException.class,
+ ConnectException.class,
+ NoRouteToHostException.class,
+ SSLException.class);
+
private final ChecksumExtractor checksumExtractor;
private final PathProcessor pathProcessor;
@@ -525,7 +543,7 @@ final class JdkTransporter extends AbstractTransporter
implements HttpTransporte
}
}
- HttpClient.Builder builder = Methanol.newBuilder()
+ Methanol.Builder builder = Methanol.newBuilder()
.version(HttpClient.Version.valueOf(ConfigUtils.getString(
session,
DEFAULT_HTTP_VERSION,
@@ -573,9 +591,71 @@ final class JdkTransporter extends AbstractTransporter
implements HttpTransporte
});
}
+ configureRetryHandler(session, repository, builder);
+
return builder.build();
}
+ protected void configureRetryHandler(
+ RepositorySystemSession session, RemoteRepository repository,
Methanol.Builder builder) {
+ int retryCount = ConfigUtils.getInteger(
+ session,
+ ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_COUNT,
+ ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT + "." +
repository.getId(),
+ ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT);
+ long retryInterval = ConfigUtils.getLong(
+ session,
+ ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_INTERVAL,
+ ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL + "." +
repository.getId(),
+ ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL);
+ long retryIntervalMax = ConfigUtils.getLong(
+ session,
+
ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_INTERVAL_MAX,
+ ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL_MAX + "."
+ repository.getId(),
+ ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL_MAX);
+ if (retryCount < 0) {
+ throw new IllegalArgumentException("retryCount must be >= 0");
+ }
+ if (retryInterval < 0L) {
+ throw new IllegalArgumentException("retryInterval must be >= 0");
+ }
+ if (retryIntervalMax < 0L) {
+ throw new IllegalArgumentException("retryIntervalMax must be >=
0");
+ }
+ String serviceUnavailableCodesString = ConfigUtils.getString(
+ session,
+
ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE,
+ ConfigurationProperties.HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE
+ "." + repository.getId(),
+
ConfigurationProperties.HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE);
+ Set<Integer> serviceUnavailableCodes = new HashSet<>();
+ try {
+ for (String code :
ConfigUtils.parseCommaSeparatedUniqueNames(serviceUnavailableCodesString)) {
+ serviceUnavailableCodes.add(Integer.parseInt(code));
+ }
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException(
+ "Illegal HTTP codes for " +
ConfigurationProperties.HTTP_RETRY_HANDLER_SERVICE_UNAVAILABLE
+ + " (list of integers): " +
serviceUnavailableCodesString);
+ }
+ if (retryCount > 0) {
+ Methanol.Interceptor rateLimitingRetryInterceptor =
RetryInterceptor.newBuilder()
+ .maxRetries(retryCount)
+ .onStatus(serviceUnavailableCodes::contains)
+ .backoff(RetryInterceptor.BackoffStrategy.linear(
+ Duration.ofMillis(retryInterval),
Duration.ofMillis(retryIntervalMax)))
+ .build();
+ builder.interceptor(rateLimitingRetryInterceptor);
+ Methanol.Interceptor retryIoExceptionsInterceptor =
RetryInterceptor.newBuilder()
+ // this is in addition to the JDK internal retries
(https://github.com/mizosoft/methanol/issues/174)
+ // e.g. for connection timeouts this is hardcoded to 2
attempts:
+ //
https://github.com/openjdk/jdk/blob/640343f7d94894b0378ea5b1768eeac203a9aaf8/src/java.net.http/share/classes/jdk/internal/net/http/MultiExchange.java#L665
+ .maxRetries(retryCount)
+ .onException(t -> t instanceof IOException &&
!NON_RETRIABLE_IO_EXCEPTIONS.contains(t.getClass()))
+ .build();
+ builder.interceptor(retryIoExceptionsInterceptor);
+ }
+ }
+
private static InetAddress getHttpLocalAddress(RepositorySystemSession
session, RemoteRepository repository) {
String bindAddress = ConfigUtils.getString(
session,
diff --git
a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/test/java/org/eclipse/aether/transport/jdk/JdkTransporterTest.java
b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/test/java/org/eclipse/aether/transport/jdk/JdkTransporterTest.java
index dd166935c..c480e8f55 100644
---
a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/test/java/org/eclipse/aether/transport/jdk/JdkTransporterTest.java
+++
b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/src/test/java/org/eclipse/aether/transport/jdk/JdkTransporterTest.java
@@ -87,17 +87,21 @@ class JdkTransporterTest extends HttpTransporterTest {
@Override
@Disabled
@Test
- protected void testRetryHandler_defaultCount_positive() {}
-
- @Override
- @Disabled
- @Test
- protected void testRetryHandler_explicitCount_positive() {}
+ protected void
testPut_Authenticated_ExpectContinueRejected_ExplicitlyConfiguredHeader() {}
@Override
- @Disabled
@Test
- protected void
testPut_Authenticated_ExpectContinueRejected_ExplicitlyConfiguredHeader() {}
+ protected void testRetryHandler_defaultCount_negative() throws Exception {
+ // internally JDK client uses its own retry mechanism with 1 retry for
ConnectionExpiredException, therefore
+ // close more connections to trigger failed retries
+ // Compare with https://github.com/mizosoft/methanol/issues/174
+ httpServer.setConnectionsToClose(8);
+ try {
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ fail("Expected error");
+ } catch (Exception expected) {
+ }
+ }
public JdkTransporterTest() {
super(() -> new JdkTransporterFactory(standardChecksumExtractor(), new
DefaultPathProcessor()));
diff --git
a/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java
b/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java
index ec9af47c2..94bfaa92e 100644
---
a/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java
+++
b/maven-resolver-transport-jetty/src/test/java/org/eclipse/aether/transport/jetty/JettyTransporterTest.java
@@ -53,6 +53,16 @@ class JettyTransporterTest extends HttpTransporterTest {
@Test
protected void testRetryHandler_explicitCount_positive() {}
+ @Override
+ @Disabled
+ @Test
+ protected void testRetryHandler_tooManyRequests_explicitCount_positive() {}
+
+ @Override
+ @Disabled
+ @Test
+ protected void testRetryHandler_tooManyRequests_explicitCount_negative() {}
+
@Override
@Disabled
@Test