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

Reply via email to