From 1b4ec47da999cff2ac5f9e99a095d314541ed12e Mon Sep 17 00:00:00 2001 From: Nick McKinney Date: Fri, 8 Mar 2024 15:33:38 -0700 Subject: [PATCH] ensuring connection timeouts get wrapped by MavenDownloadingException (#4081) * ensuring connection timeouts get wrapped by MavenDownloadingException * polish * removed a condition which I believe no longer made sense, and was causing tests to fail with latest changes * Apply suggestions from code review * adjusting so that IOExceptions can get caught and wrapped into MavenDownloadingException * clarifying checked exception types for `sendRequest` and related methods --------- Co-authored-by: Tim te Beek --- .../maven/internal/MavenPomDownloader.java | 25 ++++++++++--------- .../internal/MavenPomDownloaderTest.java | 20 ++++++++++++--- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java index d02a639979b..e40b3f4a7ce 100644 --- a/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java +++ b/rewrite-maven/src/main/java/org/openrewrite/maven/internal/MavenPomDownloader.java @@ -135,7 +135,7 @@ public MavenPomDownloader(Map projectPoms, HttpSender httpSender, Exe this.mavenCache = this.ctx.getPomCache(); } - byte[] sendRequest(HttpSender.Request request) throws Throwable { + byte[] sendRequest(HttpSender.Request request) throws IOException, HttpSenderResponseException { long start = System.nanoTime(); try { return Failsafe.with(retryPolicy).get(() -> { @@ -148,7 +148,12 @@ byte[] sendRequest(HttpSender.Request request) throws Throwable { } }); } catch (FailsafeException failsafeException) { - throw failsafeException.getCause() == null ? failsafeException : failsafeException.getCause(); + if (failsafeException.getCause() instanceof HttpSenderResponseException) { + throw (HttpSenderResponseException) failsafeException.getCause(); + } + throw failsafeException; + } catch (UncheckedIOException e) { + throw e.getCause(); } finally { this.ctx.recordResolutionTime(Duration.ofNanos(System.nanoTime() - start)); } @@ -282,7 +287,7 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv .increment(); result = Optional.of(derivedMeta); } - } catch (HttpSenderResponseException | MavenDownloadingException e) { + } catch (HttpSenderResponseException | MavenDownloadingException | IOException e) { repositoryResponses.put(repo, e.getMessage()); } } @@ -328,7 +333,7 @@ public MavenMetadata downloadMetadata(GroupArtifactVersion gav, @Nullable Resolv * @return Metadata or null if the metadata cannot be derived. */ @Nullable - private MavenMetadata deriveMetadata(GroupArtifactVersion gav, MavenRepository repo) throws HttpSenderResponseException, MavenDownloadingException { + private MavenMetadata deriveMetadata(GroupArtifactVersion gav, MavenRepository repo) throws HttpSenderResponseException, IOException, MavenDownloadingException { if ((repo.getDeriveMetadataIfMissing() != null && !repo.getDeriveMetadataIfMissing()) || gav.getVersion() != null) { // Do not derive metadata if we cannot navigate/browse the artifacts. // Do not derive metadata if a specific version has been defined. @@ -590,6 +595,8 @@ public Pom download(GroupArtifactVersion gav, //If the exception is a common, client-side exception, cache an empty result. mavenCache.putPom(resolvedGav, null); } + } catch (IOException e) { + repositoryResponses.put(repo, e.getMessage()); } } } else if (result.isPresent()) { @@ -769,8 +776,6 @@ public MavenRepository normalizeRepository(MavenRepository originalRepository, M repository.getSnapshots(), repository.getUsername(), repository.getPassword()); - } else if (!e.isClientSideException()) { - return originalRepository; } } catch (Throwable e) { // ok to fall through here and cache a null @@ -797,7 +802,7 @@ public MavenRepository normalizeRepository(MavenRepository originalRepository, M /** * Replicates Apache Maven's behavior to attempt anonymous download if repository credentials prove invalid */ - private byte[] requestAsAuthenticatedOrAnonymous(MavenRepository repo, String uriString) throws HttpSenderResponseException { + private byte[] requestAsAuthenticatedOrAnonymous(MavenRepository repo, String uriString) throws HttpSenderResponseException, IOException { try { return sendRequest(applyAuthenticationToRequest(repo, httpSender.get(uriString)).build()); } catch (HttpSenderResponseException e) { @@ -806,12 +811,10 @@ private byte[] requestAsAuthenticatedOrAnonymous(MavenRepository repo, String ur } else { throw e; } - } catch (Throwable t) { - throw new RuntimeException(t); // unreachable } } - private byte[] retryRequestAnonymously(String uriString, HttpSenderResponseException originalException) throws HttpSenderResponseException { + private byte[] retryRequestAnonymously(String uriString, HttpSenderResponseException originalException) throws HttpSenderResponseException, IOException { try { return sendRequest(httpSender.get(uriString).build()); } catch (HttpSenderResponseException retryException) { @@ -820,8 +823,6 @@ private byte[] retryRequestAnonymously(String uriString, HttpSenderResponseExcep } else { throw retryException; } - } catch (Throwable t) { - throw new RuntimeException(t); // unreachable } } diff --git a/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java b/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java index c2812f5b81a..dbb1f0abbba 100755 --- a/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java +++ b/rewrite-maven/src/test/java/org/openrewrite/maven/internal/MavenPomDownloaderTest.java @@ -36,6 +36,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; +import java.net.UnknownHostException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -48,7 +49,6 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; -import static java.util.Objects.requireNonNull; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; @@ -86,7 +86,7 @@ void ossSonatype() { null, "true", false, null, null, null); MavenRepository repo = new MavenPomDownloader(ctx).normalizeRepository(ossSonatype, MavenExecutionContextView.view(ctx), null); - assertThat(requireNonNull(repo).getUri()).isEqualTo(ossSonatype.getUri()); + assertThat(repo).isNotNull().extracting((MavenRepository::getUri)).isEqualTo(ossSonatype.getUri()); } @Issue("https://github.com/openrewrite/rewrite/issues/3908") @@ -188,7 +188,7 @@ public void repositoryAccessFailed(String uri, Throwable e) { // not expected to succeed } assertThat(attemptedUris).isNotEmpty(); - assertThat(attemptedUris.get(httpUrl)).hasMessageContaining("java.net.UnknownHostException"); + assertThat(attemptedUris.get(httpUrl)).isInstanceOf(UnknownHostException.class); assertThat(discoveredRepositories).isEmpty(); } @@ -742,6 +742,20 @@ void doNotRenameRepoForCustomMavenLocal(@TempDir Path tempDir) throws MavenDownl assertThat(result.getRepository().getUri()).startsWith(tempDir.toUri().toString()); } + @Issue("https://github.com/openrewrite/rewrite/issues/4080") + @Test + void connectTimeout() { + var downloader = new MavenPomDownloader(ctx); + var gav = new GroupArtifactVersion("org.openrewrite", "rewrite-core", "7.0.0"); + var repos = singletonList(MavenRepository.builder() + .id("non-routable").uri("http://10.0.0.0/maven").knownToExist(true).build()); + + assertThatThrownBy(() -> downloader.download(gav, null, null, repos)) + .isInstanceOf(MavenDownloadingException.class) + .hasMessageContaining("rewrite-core") + .hasMessageContaining("10.0.0.0"); + } + private static GroupArtifactVersion createArtifact(Path repository) throws IOException { Path target = repository.resolve(Paths.get("org", "openrewrite", "rewrite", "1.0.0")); Path pom = target.resolve("rewrite-1.0.0.pom");