[MRESOLVER-627] Improve transport selection and logging (#599)

In previous releases transport selection was really secretive and broken, as it considered invalid configuration as "protocol not supported" and Resolver did not log anything or worse, silently skipped to new transport.

Changes:
* transport provider: was logging ONLY if transport selection failed (no transport was selected), changed to continuously log (in DEBUG) why a transport was ignored and next selection performed. Also, in case of total failure, exceptions are added now as suppressed ones instead just lost.
* multiple transporter factories: smaller fixes for earlier protocol detection (file is exception to support cases like JIMFS), and when client creation is attempted but it fails due runtime exception, fail the whole process (like bad enum in JDK transport config).
* before all transporter "blanket" threw NoTransporterEx and Basic connector was "protecting" caller from it, hence to continue doing so, no basic connector "blankets" all runtime and non-runtime exceptions coming from transport to "early report" problem as it happens in basic connector ctor.

---

https://issues.apache.org/jira/browse/MRESOLVER-627

maven-resolver-connector-basic/src/main/java/org/eclipse/aether/connector/basic/BasicRepositoryConnector.java

@@ -58,7 +58,6 @@
 import org.eclipse.aether.transfer.ChecksumFailureException;
 import org.eclipse.aether.transfer.NoRepositoryConnectorException;
 import org.eclipse.aether.transfer.NoRepositoryLayoutException;
-import org.eclipse.aether.transfer.NoTransporterException;
 import org.eclipse.aether.transfer.TransferEvent;
 import org.eclipse.aether.transfer.TransferResource;
 import org.eclipse.aether.util.ConfigUtils;
@@ -130,7 +129,10 @@ final class BasicRepositoryConnector implements RepositoryConnector {
         }
         try {
             transporter = transporterProvider.newTransporter(session, repository);
-        } catch (NoTransporterException e) {
+        } catch (RuntimeException e) {
+            throw new NoRepositoryConnectorException(
+                    repository, "Transporter configuration issue: " + e.getMessage(), e);
+        } catch (Exception e) {
             throw new NoRepositoryConnectorException(repository, e.getMessage(), e);
         }
         this.checksumPolicyProvider = checksumPolicyProvider;

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultLocalRepositoryProvider.java

@@ -83,12 +83,8 @@ public LocalRepositoryManager newLocalRepositoryManager(RepositorySystemSession
                 return manager;
             } catch (NoLocalRepositoryManagerException e) {
                 // continue and try next factory
-                errors.add(e);
-            }
-        }
-        if (LOGGER.isDebugEnabled() && errors.size() > 1) {
-            for (Exception e : errors) {
                 LOGGER.debug("Could not obtain local repository manager for {}", repository, e);
+                errors.add(e);
             }
         }
 
@@ -102,7 +98,13 @@ public LocalRepositoryManager newLocalRepositoryManager(RepositorySystemSession
             factories.list(buffer);
         }
 
-        throw new NoLocalRepositoryManagerException(
+        // create exception: if one error, make it cause
+        NoLocalRepositoryManagerException ex = new NoLocalRepositoryManagerException(
                 repository, buffer.toString(), errors.size() == 1 ? errors.get(0) : null);
+        // if more errors, make them all suppressed
+        if (errors.size() > 1) {
+            errors.forEach(ex::addSuppressed);
+        }
+        throw ex;
     }
 }

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java

@@ -31,8 +31,6 @@
 import org.eclipse.aether.impl.RemoteRepositoryFilterManager;
 import org.eclipse.aether.impl.RepositoryConnectorProvider;
 import org.eclipse.aether.internal.impl.filter.FilteringRepositoryConnector;
-import org.eclipse.aether.repository.Authentication;
-import org.eclipse.aether.repository.Proxy;
 import org.eclipse.aether.repository.RemoteRepository;
 import org.eclipse.aether.spi.connector.RepositoryConnector;
 import org.eclipse.aether.spi.connector.RepositoryConnectorFactory;
@@ -93,25 +91,6 @@ public RepositoryConnector newRepositoryConnector(RepositorySystemSession sessio
                     Utils.appendClassLoader(buffer, connector);
                     buffer.append(" with priority ").append(factory.getPriority());
                     buffer.append(" for ").append(repository.getUrl());
-
-                    Authentication auth = repository.getAuthentication();
-                    if (auth != null) {
-                        buffer.append(" with ").append(auth);
-                    }
-
-                    Proxy proxy = repository.getProxy();
-                    if (proxy != null) {
-                        buffer.append(" via ")
-                                .append(proxy.getHost())
-                                .append(':')
-                                .append(proxy.getPort());
-
-                        auth = proxy.getAuthentication();
-                        if (auth != null) {
-                            buffer.append(" with ").append(auth);
-                        }
-                    }
-
                     LOGGER.debug(buffer.toString());
                 }
 
@@ -122,12 +101,8 @@ public RepositoryConnector newRepositoryConnector(RepositorySystemSession sessio
                 }
             } catch (NoRepositoryConnectorException e) {
                 // continue and try next factory
-                errors.add(e);
-            }
-        }
-        if (LOGGER.isDebugEnabled() && errors.size() > 1) {
-            for (Exception e : errors) {
                 LOGGER.debug("Could not obtain connector factory for {}", repository, e);
+                errors.add(e);
             }
         }
 
@@ -141,7 +116,13 @@ public RepositoryConnector newRepositoryConnector(RepositorySystemSession sessio
             factories.list(buffer);
         }
 
-        throw new NoRepositoryConnectorException(
+        // create exception: if one error, make it cause
+        NoRepositoryConnectorException ex = new NoRepositoryConnectorException(
                 repository, buffer.toString(), errors.size() == 1 ? errors.get(0) : null);
+        // if more errors, make them all suppressed
+        if (errors.size() > 1) {
+            errors.forEach(ex::addSuppressed);
+        }
+        throw ex;
     }
 }

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryLayoutProvider.java

@@ -68,12 +68,8 @@ public RepositoryLayout newRepositoryLayout(RepositorySystemSession session, Rem
                 return factory.getComponent().newInstance(session, repository);
             } catch (NoRepositoryLayoutException e) {
                 // continue and try next factory
-                errors.add(e);
-            }
-        }
-        if (LOGGER.isDebugEnabled() && errors.size() > 1) {
-            for (Exception e : errors) {
                 LOGGER.debug("Could not obtain layout factory for {}", repository, e);
+                errors.add(e);
             }
         }
 
@@ -87,6 +83,13 @@ public RepositoryLayout newRepositoryLayout(RepositorySystemSession session, Rem
             factories.list(buffer);
         }
 
-        throw new NoRepositoryLayoutException(repository, buffer.toString(), errors.size() == 1 ? errors.get(0) : null);
+        // create exception: if one error, make it cause
+        NoRepositoryLayoutException ex = new NoRepositoryLayoutException(
+                repository, buffer.toString(), errors.size() == 1 ? errors.get(0) : null);
+        // if more errors, make them all suppressed
+        if (errors.size() > 1) {
+            errors.forEach(ex::addSuppressed);
+        }
+        throw ex;
     }
 }

maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultTransporterProvider.java

@@ -28,6 +28,8 @@
 import java.util.Map;
 
 import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.repository.Authentication;
+import org.eclipse.aether.repository.Proxy;
 import org.eclipse.aether.repository.RemoteRepository;
 import org.eclipse.aether.spi.connector.transport.Transporter;
 import org.eclipse.aether.spi.connector.transport.TransporterFactory;
@@ -74,18 +76,33 @@ public Transporter newTransporter(RepositorySystemSession session, RemoteReposit
                     Utils.appendClassLoader(buffer, transporter);
                     buffer.append(" with priority ").append(factory.getPriority());
                     buffer.append(" for ").append(repository.getUrl());
+
+                    Authentication auth = repository.getAuthentication();
+                    if (auth != null) {
+                        buffer.append(" with ").append(auth);
+                    }
+
+                    Proxy proxy = repository.getProxy();
+                    if (proxy != null) {
+                        buffer.append(" via ")
+                                .append(proxy.getHost())
+                                .append(':')
+                                .append(proxy.getPort());
+
+                        auth = proxy.getAuthentication();
+                        if (auth != null) {
+                            buffer.append(" with ").append(auth);
+                        }
+                    }
+
                     LOGGER.debug(buffer.toString());
                 }
 
                 return transporter;
             } catch (NoTransporterException e) {
                 // continue and try next factory
-                errors.add(e);
-            }
-        }
-        if (LOGGER.isDebugEnabled() && errors.size() > 1) {
-            for (Exception e : errors) {
                 LOGGER.debug("Could not obtain transporter factory for {}", repository, e);
+                errors.add(e);
             }
         }
 
@@ -98,6 +115,13 @@ public Transporter newTransporter(RepositorySystemSession session, RemoteReposit
             factories.list(buffer);
         }
 
-        throw new NoTransporterException(repository, buffer.toString(), errors.size() == 1 ? errors.get(0) : null);
+        // create exception: if one error, make it cause
+        NoTransporterException ex =
+                new NoTransporterException(repository, buffer.toString(), errors.size() == 1 ? errors.get(0) : null);
+        // if more errors, make them all suppressed
+        if (errors.size() > 1) {
+            errors.forEach(ex::addSuppressed);
+        }
+        throw ex;
     }
 }

maven-resolver-spi/src/main/java/org/eclipse/aether/spi/connector/transport/TransporterFactory.java

@@ -40,6 +40,7 @@ public interface TransporterFactory {
      * @param repository The remote repository to create a transporter for, must not be {@code null}.
      * @return The transporter for the given repository, never {@code null}.
      * @throws NoTransporterException If the factory cannot create a transporter for the specified remote repository.
+     * @throws RuntimeException If the factory could create a transporter for the specified remote repository but fails so (ie invalid configuration).
      */
     Transporter newInstance(RepositorySystemSession session, RemoteRepository repository) throws NoTransporterException;
 

maven-resolver-transport-apache/src/main/java/org/eclipse/aether/transport/apache/ApacheTransporter.java

@@ -157,9 +157,6 @@ final class ApacheTransporter extends AbstractTransporter implements HttpTranspo
             ChecksumExtractor checksumExtractor,
             PathProcessor pathProcessor)
             throws NoTransporterException {
-        if (!"http".equalsIgnoreCase(repository.getProtocol()) && !"https".equalsIgnoreCase(repository.getProtocol())) {
-            throw new NoTransporterException(repository);
-        }
         this.checksumExtractor = checksumExtractor;
         this.pathProcessor = pathProcessor;
         try {

maven-resolver-transport-apache/src/main/java/org/eclipse/aether/transport/apache/ApacheTransporterFactory.java

@@ -73,6 +73,10 @@ public HttpTransporter newInstance(RepositorySystemSession session, RemoteReposi
         requireNonNull(session, "session cannot be null");
         requireNonNull(repository, "repository cannot be null");
 
+        if (!"http".equalsIgnoreCase(repository.getProtocol()) && !"https".equalsIgnoreCase(repository.getProtocol())) {
+            throw new NoTransporterException(repository);
+        }
+
         return new ApacheTransporter(repository, session, checksumExtractor, pathProcessor);
     }
 }

maven-resolver-transport-classpath/src/main/java/org/eclipse/aether/transport/classpath/ClasspathTransporter.java

@@ -43,10 +43,6 @@ final class ClasspathTransporter extends AbstractTransporter {
     private final ClassLoader classLoader;
 
     ClasspathTransporter(RepositorySystemSession session, RemoteRepository repository) throws NoTransporterException {
-        if (!"classpath".equalsIgnoreCase(repository.getProtocol())) {
-            throw new NoTransporterException(repository);
-        }
-
         String base;
         try {
             URI uri = new URI(repository.getUrl());

maven-resolver-transport-classpath/src/main/java/org/eclipse/aether/transport/classpath/ClasspathTransporterFactory.java

@@ -75,6 +75,10 @@ public Transporter newInstance(RepositorySystemSession session, RemoteRepository
         requireNonNull(session, "session cannot be null");
         requireNonNull(repository, "repository cannot be null");
 
+        if (!"classpath".equalsIgnoreCase(repository.getProtocol())) {
+            throw new NoTransporterException(repository);
+        }
+
         return new ClasspathTransporter(session, repository);
     }
 }

maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporterFactory.java

@@ -50,6 +50,7 @@ public FileTransporterFactory() {
         // enables default constructor
     }
 
+    @Override
     public float getPriority() {
         return priority;
     }
@@ -65,11 +66,14 @@ public FileTransporterFactory setPriority(float priority) {
         return this;
     }
 
+    @Override
     public Transporter newInstance(RepositorySystemSession session, RemoteRepository repository)
             throws NoTransporterException {
         requireNonNull(session, "session cannot be null");
         requireNonNull(repository, "repository cannot be null");
 
+        // special case in file transport: to support custom FS providers (like JIMFS), we cannot
+        // cover "all possible protocols" to throw NoTransporterEx, but we rely on FS rejecting the URI
         FileTransporter.FileOp fileOp = FileTransporter.FileOp.COPY;
         String repositoryUrl = repository.getUrl();
         if (repositoryUrl.startsWith("symlink+")) {

maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporter.java

@@ -228,11 +228,7 @@ final class JdkTransporter extends AbstractTransporter implements HttpTransporte
                 CONFIG_PROP_MAX_CONCURRENT_REQUESTS));
 
         this.headers = headers;
-        try {
-            this.client = createClient(session, repository, insecure);
-        } catch (Exception e) {
-            throw new NoTransporterException(repository, e);
-        }
+        this.client = createClient(session, repository, insecure);
     }
 
     private URI resolve(TransportTask task) {
@@ -431,7 +427,7 @@ protected void implClose() {
     }
 
     private HttpClient createClient(RepositorySystemSession session, RemoteRepository repository, boolean insecure)
-            throws Exception {
+            throws RuntimeException {
 
         HashMap<Authenticator.RequestorType, PasswordAuthentication> authentications = new HashMap<>();
         SSLContext sslContext = null;
@@ -449,35 +445,43 @@ private HttpClient createClient(RepositorySystemSession session, RemoteRepositor
         }
 
         if (sslContext == null) {
-            if (insecure) {
-                sslContext = SSLContext.getInstance("TLS");
-                X509ExtendedTrustManager tm = new X509ExtendedTrustManager() {
-                    @Override
-                    public void checkClientTrusted(X509Certificate[] chain, String authType) {}
+            try {
+                if (insecure) {
+                    sslContext = SSLContext.getInstance("TLS");
+                    X509ExtendedTrustManager tm = new X509ExtendedTrustManager() {
+                        @Override
+                        public void checkClientTrusted(X509Certificate[] chain, String authType) {}
 
-                    @Override
-                    public void checkServerTrusted(X509Certificate[] chain, String authType) {}
+                        @Override
+                        public void checkServerTrusted(X509Certificate[] chain, String authType) {}
 
-                    @Override
-                    public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) {}
+                        @Override
+                        public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) {}
 
-                    @Override
-                    public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) {}
+                        @Override
+                        public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) {}
 
-                    @Override
-                    public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
+                        @Override
+                        public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
 
-                    @Override
-                    public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
+                        @Override
+                        public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) {}
 
-                    @Override
-                    public X509Certificate[] getAcceptedIssuers() {
-                        return null;
-                    }
-                };
-                sslContext.init(null, new X509TrustManager[] {tm}, null);
-            } else {
-                sslContext = SSLContext.getDefault();
+                        @Override
+                        public X509Certificate[] getAcceptedIssuers() {
+                            return null;
+                        }
+                    };
+                    sslContext.init(null, new X509TrustManager[] {tm}, null);
+                } else {
+                    sslContext = SSLContext.getDefault();
+                }
+            } catch (Exception e) {
+                if (e instanceof RuntimeException) {
+                    throw (RuntimeException) e;
+                } else {
+                    throw new IllegalStateException("SSL Context setup failure", e);
+                }
             }
         }
 

maven-resolver-transport-jdk-parent/maven-resolver-transport-jdk-11/src/main/java/org/eclipse/aether/transport/jdk/JdkTransporterFactory.java

@@ -69,7 +69,7 @@ public HttpTransporter newInstance(RepositorySystemSession session, RemoteReposi
         requireNonNull(repository, "repository cannot be null");
 
         if (!"http".equalsIgnoreCase(repository.getProtocol()) && !"https".equalsIgnoreCase(repository.getProtocol())) {
-            throw new NoTransporterException(repository, "Only HTTP/HTTPS is supported");
+            throw new NoTransporterException(repository);
         }
 
         return new JdkTransporter(session, repository, javaVersion(), checksumExtractor, pathProcessor);