davsclaus commented on code in PR #29781:
URL: https://github.com/apache/camel/pull/29781#discussion_r4560690789
##########
tooling/camel-tooling-maven/src/main/java/org/apache/camel/tooling/maven/MavenDownloaderImpl.java:
##########
@@ -840,9 +857,91 @@ private List<RemoteRepository> resolutionRepositories(
session, extraRepos);
repositories.addAll(resolvedExtraRepos);
}
+ if (!unreachableRepositories.isEmpty()) {
+ // asking them again would only wait for the connect timeout again
+ repositories.removeIf(r ->
unreachableRepositories.contains(repositoryUrl(r.getUrl())));
+ }
return repositories;
}
+ /**
+ * Sets the request timeout of Camel (unless configured, such as with the
aether.connector.requestTimeout system
+ * property), and stops using a repository that cannot be connected to.
+ */
+ private RepositorySystemSession
withTimeoutsAndReachability(RepositorySystemSession session) {
+ DefaultRepositorySystemSession answer = new
DefaultRepositorySystemSession(session);
+ if
(session.getConfigProperties().get(ConfigurationProperties.REQUEST_TIMEOUT) ==
null) {
+ answer.setConfigProperty(ConfigurationProperties.REQUEST_TIMEOUT,
requestTimeout);
+ }
+
answer.setTransferListener(ChainedTransferListener.newInstance(session.getTransferListener(),
+ new UnreachableRepositoryListener(unreachableRepositories)));
+ return answer;
+ }
+
+ /**
+ * Notes the repositories that cannot be connected to: a host that does
not resolve, refuses the connection, or does
+ * not answer within the connect timeout. A repository that is slow to
send an artifact is still used.
+ */
+ static final class UnreachableRepositoryListener extends
AbstractTransferListener {
+
+ private final Set<String> unreachable;
+
+ UnreachableRepositoryListener(Set<String> unreachable) {
+ this.unreachable = unreachable;
+ }
+
+ @Override
+ public void transferFailed(TransferEvent event) {
+ Throwable cause = connectFailure(event.getException());
+ if (cause != null && event.getResource() != null &&
event.getResource().getRepositoryUrl() != null) {
+ String url =
repositoryUrl(event.getResource().getRepositoryUrl());
+ if (unreachable.add(url)) {
+ LOG.warn("Cannot connect to Maven repository: {} due to:
{}. The repository is not used anymore.",
+ url, cause.getMessage() != null ?
cause.getMessage() : cause.getClass().getName());
+ }
+ }
+ }
+
+ static Throwable connectFailure(Throwable e) {
+ // guard against a cause that points back to an exception in the
chain
+ int depth = 0;
+ for (Throwable t = e; t != null && depth < 20; t = t.getCause(),
depth++) {
+ if (t instanceof ConnectException || t instanceof
UnknownHostException) {
+ return t;
+ }
+ // the connect timeout of the JDK, and of Apache HttpClient
(ConnectTimeoutException)
+ if (t instanceof SocketTimeoutException && "Connect timed
out".equalsIgnoreCase(t.getMessage())
+ ||
"org.apache.http.conn.ConnectTimeoutException".equals(t.getClass().getName())) {
Review Comment:
Done differently in 212303df2aa5: the JDK socket case and the HTTP client
case are now two separate `if`s, so there is no precedence to read.
_Claude Code on behalf of Claus Ibsen (davsclaus)_
##########
tooling/camel-tooling-maven/src/test/java/org/apache/camel/tooling/maven/MavenDownloaderImplTest.java:
##########
@@ -1130,4 +1138,94 @@ private static void writePom(File repo, String
artifactId, String body) throws E
""".formatted(artifactId, body);
Files.writeString(new File(dir, artifactId + "-1.0.0.pom").toPath(),
pom);
}
+
+ @Test
+ void testUnreachableRepositoryIsNotUsedAnymore() throws Exception {
+ File repo = createDescriptorFixtureRepository();
+ File localRepo = new File(tempDir, "unreachable-local-repo");
+ // nothing listens on this port, so the connection is refused
+ String dead = "http://localhost:"; +
AvailablePortFinder.getNextAvailable(20000, 30000) + "/maven";
+
+ try (MavenDownloaderImpl downloader = new MavenDownloaderImpl()) {
+ downloader.setMavenCentralEnabled(false);
+ downloader.setMavenApacheSnapshotEnabled(false);
+ downloader.setMavenSettingsLocation("false");
+ downloader.setRepos(dead + "," + repo.toURI());
+ downloader.build();
+ MavenDownloader customDownloader =
downloader.customize(localRepo.getAbsolutePath(), 5000, 5000);
+
+ // an artifact no repository has: both are asked
+ assertThrows(MavenResolutionException.class,
+ () ->
customDownloader.resolveArtifacts(List.of("org.example.fixture:missing:1.0.0"),
null, false,
+ false));
+ assertEquals(Set.of(dead),
downloader.getUnreachableRepositories());
+
+ // the other repository is still used
+ List<MavenArtifact> artifacts = customDownloader.resolveArtifacts(
+ List.of("org.example.fixture:fixture-bom:pom:1.0.0"),
null, false, false);
+ assertEquals(1, artifacts.size());
+
+ // the repository is not asked again, and the failure says so
+ MavenResolutionException e =
assertThrows(MavenResolutionException.class,
+ () ->
customDownloader.resolveArtifacts(List.of("org.example.fixture:missing2:1.0.0"),
null, false,
+ false));
+ assertTrue(e.getRepositories().contains(dead + " (not
reachable)"), e.getRepositories().toString());
+ assertFalse(e.getRepositories().contains(dead),
e.getRepositories().toString());
+ }
+ }
+
+ @Test
+ void testRepositoryThatDoesNotAnswerFailsOnTheRequestTimeout() throws
Exception {
+ File localRepo = new File(tempDir, "stalled-local-repo");
+ // accepts the connection, and never answers
+ try (ServerSocket server = new ServerSocket(0);
+ MavenDownloaderImpl downloader = new MavenDownloaderImpl()) {
+ List<Socket> accepted = new CopyOnWriteArrayList<>();
+ Thread acceptor = new Thread(() -> {
+ try {
+ while (!server.isClosed()) {
+ accepted.add(server.accept());
+ }
+ } catch (Exception e) {
+ // closed
+ }
+ });
+ acceptor.setDaemon(true);
+ acceptor.start();
+ String stalled = "http://localhost:"; + server.getLocalPort() +
"/maven";
+
+ downloader.setMavenCentralEnabled(false);
+ downloader.setMavenApacheSnapshotEnabled(false);
+ downloader.setMavenSettingsLocation("false");
+ downloader.setRepos(stalled);
+ downloader.build();
+ assertEquals(60000, downloader.getRequestTimeout());
+ MavenDownloader customDownloader =
downloader.customize(localRepo.getAbsolutePath(), 5000, 1000);
+
+ long start = System.nanoTime();
+ assertThrows(MavenResolutionException.class,
+ () ->
customDownloader.resolveArtifacts(List.of("org.example.fixture:stalled:1.0.0"),
null, false,
+ false));
+ assertTrue(TimeUnit.NANOSECONDS.toSeconds(System.nanoTime() -
start) < 30);
+ // a slow repository is still used
+ assertTrue(downloader.getUnreachableRepositories().isEmpty());
+ for (Socket socket : accepted) {
+ socket.close();
+ }
+ }
+ }
+
+ @Test
+ void testConnectFailures() {
+
assertNotNull(MavenDownloaderImpl.UnreachableRepositoryListener.connectFailure(
+ new RuntimeException(new ConnectException("Connection
refused"))));
+
assertNotNull(MavenDownloaderImpl.UnreachableRepositoryListener.connectFailure(new
UnknownHostException("nope")));
+
assertNotNull(MavenDownloaderImpl.UnreachableRepositoryListener.connectFailure(
+ new SocketTimeoutException("Connect timed out")));
+ // a read timeout is a slow repository, not an unreachable one
+
assertNull(MavenDownloaderImpl.UnreachableRepositoryListener.connectFailure(
+ new SocketTimeoutException("Read timed out")));
+
assertNull(MavenDownloaderImpl.UnreachableRepositoryListener.connectFailure(new
IOException("Not found")));
+
assertNull(MavenDownloaderImpl.UnreachableRepositoryListener.connectFailure(null));
+ }
Review Comment:
Added in 212303df2aa5: a wrapped HttpClient 4 `ConnectTimeoutException`, and
the JDK `HttpConnectTimeoutException`.
_Claude Code on behalf of Claus Ibsen (davsclaus)_
##########
tooling/camel-tooling-maven/src/main/java/org/apache/camel/tooling/maven/MavenDownloaderImpl.java:
##########
@@ -840,9 +857,91 @@ private List<RemoteRepository> resolutionRepositories(
session, extraRepos);
repositories.addAll(resolvedExtraRepos);
}
+ if (!unreachableRepositories.isEmpty()) {
+ // asking them again would only wait for the connect timeout again
+ repositories.removeIf(r ->
unreachableRepositories.contains(repositoryUrl(r.getUrl())));
+ }
return repositories;
}
+ /**
+ * Sets the request timeout of Camel (unless configured, such as with the
aether.connector.requestTimeout system
+ * property), and stops using a repository that cannot be connected to.
+ */
+ private RepositorySystemSession
withTimeoutsAndReachability(RepositorySystemSession session) {
+ DefaultRepositorySystemSession answer = new
DefaultRepositorySystemSession(session);
+ if
(session.getConfigProperties().get(ConfigurationProperties.REQUEST_TIMEOUT) ==
null) {
+ answer.setConfigProperty(ConfigurationProperties.REQUEST_TIMEOUT,
requestTimeout);
+ }
+
answer.setTransferListener(ChainedTransferListener.newInstance(session.getTransferListener(),
+ new UnreachableRepositoryListener(unreachableRepositories)));
+ return answer;
+ }
+
+ /**
+ * Notes the repositories that cannot be connected to: a host that does
not resolve, refuses the connection, or does
+ * not answer within the connect timeout. A repository that is slow to
send an artifact is still used.
+ */
+ static final class UnreachableRepositoryListener extends
AbstractTransferListener {
+
+ private final Set<String> unreachable;
+
+ UnreachableRepositoryListener(Set<String> unreachable) {
+ this.unreachable = unreachable;
+ }
+
+ @Override
+ public void transferFailed(TransferEvent event) {
+ Throwable cause = connectFailure(event.getException());
+ if (cause != null && event.getResource() != null &&
event.getResource().getRepositoryUrl() != null) {
+ String url =
repositoryUrl(event.getResource().getRepositoryUrl());
+ if (unreachable.add(url)) {
+ LOG.warn("Cannot connect to Maven repository: {} due to:
{}. The repository is not used anymore.",
+ url, cause.getMessage() != null ?
cause.getMessage() : cause.getClass().getName());
+ }
+ }
+ }
+
+ static Throwable connectFailure(Throwable e) {
+ // guard against a cause that points back to an exception in the
chain
+ int depth = 0;
+ for (Throwable t = e; t != null && depth < 20; t = t.getCause(),
depth++) {
+ if (t instanceof ConnectException || t instanceof
UnknownHostException) {
+ return t;
+ }
+ // the connect timeout of the JDK, and of Apache HttpClient
(ConnectTimeoutException)
+ if (t instanceof SocketTimeoutException && "Connect timed
out".equalsIgnoreCase(t.getMessage())
+ ||
"org.apache.http.conn.ConnectTimeoutException".equals(t.getClass().getName())) {
+ return t;
Review Comment:
Rather than only a comment, the check in 212303df2aa5 now matches a class
whose simple name ends in `ConnectTimeoutException`, so HttpClient 4,
HttpClient 5 (`org.apache.hc.client5.http.ConnectTimeoutException`) and the JDK
`HttpConnectTimeoutException` are all detected without depending on any of
them; the comment lists the three.
_Claude Code on behalf of Claus Ibsen (davsclaus)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
