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 f668b0ac2 Log retries in ApacheTransporter (#2236)
f668b0ac2 is described below
commit f668b0ac2aeedd78748ffdccae70fef5313c9f1b
Author: Konrad Windszus <[email protected]>
AuthorDate: Wed Oct 28 09:59:59 2026 +0100
Log retries in ApacheTransporter (#2236)
When retry delay exceed the maximum delay just cap it (instead of failing
right away).
Extract logging helpers into HttpTransporter.
Add ITs for all HttpTransporters.
Same tests executed for JDK and Apache transporter.
This closes #2233
---------
Co-authored-by: Guillaume Nodet - AI Bot <[email protected]>
---
.../eclipse/aether/ConfigurationProperties.java | 16 +--
.../connector/transport/http/HttpTransporter.java | 60 ++++++++++++
maven-resolver-test-http/pom.xml | 6 +-
.../test/util/http/HttpTransporterTest.java | 109 ++++++++++++++++++++-
.../internal/test/util/http/RetryLogCapture.java | 58 +++++++++++
.../src/main/resources/logback.xml | 32 ++++++
maven-resolver-transport-apache/pom.xml | 5 -
.../aether/transport/apache/ApacheTransporter.java | 71 ++++++++++++--
.../transport/apache/ApacheTransporterTest.java | 11 +++
.../maven-resolver-transport-jdk11/pom.xml | 5 -
.../aether/transport/jdk/JdkTransporter.java | 35 ++++---
.../aether/transport/jdk/JdkTransporterTest.java | 10 ++
maven-resolver-transport-jetty/pom.xml | 5 -
.../transport/jetty/JettyTransporterTest.java | 5 +
maven-resolver-transport-url/pom.xml | 5 -
.../aether/transport/url/UrlTransporterTest.java | 5 +
16 files changed, 379 insertions(+), 59 deletions(-)
diff --git
a/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java
b/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java
index 22bc177d0..afca6686a 100644
---
a/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java
+++
b/maven-resolver-api/src/main/java/org/eclipse/aether/ConfigurationProperties.java
@@ -386,9 +386,10 @@ public final class ConfigurationProperties {
public static final int DEFAULT_HTTP_RETRY_HANDLER_COUNT = 3;
/**
- * The initial retry interval in millis of request to a remote server
should be waited in case of
- * "too many requests" (HTTP codes 429 and 503). Accepts long as
milliseconds. This value is used if remote server
- * does not use {@code Retry-After} header, in which case Server value is
obeyed.
+ * The initial delay in milliseconds for retried requests after receiving
a server response with status
+ * "too many requests" (HTTP codes 429 and 503). Accepts long as
milliseconds. This value is used if the remote server
+ * does not use {@code Retry-After} header, in which case the server value
is obeyed.
+ * This delay is increased for every retry, up to {@link
#HTTP_RETRY_HANDLER_INTERVAL_MAX}.
*
* @since 1.9.16
* @configurationSource {@link
RepositorySystemSession#getConfigProperties()}
@@ -399,7 +400,7 @@ public final class ConfigurationProperties {
public static final String HTTP_RETRY_HANDLER_INTERVAL =
PREFIX_TRANSPORT_HTTP + "retryHandler.interval";
/**
- * The default initial retry interval to use if {@link
#HTTP_RETRY_HANDLER_INTERVAL} isn't set.
+ * The default initial retry delay to use if {@link
#HTTP_RETRY_HANDLER_INTERVAL} isn't set.
* Default value 5000ms.
*
* @since 1.9.16
@@ -407,9 +408,8 @@ public final class ConfigurationProperties {
public static final long DEFAULT_HTTP_RETRY_HANDLER_INTERVAL = 5000L;
/**
- * The maximum retry interval in millis of request to a remote server
above which the request should be aborted
- * instead. In theory, a malicious server could tell Maven "come back
after 100 years" that would stall the build
- * for some. Using this parameter Maven will fail the request instead, if
interval is above this value.
+ * The maximum delay in milliseconds for retried requests. Every delay is
capped to this value.
+ * This is a security measure to avoid malicious servers from stalling the
build for too long.
*
* @since 1.9.16
* @configurationSource {@link
RepositorySystemSession#getConfigProperties()}
@@ -420,7 +420,7 @@ public final class ConfigurationProperties {
public static final String HTTP_RETRY_HANDLER_INTERVAL_MAX =
PREFIX_TRANSPORT_HTTP + "retryHandler.intervalMax";
/**
- * The default retry interval maximum to use if {@link
#HTTP_RETRY_HANDLER_INTERVAL_MAX} isn't set.
+ * The default retry delay maximum to use if {@link
#HTTP_RETRY_HANDLER_INTERVAL_MAX} isn't set.
* Default value 5 minutes.
*
* @since 1.9.16
diff --git
a/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/transport/http/HttpTransporter.java
b/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/transport/http/HttpTransporter.java
index c3ed470fa..ed776024f 100644
---
a/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/transport/http/HttpTransporter.java
+++
b/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/transport/http/HttpTransporter.java
@@ -18,7 +18,10 @@
*/
package org.eclipse.aether.spi.connector.transport.http;
+import java.net.URI;
+
import org.eclipse.aether.spi.connector.transport.Transporter;
+import org.slf4j.Logger;
/**
* A transporter using HTTP protocol.
@@ -36,4 +39,61 @@ public interface HttpTransporter extends Transporter {
}
return ERROR_OTHER;
}
+
+ /**
+ * Log a retry message for an immediate retry.
+ * @param logger the logger to log the retry message
+ * @param method the HTTP method of the request (e.g. GET, PUT, etc.)
+ * @param uri the URI of the request
+ * @param requestAttempt the 0-based counter of the previous request
attempt (0 for the first attempt, 1 for the first retry, etc.)
+ * @param maxRequestAttempt the maximum number of request attempts (this
is including this original request)
+ * @param exception the exception that caused the immediate retry
+ */
+ static void logImmediateRetry(
+ Logger logger, String method, URI uri, int requestAttempt, int
maxRequestAttempt, Throwable exception) {
+ String reason = String.format("%s: %s",
exception.getClass().getName(), exception.getMessage());
+ logRetry(logger, method, uri, requestAttempt, maxRequestAttempt,
reason, 0L, null);
+ }
+
+ /**
+ * Log a retry message for a delayed retry.
+ * @param logger the logger to log the retry message
+ * @param method the HTTP method of the request (e.g. GET, PUT, etc.)
+ * @param uri the URI of the request
+ * @param requestAttempt the 0-based counter of the previous request
attempt (0 for the first attempt, 1 for the first retry, etc.)
+ * @param maxRequestAttempt the maximum number of request attempts (this
is including this original request)
+ * @param reason the reason for the retry (usually a status code or an
exception)
+ * @param retryDelay the interval in milliseconds before the next retry (0
or negative for immediate retry)
+ * @param retryDelayReason the reason for the retry interval (may be empty
if unknown)
+ */
+ @SuppressWarnings("checkstyle:parameternumber")
+ static void logRetry(
+ Logger logger,
+ String method,
+ URI uri,
+ int requestAttempt,
+ int maxRequestAttempt,
+ String reason,
+ long retryDelay,
+ String retryDelayReason) {
+
+ final String retryingDelayMessage;
+ if (retryDelay > 0L) {
+ if (retryDelayReason != null && !retryDelayReason.isEmpty()) {
+ retryingDelayMessage = "Retrying in " + retryDelay + " ms (due
to " + retryDelayReason + ")";
+ } else {
+ retryingDelayMessage = "Retrying in " + retryDelay + " ms";
+ }
+ } else {
+ retryingDelayMessage = "Retrying immediately";
+ }
+ logger.warn(
+ "{} request to {} failed (attempt {} of {}) due to {}. {}...",
+ method,
+ uri,
+ requestAttempt + 1,
+ maxRequestAttempt,
+ reason,
+ retryingDelayMessage);
+ }
}
diff --git a/maven-resolver-test-http/pom.xml b/maven-resolver-test-http/pom.xml
index 5d7993975..174333721 100644
--- a/maven-resolver-test-http/pom.xml
+++ b/maven-resolver-test-http/pom.xml
@@ -115,9 +115,11 @@
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
</dependency>
+ <!-- used to capture log events -->
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
+ <groupId>ch.qos.logback</groupId>
+ <artifactId>logback-classic</artifactId>
+ <version>1.5.32</version>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
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 1a060c889..20d480dbc 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
@@ -39,10 +39,12 @@ import java.security.NoSuchAlgorithmException;
import java.time.Duration;
import java.util.Enumeration;
import java.util.HashMap;
+import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
+import java.util.regex.Pattern;
import java.util.stream.Stream;
import org.awaitility.Awaitility;
@@ -87,6 +89,7 @@ import org.junit.jupiter.params.provider.ValueSource;
import static java.util.Objects.requireNonNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertLinesMatch;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -97,14 +100,12 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue;
/**
* Common set of tests against Http transporter.
*/
-@SuppressWarnings({"checkstyle:MethodName"})
+@SuppressWarnings({"checkstyle:methodname", "checkstyle:sizes"})
@TestMethodOrder(MethodOrderer.MethodName.class)
public abstract class HttpTransporterTest {
protected static final Path SERVER_STORE_PATH =
Paths.get("target/server-store");
-
protected static final Path CLIENT_STORE_PATH =
Paths.get("target/client-store");
-
protected static final Path PEM_QUICHE_SERVER_PATH =
Paths.get("target/pems");
protected static SSLContext defaultSslContext;
@@ -377,6 +378,106 @@ public abstract class HttpTransporterTest {
transporter.peek(new PeekTask(URI.create("repo/file.txt")));
}
+ protected boolean supportsRetryLogging() {
+ return true;
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {429, 503})
+ protected void testRetryLogging_HttpStatus(int status) throws Exception {
+ assumeTrue(supportsRetryLogging());
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 2);
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL,
1);
+ newTransporter(httpServer.getHttpUrl());
+ httpServer.setServerErrorsBeforeWorks(2, status);
+
+ try (RetryLogCapture capture = new
RetryLogCapture(transporter.getClass())) {
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ String request = "HEAD request to " + httpServer.getHttpUrl() +
"/repo/file.txt";
+ assertLinesMatch(
+ List.of(
+ Pattern.quote(request + " failed (attempt 1 of 3)
due to status " + status
+ + ". Retrying in 1 ms")
+ + ".*\\.\\.\\.",
+ Pattern.quote(request + " failed (attempt 2 of 3)
due to status " + status
+ + ". Retrying in 2 ms")
+ + ".*\\.\\.\\."),
+ capture.messages());
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {429, 503})
+ protected void testRetryLogging_HttpStatusWithRetryAfter(int status)
throws Exception {
+ assumeTrue(supportsRetryLogging());
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 1);
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL,
1);
+ newTransporter(httpServer.getHttpUrl());
+ httpServer.setServerErrorsBeforeWorks(1, status, Map.of("Retry-After",
"1"));
+
+ try (RetryLogCapture capture = new
RetryLogCapture(transporter.getClass())) {
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ String request = "HEAD request to " + httpServer.getHttpUrl() +
"/repo/file.txt";
+ assertLinesMatch(
+ List.of(Pattern.quote(request + " failed (attempt 1 of 2)
due to status " + status
+ + ". Retrying in 1000 ms")
+ + ".*\\.\\.\\."),
+ capture.messages());
+ }
+ }
+
+ @Test
+ protected void testRetryLogging_IoException() throws Exception {
+ assumeTrue(supportsRetryLogging());
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 1);
+ newTransporter(httpServer.getHttpUrl());
+ // Exhaust the JDK client's internal retries as well as the
transporter's retries.
+ httpServer.setConnectionsToClose(100);
+
+ try (RetryLogCapture capture = new
RetryLogCapture(transporter.getClass())) {
+ assertThrows(IOException.class, () -> transporter.peek(new
PeekTask(URI.create("repo/file.txt"))));
+ String request = "HEAD request to " + httpServer.getHttpUrl() +
"/repo/file.txt";
+ assertLinesMatch(
+ List.of(Pattern.quote(request + " failed (attempt 1 of 2)
due to ") + ".*"
+ + Pattern.quote("Retrying immediately...")),
+ capture.messages());
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {0, 1})
+ protected void testRetryLogging_NoRetry(int retryCount) throws Exception {
+ assumeTrue(supportsRetryLogging());
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT,
retryCount);
+ newTransporter(httpServer.getHttpUrl());
+ httpServer.setServerErrorsBeforeWorks(1, retryCount == 0 ? 503 : 404);
+
+ try (RetryLogCapture capture = new
RetryLogCapture(transporter.getClass())) {
+ assertThrows(
+ HttpTransporterException.class, () -> transporter.peek(new
PeekTask(URI.create("repo/file.txt"))));
+ assertEquals(List.of(), capture.messages());
+ }
+ }
+
+ @Test
+ protected void testRetryLogging_IntervalTooLong() throws Exception {
+ assumeTrue(supportsRetryLogging());
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 1);
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_INTERVAL_MAX,
1);
+ newTransporter(httpServer.getHttpUrl());
+ httpServer.setServerErrorsBeforeWorks(1, 503, Map.of("Retry-After",
"1"));
+
+ try (RetryLogCapture capture = new
RetryLogCapture(transporter.getClass())) {
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ String request = "HEAD request to " + httpServer.getHttpUrl() +
"/repo/file.txt";
+ assertEquals(
+ List.of(
+ request + " failed (attempt 1 of 2) due to status
" + 503
+ + ". Retrying in 1 ms (due to Retry-After
header with seconds value 1, capped at max value)..."),
+ capture.messages());
+ }
+ }
+
@Test
protected void testRetryHandler_defaultCount_negative() throws Exception {
httpServer.setConnectionsToClose(4);
@@ -1865,9 +1966,7 @@ public abstract class HttpTransporterTest {
assertEquals("test", task.getDataString());
// there ARE challenge round-trips
assertEquals(2, httpServer.getLogEntries().size());
-
httpServer.getLogEntries().clear();
-
session.setConfigProperty(ConfigurationProperties.HTTP_PREEMPTIVE_AUTH, true);
newTransporter(httpServer.getHttpUrl());
task = new GetTask(URI.create("repo/file.txt"));
diff --git
a/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/RetryLogCapture.java
b/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/RetryLogCapture.java
new file mode 100644
index 000000000..86b21457c
--- /dev/null
+++
b/maven-resolver-test-http/src/main/java/org/eclipse/aether/internal/test/util/http/RetryLogCapture.java
@@ -0,0 +1,58 @@
+/*
+ * 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.
+ */
+package org.eclipse.aether.internal.test.util.http;
+
+import java.util.List;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.read.ListAppender;
+import org.slf4j.LoggerFactory;
+
+final class RetryLogCapture implements AutoCloseable {
+ private final Logger logger;
+
+ private final ListAppender<ILoggingEvent> appender = new ListAppender<>();
+
+ RetryLogCapture(Class<?> transporterClass) {
+ logger = (Logger) LoggerFactory.getLogger(transporterClass);
+ appender.start();
+ logger.addAppender(appender);
+ }
+
+ /**
+ * Returns the captured log messages that contain "Retrying" and are at
WARN level.
+ *
+ * @return a list of log messages
+ */
+ List<String> messages() {
+ return appender.list.stream()
+ .filter(event ->
event.getFormattedMessage().contains("Retrying"))
+ .filter(event -> event.getLevel() == Level.WARN)
+ .map(ILoggingEvent::getFormattedMessage)
+ .toList();
+ }
+
+ @Override
+ public void close() {
+ logger.detachAppender(appender);
+ appender.stop();
+ }
+}
diff --git a/maven-resolver-test-http/src/main/resources/logback.xml
b/maven-resolver-test-http/src/main/resources/logback.xml
new file mode 100644
index 000000000..6c52a5195
--- /dev/null
+++ b/maven-resolver-test-http/src/main/resources/logback.xml
@@ -0,0 +1,32 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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
+
+ https://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.
+-->
+<configuration>
+ <appender name="CONSOLE" class="ch.qos.logback.core.ConsoleAppender">
+ <filter class="ch.qos.logback.classic.filter.ThresholdFilter">
+ <level>INFO</level>
+ </filter>
+ <encoder>
+ <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} -
%msg%n</pattern>
+ </encoder>
+ </appender>
+ <root level="INFO">
+ <appender-ref ref="CONSOLE"/>
+ </root>
+</configuration>
diff --git a/maven-resolver-transport-apache/pom.xml
b/maven-resolver-transport-apache/pom.xml
index 627823066..4efc0b671 100644
--- a/maven-resolver-transport-apache/pom.xml
+++ b/maven-resolver-transport-apache/pom.xml
@@ -94,11 +94,6 @@
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
- <scope>test</scope>
- </dependency>
<dependency>
<groupId>org.apache.maven.resolver</groupId>
<artifactId>maven-resolver-test-util</artifactId>
diff --git
a/maven-resolver-transport-apache/src/main/java/org/eclipse/aether/transport/apache/ApacheTransporter.java
b/maven-resolver-transport-apache/src/main/java/org/eclipse/aether/transport/apache/ApacheTransporter.java
index 6109ac87e..c3f9c033f 100644
---
a/maven-resolver-transport-apache/src/main/java/org/eclipse/aether/transport/apache/ApacheTransporter.java
+++
b/maven-resolver-transport-apache/src/main/java/org/eclipse/aether/transport/apache/ApacheTransporter.java
@@ -282,7 +282,13 @@ final class ApacheTransporter extends AbstractTransporter
implements HttpTranspo
.setDefaultSocketConfig(socketConfig)
.setDefaultRequestConfig(requestConfig)
.setServiceUnavailableRetryStrategy(serviceUnavailableRetryStrategy)
- .setRetryHandler(retryHandler)
+ .setRetryHandler((exception, executionCount, context) -> {
+ if (retryHandler.retryRequest(exception, executionCount,
context)) {
+ logImmediateRetry(context, executionCount, retryCount,
exception);
+ return true;
+ }
+ return false;
+ })
.setDefaultAuthSchemeRegistry(authSchemeRegistry)
.setConnectionManager(state.getConnectionManager())
.setConnectionManagerShared(true)
@@ -764,6 +770,34 @@ final class ApacheTransporter extends AbstractTransporter
implements HttpTranspo
}
}
+ private static void logImmediateRetry(
+ HttpContext context, int executionCount, int retryCount, Throwable
exception) {
+ HttpCoreContext coreContext = HttpCoreContext.adapt(context);
+ HttpRequest request = coreContext.getRequest();
+ String method = request.getRequestLine().getMethod();
+ URI uri = URIUtils.resolve(
+ URI.create(coreContext.getTargetHost().toURI()),
+ request.getRequestLine().getUri());
+ HttpTransporter.logImmediateRetry(LOGGER, method, uri, executionCount
- 1, retryCount + 1, exception);
+ }
+
+ private static void logRetry(
+ HttpContext context,
+ int executionCount,
+ int retryCount,
+ String reason,
+ long retryInterval,
+ String intervalReason) {
+ HttpCoreContext coreContext = HttpCoreContext.adapt(context);
+ HttpRequest request = coreContext.getRequest();
+ String method = request.getRequestLine().getMethod();
+ URI uri = URIUtils.resolve(
+ URI.create(coreContext.getTargetHost().toURI()),
+ request.getRequestLine().getUri());
+ HttpTransporter.logRetry(
+ LOGGER, method, uri, executionCount - 1, retryCount + 1,
reason, retryInterval, intervalReason);
+ }
+
private static class ResolverServiceUnavailableRetryStrategy implements
ServiceUnavailableRetryStrategy {
private final int retryCount;
@@ -803,11 +837,8 @@ final class ApacheTransporter extends AbstractTransporter
implements HttpTranspo
&& (serviceUnavailableHttpCodes.contains(
response.getStatusLine().getStatusCode()));
if (retry) {
- Long retryInterval = retryInterval(response, executionCount,
context);
- if (retryInterval != null) {
- RETRY_INTERVAL_HOLDER.set(retryInterval);
- return true;
- }
+ RETRY_INTERVAL_HOLDER.set(retryInterval(response,
executionCount, context));
+ return true;
}
RETRY_INTERVAL_HOLDER.remove();
return false;
@@ -818,10 +849,11 @@ final class ApacheTransporter extends AbstractTransporter
implements HttpTranspo
* Otherwise, it returns {@link this#retryInterval} long value
multiplied with {@code executionCount} (starts
* from 1 and goes 2, 3,...).
*
- * @return Long representing the retry interval as millis, or {@code
null} if the request should be failed.
+ * @return long representing the retry interval as milliseconds,
capped at {@link #retryIntervalMax}.
*/
- private Long retryInterval(HttpResponse httpResponse, int
executionCount, HttpContext httpContext) {
+ private long retryInterval(HttpResponse httpResponse, int
executionCount, HttpContext httpContext) {
Long result = null;
+ String intervalReason = "";
Header header =
httpResponse.getFirstHeader(HttpHeaders.RETRY_AFTER);
if (header != null && header.getValue() != null) {
String headerValue = header.getValue();
@@ -829,21 +861,42 @@ final class ApacheTransporter extends AbstractTransporter
implements HttpTranspo
Date when = DateUtils.parseDate(headerValue); //
presumably future
if (when != null) {
result = Math.max(when.getTime() -
System.currentTimeMillis(), 0L);
+ intervalReason = "Retry-After header with date value "
+ headerValue;
+ } else {
+ LOGGER.warn(
+ "Invalid Retry-After header value {} for
request to {}",
+ headerValue,
+
httpContext.getAttribute(HttpCoreContext.HTTP_TARGET_HOST));
}
} else {
try {
result = Long.parseLong(headerValue) * 1000L; // is in
seconds
+ intervalReason = "Retry-After header with seconds
value " + headerValue;
} catch (NumberFormatException e) {
// fall through
+ LOGGER.warn(
+ "Invalid Retry-After header value {} for
request to {}",
+ headerValue,
+
httpContext.getAttribute(HttpCoreContext.HTTP_TARGET_HOST));
}
}
}
if (result == null) {
result = executionCount * this.retryInterval;
+ intervalReason =
+ "retryInterval " + this.retryInterval + " ms
multiplied by execution count " + executionCount;
}
if (result > retryIntervalMax) {
- return null;
+ intervalReason += ", capped at max value";
+ result = retryIntervalMax;
}
+ logRetry(
+ httpContext,
+ executionCount,
+ retryCount,
+ "status " + httpResponse.getStatusLine().getStatusCode(),
+ result,
+ intervalReason);
return result;
}
diff --git
a/maven-resolver-transport-apache/src/test/java/org/eclipse/aether/transport/apache/ApacheTransporterTest.java
b/maven-resolver-transport-apache/src/test/java/org/eclipse/aether/transport/apache/ApacheTransporterTest.java
index ca15acfe0..832514c32 100644
---
a/maven-resolver-transport-apache/src/test/java/org/eclipse/aether/transport/apache/ApacheTransporterTest.java
+++
b/maven-resolver-transport-apache/src/test/java/org/eclipse/aether/transport/apache/ApacheTransporterTest.java
@@ -123,6 +123,17 @@ class ApacheTransporterTest extends HttpTransporterTest {
super.tearDown();
}
+ @Override
+ protected boolean supportsRetryLogging() {
+ return true;
+ }
+
+ @Test
+ void testRetryLogging_DefaultHandler() throws Exception {
+
session.setConfigProperty(ApacheTransporterConfigurationKeys.CONFIG_PROP_HTTP_RETRY_HANDLER_NAME,
"default");
+ super.testRetryLogging_IoException();
+ }
+
@ParameterizedTest
@ValueSource(strings = {"http", "https"})
void testTransfers_SystemProxyAuthenticated(String protocol) throws
Exception {
diff --git
a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/pom.xml
b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/pom.xml
index 736c49b1d..d2626e70c 100644
--- a/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/pom.xml
+++ b/maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk11/pom.xml
@@ -69,11 +69,6 @@
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
- <scope>test</scope>
- </dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>jul-to-slf4j</artifactId>
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 d449e713c..e63a698cc 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
@@ -955,24 +955,29 @@ final class JdkTransporter extends AbstractTransporter
implements HttpTransporte
@Override
public void onRetry(Context<?> context, HttpRequest nextRequest,
Duration delay) {
- LOGGER.warn(
- "{} request to {} failed (attempt {} of {}) due to {}.
Retrying in {} ms...",
- context.request().method(),
- context.request().uri(),
- context.retryCount() + 1,
- maxNumRetries + 1,
- getReason(context),
- delay.toMillis());
- }
-
- String getReason(Context<?> context) {
if (context.exception().isPresent()) {
- return context.exception().get().getMessage();
+ HttpTransporter.logImmediateRetry(
+ LOGGER,
+ context.request().method(),
+ context.request().uri(),
+ context.retryCount(),
+ maxNumRetries + 1,
+ context.exception().get());
} else if (context.response().isPresent()) {
- return "status " + context.response().get().statusCode();
+ // Methanol does not expose the reason for the backoff delay
+ // (https://github.com/mizosoft/methanol/issues/200)
+ HttpTransporter.logRetry(
+ LOGGER,
+ context.request().method(),
+ context.request().uri(),
+ context.retryCount(),
+ maxNumRetries + 1,
+ "status " + context.response().get().statusCode(),
+ delay.toMillis(),
+ "");
+ } else {
+ throw new IllegalStateException("Retry context has neither
exception nor response");
}
- // should not happen
- throw new IllegalStateException("No exception or response present
in retry context");
}
}
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 cf0d8ab44..0a4a1ae4e 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
@@ -54,6 +54,16 @@ import static org.junit.jupiter.api.Assertions.fail;
*/
class JdkTransporterTest extends HttpTransporterTest {
+ @Override
+ protected boolean supportsRetryLogging() {
+ return true;
+ }
+
+ @Override
+ @Disabled("JDK transport caps the retry interval instead of rejecting the
retry")
+ @Test
+ protected void testRetryLogging_IntervalTooLong() {}
+
static {
// uncomment to enable http client logging
//
https://docs.oracle.com/en/java/javase/21/docs/api/java.net.http/module-summary.html#jdk.httpclient.HttpClient.log
diff --git a/maven-resolver-transport-jetty/pom.xml
b/maven-resolver-transport-jetty/pom.xml
index 10b1c4ddb..c73f775bd 100644
--- a/maven-resolver-transport-jetty/pom.xml
+++ b/maven-resolver-transport-jetty/pom.xml
@@ -115,11 +115,6 @@
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
- <scope>test</scope>
- </dependency>
<dependency>
<groupId>org.apache.maven.resolver</groupId>
<artifactId>maven-resolver-test-util</artifactId>
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 ab0f3fab3..a37742759 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
@@ -36,6 +36,11 @@ class JettyTransporterTest extends HttpTransporterTest {
return Stream.of("gzip", "deflate", "br", "zstd");
}
+ @Override
+ protected boolean supportsRetryLogging() {
+ return false; // Jetty does not yet support configurable retries
+ }
+
@Override
@Disabled
@Test
diff --git a/maven-resolver-transport-url/pom.xml
b/maven-resolver-transport-url/pom.xml
index 467175f55..7b8a5a671 100644
--- a/maven-resolver-transport-url/pom.xml
+++ b/maven-resolver-transport-url/pom.xml
@@ -56,11 +56,6 @@
<artifactId>junit-jupiter-api</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
- <scope>test</scope>
- </dependency>
<dependency>
<groupId>org.apache.maven.resolver</groupId>
<artifactId>maven-resolver-test-util</artifactId>
diff --git
a/maven-resolver-transport-url/src/test/java/org/eclipse/aether/transport/url/UrlTransporterTest.java
b/maven-resolver-transport-url/src/test/java/org/eclipse/aether/transport/url/UrlTransporterTest.java
index 97b09933e..34af7d610 100644
---
a/maven-resolver-transport-url/src/test/java/org/eclipse/aether/transport/url/UrlTransporterTest.java
+++
b/maven-resolver-transport-url/src/test/java/org/eclipse/aether/transport/url/UrlTransporterTest.java
@@ -60,6 +60,11 @@ class UrlTransporterTest extends HttpTransporterTest {
return false;
}
+ @Override
+ protected boolean supportsRetryLogging() {
+ return false;
+ }
+
@Override
@Disabled("HTTP2 unsupported")
@Test
