This is an automated email from the ASF dual-hosted git repository. kwin pushed a commit to branch feature/jdk-http-transport-retries in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
commit 1c19b6878413c4862adfc5aa1f7d48fa827deef9 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 | 16 +++++ .../aether/transport/jdk/JdkTransporter.java | 80 +++++++++++++++++++++- .../aether/transport/jdk/JdkTransporterTest.java | 19 ++--- 4 files changed, 113 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..cb3e03387 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 @@ -242,6 +242,22 @@ public class HttpTransporterTest { } } + @Test + protected void testRetryHandler_serviceunavailable_defaultCount_positive() throws Exception { + httpServer.setServerErrorsBeforeWorks(3, 429); + transporter.peek(new PeekTask(URI.create("repo/file.txt"))); + } + + @Test + protected void testRetryHandler_serviceunavailable_defaultCount_negative() throws Exception { + httpServer.setServerErrorsBeforeWorks(4, 429); + try { + transporter.peek(new PeekTask(URI.create("repo/file.txt"))); + fail("Expected error"); + } catch (Exception expected) { + } + } + @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..a1584972e 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 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,6 +591,64 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte }); } + 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)) + .withJitter()) + .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); + } + return builder.build(); } 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..d57e4f311 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,20 @@ 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 + 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()));
