From 549cef77a9f26784f8d3dfd593356659a0cb65d6 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 22:17:06 -0400 Subject: [PATCH 1/9] Update github actions to use newer versions --- .github/workflows/test.yml | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b186ca6..37a9521 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,29 +20,29 @@ jobs: tests: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up JDK 17 - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: '17' distribution: 'temurin' - - name: Build with Gradle - uses: gradle/gradle-build-action@bd5760595778326ba7f1441bcf7e88b49de61a25 # v2.6.0 - with: - arguments: jacocoTestReport + - name: Setup Gradle + uses: gradle/actions/setup-gradle@v3 + - name: Build and test + uses: ./gradlew test javadoc: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v4 - name: Set up JDK 17 - uses: actions/setup-java@v3 + uses: actions/setup-java@v4 with: java-version: '17' distribution: 'temurin' - - name: Build with Gradle - uses: gradle/gradle-build-action@bd5760595778326ba7f1441bcf7e88b49de61a25 # v2.6.0 - with: - arguments: javadoc + - name: Setup Gradle + uses: gradle/actions/setup-gradle@v3 + - name: Build and test + uses: ./gradlew javadoc From 7589c280d7ae22de51950b2a071243ce1b8f9932 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 22:23:56 -0400 Subject: [PATCH 2/9] hmm --- .github/workflows/test.yml | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 37a9521..9188f8e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -20,7 +20,8 @@ jobs: tests: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 + - name: Checkout sources + uses: actions/checkout@v4 - name: Set up JDK 17 uses: actions/setup-java@v4 with: @@ -34,15 +35,14 @@ jobs: javadoc: runs-on: ubuntu-latest steps: - - uses: actions/checkout@v4 - - name: Set up JDK 17 - uses: actions/setup-java@v4 - with: - java-version: '17' - distribution: 'temurin' - - name: Setup Gradle - uses: gradle/actions/setup-gradle@v3 - - name: Build and test - uses: ./gradlew javadoc - - + - name: Checkout sources + uses: actions/checkout@v4 + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + java-version: '17' + distribution: 'temurin' + - name: Setup Gradle + uses: gradle/actions/setup-gradle@v3 + - name: Build and test + uses: ./gradlew javadoc \ No newline at end of file From 4854f34261bff15bfe17c2530aa40d39ec66e5cb Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 22:27:44 -0400 Subject: [PATCH 3/9] hmm --- .github/workflows/test.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9188f8e..204d6ff 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,12 +16,13 @@ on: permissions: contents: read + + jobs: tests: runs-on: ubuntu-latest steps: - - name: Checkout sources - uses: actions/checkout@v4 + - uses: actions/checkout@v4 - name: Set up JDK 17 uses: actions/setup-java@v4 with: From c49a44bd925fca01cc91e1e15b4c6b6d885cfb78 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 22:29:30 -0400 Subject: [PATCH 4/9] hmm --- .github/workflows/test.yml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 204d6ff..ffa3390 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,8 +16,6 @@ on: permissions: contents: read - - jobs: tests: runs-on: ubuntu-latest @@ -31,7 +29,7 @@ jobs: - name: Setup Gradle uses: gradle/actions/setup-gradle@v3 - name: Build and test - uses: ./gradlew test + run: ./gradlew test javadoc: runs-on: ubuntu-latest @@ -46,4 +44,4 @@ jobs: - name: Setup Gradle uses: gradle/actions/setup-gradle@v3 - name: Build and test - uses: ./gradlew javadoc \ No newline at end of file + run: ./gradlew javadoc \ No newline at end of file From 8d09f67333b089c61b496038899667e44bf0ed16 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 22:40:12 -0400 Subject: [PATCH 5/9] update localhost --- .../org/broadinstitute/http/nio/MockedIntegrationTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java b/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java index 73c139f..4b3ebfc 100644 --- a/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java +++ b/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java @@ -33,6 +33,7 @@ public class MockedIntegrationTest extends BaseTest { public static final UrlPattern FILE_URL = urlEqualTo("/file.txt"); public static final String BODY = "Hello"; + public static final String LOCAL_HOST = "127.0.0.1"; // the string localhost seems to be problematic on github actions but the IP may work better WireMockServer wireMockServer; WireMock wireMock; @@ -42,8 +43,8 @@ void start(){ .wireMockConfig() .dynamicPort()); wireMockServer.start(); - configureFor("localhost", wireMockServer.port()); - wireMock = new WireMock("localhost", wireMockServer.port()); + configureFor(LOCAL_HOST, wireMockServer.port()); + wireMock = new WireMock(LOCAL_HOST, wireMockServer.port()); } @AfterMethod From 031b7d3657c2a23f84b928becd825a947ab52240 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 22:57:17 -0400 Subject: [PATCH 6/9] add ClosedChannelException to default list --- src/main/java/org/broadinstitute/http/nio/RetryHandler.java | 4 +++- .../org/broadinstitute/http/nio/MockedIntegrationTest.java | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/broadinstitute/http/nio/RetryHandler.java b/src/main/java/org/broadinstitute/http/nio/RetryHandler.java index e379ce3..c119d88 100644 --- a/src/main/java/org/broadinstitute/http/nio/RetryHandler.java +++ b/src/main/java/org/broadinstitute/http/nio/RetryHandler.java @@ -12,6 +12,7 @@ import java.net.SocketException; import java.net.SocketTimeoutException; import java.net.URI; +import java.nio.channels.ClosedChannelException; import java.time.Duration; import java.time.Instant; import java.util.Collection; @@ -42,7 +43,8 @@ public class RetryHandler { EOFException.class, SocketException.class, SocketTimeoutException.class, - InterruptedIOException.class + InterruptedIOException.class, + ClosedChannelException.class ); /** diff --git a/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java b/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java index 4b3ebfc..100e32b 100644 --- a/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java +++ b/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java @@ -77,7 +77,6 @@ public Object[][] getFaults(){ } @Test(dataProvider = "getFaults", expectedExceptions = OutOfRetriesException.class) public void testConnectionReset(Fault fault) throws IOException { - final String body = "Hello"; final UrlPattern fileUrl = urlEqualTo("/file.txt"); wireMockServer.stubFor(get(fileUrl) .willReturn(aResponse().withFault(fault))); From 60c111b5917adf74fcb34394f5afef3f32cfe802 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 23:04:54 -0400 Subject: [PATCH 7/9] add reset by peer to retries --- src/main/java/org/broadinstitute/http/nio/RetryHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/broadinstitute/http/nio/RetryHandler.java b/src/main/java/org/broadinstitute/http/nio/RetryHandler.java index c119d88..8b35094 100644 --- a/src/main/java/org/broadinstitute/http/nio/RetryHandler.java +++ b/src/main/java/org/broadinstitute/http/nio/RetryHandler.java @@ -32,7 +32,7 @@ public class RetryHandler { /** * the default set of exception messages which are retried when encountered */ - public static final Set DEFALT_RETRYABLE_MESSAGES = Set.of("protocol error:"); + public static final Set DEFALT_RETRYABLE_MESSAGES = Set.of("protocol error:", "Connection reset by peer"); //IOExceptions with the string `protocol error` can happen when there is bad data returned during an http request /** From 236e62c4fd0d5777d61678c0da6c090e557c8ed5 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 23:06:02 -0400 Subject: [PATCH 8/9] make channelclosed retriable --- .../java/org/broadinstitute/http/nio/RetryHandlerUnitTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/broadinstitute/http/nio/RetryHandlerUnitTest.java b/src/test/java/org/broadinstitute/http/nio/RetryHandlerUnitTest.java index 9f7dea5..e5aa3c0 100644 --- a/src/test/java/org/broadinstitute/http/nio/RetryHandlerUnitTest.java +++ b/src/test/java/org/broadinstitute/http/nio/RetryHandlerUnitTest.java @@ -37,7 +37,7 @@ public static Object[][] getExceptionalConditions() { {new SocketException(), true}, {new SocketTimeoutException(), true}, {new IOException(new SocketException()), true}, - {new IOException(new ClosedChannelException()), false}, + {new IOException(new ClosedChannelException()), true}, {new IOException(new IOException()), false}, }; } From 6a3b258bdaec4fc8cfb26fe053bd179ec904cc49 Mon Sep 17 00:00:00 2001 From: Louis Bergelson Date: Fri, 21 Jun 2024 23:17:12 -0400 Subject: [PATCH 9/9] test 2 retries --- src/main/java/org/broadinstitute/http/nio/RetryHandler.java | 6 ++++-- .../org/broadinstitute/http/nio/MockedIntegrationTest.java | 5 +++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/broadinstitute/http/nio/RetryHandler.java b/src/main/java/org/broadinstitute/http/nio/RetryHandler.java index 8b35094..404c099 100644 --- a/src/main/java/org/broadinstitute/http/nio/RetryHandler.java +++ b/src/main/java/org/broadinstitute/http/nio/RetryHandler.java @@ -32,8 +32,10 @@ public class RetryHandler { /** * the default set of exception messages which are retried when encountered */ - public static final Set DEFALT_RETRYABLE_MESSAGES = Set.of("protocol error:", "Connection reset by peer"); - //IOExceptions with the string `protocol error` can happen when there is bad data returned during an http request + public static final Set DEFALT_RETRYABLE_MESSAGES = Set.of( + "protocol error:", //IOExceptions with the string `protocol error` can happen when there is bad data returned during an http request + "Connection reset by peer"); + /** * default set of exception types which will be retried when encountered diff --git a/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java b/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java index 100e32b..f6eb47a 100644 --- a/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java +++ b/src/test/java/org/broadinstitute/http/nio/MockedIntegrationTest.java @@ -89,12 +89,13 @@ public void testConnectionReset(Fault fault) throws IOException { @Test public void testRetryFixesError() throws IOException { final String body = "Hello"; + final long bodyLength = body.getBytes(StandardCharsets.UTF_8).length; + wireMockServer.stubFor(get(FILE_URL).inScenario("fail once") .whenScenarioStateIs(Scenario.STARTED) .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER)) .willSetStateTo("errored")); - final long bodyLength = body.getBytes(StandardCharsets.UTF_8).length; wireMockServer.stubFor(get(FILE_URL).inScenario("fail once") .whenScenarioStateIs("errored") .willReturn(ok(body).withHeader("content-length", String.valueOf(bodyLength))) @@ -114,7 +115,7 @@ public void testRetryFixesError() throws IOException { final URI uri = getUri("/file.txt"); final HttpFileSystemProviderSettings settings = new HttpFileSystemProviderSettings(Duration.ofSeconds(2), HttpClient.Redirect.NORMAL, - new HttpFileSystemProviderSettings.RetrySettings(1, RetryHandler.DEFAULT_RETRYABLE_HTTP_CODES, + new HttpFileSystemProviderSettings.RetrySettings(2, RetryHandler.DEFAULT_RETRYABLE_HTTP_CODES, RetryHandler.DEFAULT_RETRYABLE_EXCEPTIONS, RetryHandler.DEFALT_RETRYABLE_MESSAGES, e -> false));