[MRESOLVER-326] Introduce retries on HTTP connection errors

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.

maven-resolver-transport-http/src/main/java/org/eclipse/aether/transport/http/HttpTransporter.java

@@ -67,6 +67,7 @@
 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)

maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpServer.java

@@ -31,13 +31,15 @@
 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;
 
 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 enum ChecksumHeader {
 
     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 HttpServer setProxyAuthentication(String username, String password) {
         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 void stop() throws Exception {
         }
     }
 
+    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) {

maven-resolver-transport-http/src/test/java/org/eclipse/aether/transport/http/HttpTransporterTest.java

@@ -29,6 +29,7 @@
 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 void testPeek() throws Exception {
         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 {

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