openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.21k stars 330 forks source link

MavenPomDownloader - UncheckedIOException does not get wrapped into MavenDownloadingException #4080

Closed nmck257 closed 8 months ago

nmck257 commented 8 months ago

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

Unit tests

What is the smallest, simplest way to reproduce the problem?

In my walled-garden network environment, several tests fail from MavenPomDownloaderTest, such as skipsLocalInvalidArtifactsEmptyJar, with stack traces like this:

org.opentest4j.AssertionFailedError: Unexpected exception type thrown, 
Expected :class org.openrewrite.maven.MavenDownloadingException
Actual   :class java.lang.RuntimeException
<Click to see difference>

    at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
    at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:67)
    at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:35)
    at org.junit.jupiter.api.Assertions.assertThrows(Assertions.java:3115)
    at org.openrewrite.maven.internal.MavenPomDownloaderTest.skipsLocalInvalidArtifactsEmptyJar(MavenPomDownloaderTest.java:729)
    at java.base/java.lang.reflect.Method.invoke(Method.java:568)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: java.lang.RuntimeException: java.io.UncheckedIOException: java.net.SocketTimeoutException: Connect timed out
    at org.openrewrite.maven.internal.MavenPomDownloader.requestAsAuthenticatedOrAnonymous(MavenPomDownloader.java:810)
    at org.openrewrite.maven.internal.MavenPomDownloader.download(MavenPomDownloader.java:572)
    at org.openrewrite.maven.internal.MavenPomDownloaderTest.lambda$skipsLocalInvalidArtifactsEmptyJar$13(MavenPomDownloaderTest.java:731)
    at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:53)
    ... 6 more
Caused by: java.io.UncheckedIOException: java.net.SocketTimeoutException: Connect timed out
    at org.openrewrite.ipc.http.HttpUrlConnectionSender.send(HttpUrlConnectionSender.java:116)
    at org.openrewrite.maven.internal.MavenPomDownloader.lambda$sendRequest$1(MavenPomDownloader.java:142)
    at dev.failsafe.Functions.lambda$toCtxSupplier$11(Functions.java:243)
    at dev.failsafe.Functions.lambda$get$0(Functions.java:46)
    at dev.failsafe.internal.RetryPolicyExecutor.lambda$apply$0(RetryPolicyExecutor.java:74)
    at dev.failsafe.SyncExecutionImpl.executeSync(SyncExecutionImpl.java:187)
    at dev.failsafe.FailsafeExecutor.call(FailsafeExecutor.java:376)
    at dev.failsafe.FailsafeExecutor.get(FailsafeExecutor.java:112)
    at org.openrewrite.maven.internal.MavenPomDownloader.sendRequest(MavenPomDownloader.java:141)
    at org.openrewrite.maven.internal.MavenPomDownloader.requestAsAuthenticatedOrAnonymous(MavenPomDownloader.java:802)
    ... 9 more
Caused by: java.net.SocketTimeoutException: Connect timed out
    at java.base/sun.nio.ch.NioSocketImpl.timedFinishConnect(NioSocketImpl.java:551)
    at java.base/sun.nio.ch.NioSocketImpl.connect(NioSocketImpl.java:602)
    at java.base/java.net.SocksSocketImpl.connect(SocksSocketImpl.java:327)
    at java.base/java.net.Socket.connect(Socket.java:633)
    at java.base/sun.security.ssl.SSLSocketImpl.connect(SSLSocketImpl.java:304)
    at java.base/sun.net.NetworkClient.doConnect(NetworkClient.java:178)
    at java.base/sun.net.www.http.HttpClient.openServer(HttpClient.java:534)
    at java.base/sun.net.www.http.HttpClient.openServer(HttpClient.java:639)
    at java.base/sun.net.www.protocol.https.HttpsClient.<init>(HttpsClient.java:266)
    at java.base/sun.net.www.protocol.https.HttpsClient.New(HttpsClient.java:380)
    at java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.getNewHttpClient(AbstractDelegateHttpsURLConnection.java:193)
    at java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect0(HttpURLConnection.java:1242)
    at java.base/sun.net.www.protocol.http.HttpURLConnection.plainConnect(HttpURLConnection.java:1128)
    at java.base/sun.net.www.protocol.https.AbstractDelegateHttpsURLConnection.connect(AbstractDelegateHttpsURLConnection.java:179)
    at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1665)
    at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1589)
    at java.base/java.net.HttpURLConnection.getResponseCode(HttpURLConnection.java:529)
    at java.base/sun.net.www.protocol.https.HttpsURLConnectionImpl.getResponseCode(HttpsURLConnectionImpl.java:308)
    at org.openrewrite.ipc.http.HttpUrlConnectionSender.send(HttpUrlConnectionSender.java:97)
    ... 18 more

What did you expect to see?

Happy test, or, perhaps an easy-to-read failure from being unable to connect to the default remote Maven repo in my environment.

What did you see instead?

The resultant exception is not wrapped into MavenDownloadingException and therefore does not give useful detail about the exact request/artifact that led to an error.

So when this happens for real recipe runs, that lack of detail is really inconvenient.

What is the full stack trace of any errors you encountered?

Seen above

Are you interested in contributing a fix to OpenRewrite?

Yes, but, it's a little tricky because I know I'm in a non-standard network. I'd ideally like to define a unit test which simulates the "connect timeout" behavior, but, hard for me to know for sure that any given test will achieve that same behavior both in my network and in a "standard" network. I might have to spam Actions a little more than usual to validate that.

timtebeek commented 8 months ago

Would indeed be a nice improvement to catch & handle issues with downloading consistently, and "good" that you're in an environment that allows you that replicate that reliable already. Depending on the src/main change needed we maybe don't even have to replicate it with a test; I imagine it'll be adding an exception to a catch block somewhere.