This is an automated email from the ASF dual-hosted git repository.
cstamas 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 70824da5 [MRESOLVER-326] Introduce retries on HTTP connection errors
(#253)
70824da5 is described below
commit 70824da545680349cc77a91f68987f6b7febfdf4
Author: Jaromir Hamala <[email protected]>
AuthorDate: Fri Feb 24 12:19:11 2023 +0100
[MRESOLVER-326] Introduce retries on HTTP connection errors (#253)
Adding request retry handler and new configuration (default 3) for it.
---
https://issues.apache.org/jira/browse/MRESOLVER-326
---
.../eclipse/aether/ConfigurationProperties.java | 12 ++++++++
.../aether/transport/http/HttpTransporter.java | 9 ++++++
.../eclipse/aether/transport/http/HttpServer.java | 19 ++++++++++++
.../aether/transport/http/HttpTransporterTest.java | 36 ++++++++++++++++++++++
src/site/markdown/configuration.md | 1 +
5 files changed, 77 insertions(+)
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 fad69b02..036f032c 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
@@ -129,6 +129,18 @@ public final class ConfigurationProperties {
*/
public static final String DEFAULT_HTTP_CREDENTIAL_ENCODING = "ISO-8859-1";
+ /**
+ * The maximum number of times a request to a remote server should be
retried in case of an error.
+ *
+ * @see #DEFAULT_HTTP_RETRY_HANDLER_COUNT
+ */
+ public static final String HTTP_RETRY_HANDLER_COUNT = PREFIX_CONNECTOR +
"http.retryHandler.count";
+
+ /**
+ * The default number of retries to use if {@link
#HTTP_RETRY_HANDLER_COUNT} isn't set.
+ */
+ public static final int DEFAULT_HTTP_RETRY_HANDLER_COUNT = 3;
+
/**
* A flag indicating whether checksums which are retrieved during checksum
validation should be persisted in the
* local filesystem next to the file they provide the checksum for.
diff --git
a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java
b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java
index 64a700c9..300f5b31 100644
---
a/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java
+++
b/maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java
@@ -67,6 +67,7 @@ import org.apache.http.impl.auth.KerberosSchemeFactory;
import org.apache.http.impl.auth.NTLMSchemeFactory;
import org.apache.http.impl.auth.SPNegoSchemeFactory;
import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.DefaultHttpRequestRetryHandler;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils;
import org.eclipse.aether.ConfigurationProperties;
@@ -165,6 +166,11 @@ final class HttpTransporter extends AbstractTransporter {
ConfigurationProperties.DEFAULT_REQUEST_TIMEOUT,
ConfigurationProperties.REQUEST_TIMEOUT + "." +
repository.getId(),
ConfigurationProperties.REQUEST_TIMEOUT);
+ int retryCount = ConfigUtils.getInteger(
+ session,
+ ConfigurationProperties.DEFAULT_HTTP_RETRY_HANDLER_COUNT,
+ ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT + "." +
repository.getId(),
+ ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT);
String userAgent = ConfigUtils.getString(
session, ConfigurationProperties.DEFAULT_USER_AGENT,
ConfigurationProperties.USER_AGENT);
@@ -187,10 +193,13 @@ final class HttpTransporter extends AbstractTransporter {
.setSocketTimeout(requestTimeout)
.build();
+ DefaultHttpRequestRetryHandler retryHandler = new
DefaultHttpRequestRetryHandler(retryCount, false);
+
this.client = HttpClientBuilder.create()
.setUserAgent(userAgent)
.setDefaultSocketConfig(socketConfig)
.setDefaultRequestConfig(requestConfig)
+ .setRetryHandler(retryHandler)
.setDefaultAuthSchemeRegistry(authSchemeRegistry)
.setConnectionManager(state.getConnectionManager())
.setConnectionManagerShared(true)
diff --git
a/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpServer.java
b/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpServer.java
index 64a578c2..3834096f 100644
---
a/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpServer.java
+++
b/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpServer.java
@@ -31,6 +31,7 @@ import java.util.Enumeration;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -38,6 +39,7 @@ import org.eclipse.aether.util.ChecksumUtils;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Response;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.eclipse.jetty.server.handler.AbstractHandler;
@@ -109,6 +111,8 @@ public class HttpServer {
private String proxyPassword;
+ private final AtomicInteger connectionsToClose = new AtomicInteger(0);
+
private List<LogEntry> logEntries = Collections.synchronizedList(new
ArrayList<LogEntry>());
public String getHost() {
@@ -191,12 +195,18 @@ public class HttpServer {
return this;
}
+ public HttpServer setConnectionsToClose(int connectionsToClose) {
+ this.connectionsToClose.set(connectionsToClose);
+ return this;
+ }
+
public HttpServer start() throws Exception {
if (server != null) {
return this;
}
HandlerList handlers = new HandlerList();
+ handlers.addHandler(new ConnectionClosingHandler());
handlers.addHandler(new LogHandler());
handlers.addHandler(new ProxyAuthHandler());
handlers.addHandler(new AuthHandler());
@@ -221,6 +231,15 @@ public class HttpServer {
}
}
+ private class ConnectionClosingHandler extends AbstractHandler {
+ public void handle(String target, Request req, HttpServletRequest
request, HttpServletResponse response) {
+ if (connectionsToClose.getAndDecrement() > 0) {
+ Response jettyResponse = (Response) response;
+ jettyResponse.getHttpChannel().getConnection().close();
+ }
+ }
+ }
+
private class LogHandler extends AbstractHandler {
public void handle(String target, Request req, HttpServletRequest
request, HttpServletResponse response) {
diff --git
a/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java
b/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java
index 46a508f3..a6557ed7 100644
---
a/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java
+++
b/maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java
@@ -29,6 +29,7 @@ import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
+import org.apache.http.NoHttpResponseException;
import org.apache.http.client.HttpResponseException;
import org.apache.http.conn.ConnectTimeoutException;
import org.apache.http.pool.ConnPoolControl;
@@ -144,6 +145,41 @@ public class HttpTransporterTest {
transporter.peek(new PeekTask(URI.create("repo/file.txt")));
}
+ @Test
+ public void testRetryHandler_defaultCount_positive() throws Exception {
+ httpServer.setConnectionsToClose(3);
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ }
+
+ @Test
+ public void testRetryHandler_defaultCount_negative() throws Exception {
+ httpServer.setConnectionsToClose(4);
+ try {
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ fail("Expected error");
+ } catch (NoHttpResponseException expected) {
+ }
+ }
+
+ @Test
+ public void testRetryHandler_explicitCount_positive() throws Exception {
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 10);
+ newTransporter(httpServer.getHttpUrl());
+ httpServer.setConnectionsToClose(10);
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ }
+
+ @Test
+ public void testRetryHandler_disabled() throws Exception {
+
session.setConfigProperty(ConfigurationProperties.HTTP_RETRY_HANDLER_COUNT, 0);
+ newTransporter(httpServer.getHttpUrl());
+ httpServer.setConnectionsToClose(1);
+ try {
+ transporter.peek(new PeekTask(URI.create("repo/file.txt")));
+ } catch (NoHttpResponseException expected) {
+ }
+ }
+
@Test
public void testPeek_NotFound() throws Exception {
try {
diff --git a/src/site/markdown/configuration.md
b/src/site/markdown/configuration.md
index 187e2568..c752e290 100644
--- a/src/site/markdown/configuration.md
+++ b/src/site/markdown/configuration.md
@@ -37,6 +37,7 @@ Option | Type | Description | Default Value | Supports Repo
ID Suffix
`aether.connector.http.cacheState` | boolean | Flag indicating whether a
memory-based cache is used for user tokens, connection managers, expect
continue requests and authentication schemes. | `true` | no
`aether.connector.http.credentialEncoding` | String | The encoding/charset to
use when exchanging credentials with HTTP servers. | `"ISO-8859-1"` | yes
`aether.connector.http.headers` | `Map<String, String>` | The request headers
to use for HTTP-based repository connectors. The headers are specified using a
map of strings mapping a header name to its value. The repository-specific
headers map is supposed to be complete, i.e. is not merged with the general
headers map. | - | yes
+`aether.connector.http.retryHandler.count` | int | The maximum number of times
a request to a remote HTTP server should be retried in case of an error. | `3`
| yes
`aether.connector.https.cipherSuites` | String | Comma-separated list of
[Cipher
Suites](https://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#ciphersuites)
which are enabled for HTTPS connections. | - (no restriction) | no
`aether.connector.https.protocols` | String | Comma-separated list of
[Protocols](https://docs.oracle.com/javase/7/docs/technotes/guides/security/StandardNames.html#jssenames)
which are enabled for HTTPS connections. | - (no restriction) | no
`aether.connector.perms.fileMode` | String | [Octal numerical notation of
permissions](https://en.wikipedia.org/wiki/File_system_permissions#Numeric_notation)
to set for newly created files. Only considered by certain Wagon providers. |
- | no
