fixup! STF-322: Add tests for transport-failure retry

Author: oschwald

src/test/java/com/maxmind/geoip2/WebServiceClientTest.java

@@ -635,9 +635,8 @@ public void testNoRetryOn4xx() {
     // as plain HTTP/1.1. The HTTP/1.1 path then triggers the JDK's own
     // idempotent-GET retry inside HttpClient.send(), producing two wire
     // requests where the test expects one. This is platform-specific JDK
-    // behavior we cannot disable from application code; the equivalent
-    // POST-based test in minfraud-api-java is unaffected because the JDK
-    // does not internally retry non-idempotent requests.
+    // behavior we cannot disable from application code; only idempotent
+    // methods (GET / HEAD) are affected.
     @Test
     @DisabledOnOs(OS.WINDOWS)
     public void testMaxRetriesZeroDisablesRetry() {
@@ -658,9 +657,12 @@ public void testMaxRetriesZeroDisablesRetry() {
         wireMock.verify(1, getRequestedFor(urlEqualTo(url)));
     }
 
-    // Disabled on Windows for the same reason as testMaxRetriesZeroDisablesRetry:
-    // the JDK's internal idempotent-GET retry can stack on top of our retry
-    // loop on the HTTP/1.1 fallback path, multiplying wire counts.
+    // Disabled on Windows: the JDK's internal idempotent-GET retry on the
+    // HTTP/1.1 fallback path (triggered by Windows-specific h2c upgrade
+    // failures against an immediate RST) stacks on top of our retry loop,
+    // multiplying wire counts (each of our 3 attempts becomes 2 wire
+    // requests, so the count assertion sees 6 instead of 3). This is JDK
+    // behavior we cannot disable from application code.
     @Test
     @DisabledOnOs(OS.WINDOWS)
     public void testRetriesExhausted() {
@@ -675,11 +677,51 @@ public void testRetriesExhausted() {
             .maxRetries(2)
             .build();
 
-        assertThrows(IOException.class,
+        IOException ex = assertThrows(IOException.class,
             () -> client.insights(InetAddress.getByName("1.2.3.4")));
 
         // 1 initial attempt + 2 retries.
         wireMock.verify(3, getRequestedFor(urlEqualTo(url)));
+        // The final exception should carry the prior failures as suppressed
+        // exceptions so the full retry history is visible in stack traces.
+        assertEquals(2, ex.getSuppressed().length,
+            "expected the 2 prior IOExceptions to be attached as suppressed");
+    }
+
+    @Test
+    public void testCustomHttpClientStillRetries() throws Exception {
+        // The Javadoc on Builder.httpClient(HttpClient) promises that the SDK's
+        // transport-failure retry wraps any supplied client. Verify it.
+        String url = "/geoip/v2.1/country/1.2.3.4";
+        String body = "{\"traits\":{\"ip_address\":\"1.2.3.4\"}}";
+
+        wireMock.stubFor(get(urlEqualTo(url))
+            .inScenario("retry-custom-client")
+            .whenScenarioStateIs(Scenario.STARTED)
+            .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))
+            .willSetStateTo("succeeded"));
+
+        wireMock.stubFor(get(urlEqualTo(url))
+            .inScenario("retry-custom-client")
+            .whenScenarioStateIs("succeeded")
+            .willReturn(aResponse()
+                .withStatus(200)
+                .withHeader("Content-Type",
+                    "application/vnd.maxmind.com-country+json; charset=UTF-8; version=2.1")
+                .withBody(body)));
+
+        HttpClient customClient = HttpClient.newBuilder().build();
+        WebServiceClient client = new WebServiceClient.Builder(6, "0123456789")
+            .host("localhost")
+            .port(wireMock.getPort())
+            .disableHttps()
+            .httpClient(customClient)
+            .build();
+
+        CountryResponse response = client.country(InetAddress.getByName("1.2.3.4"));
+        assertNotNull(response);
+
+        wireMock.verify(2, getRequestedFor(urlEqualTo(url)));
     }
 
     @Test
@@ -689,13 +731,17 @@ public void testNegativeMaxRetriesThrows() {
     }
 
     @Test
-    public void testInterruptDuringRetry() {
-        // Pre-interrupting the calling thread aborts the call before any wire
-        // request is dispatched: HttpClient.send checks the interrupt status
-        // and throws InterruptedException, which is caught and rewrapped as
-        // GeoIp2Exception with the interrupt flag restored. The wire-count
-        // assertion (zero) guards against a regression where pre-interrupt
-        // would silently let the request proceed.
+    public void testInterruptedThreadAbortsBeforeSend() {
+        // When the calling thread is already interrupted, HttpClient.send
+        // checks the interrupt status and throws InterruptedException before
+        // dispatching any wire request. The exception is caught and rewrapped
+        // as GeoIp2Exception, with the interrupt flag restored on the calling
+        // thread. The wire-count assertion (zero) guards against a regression
+        // where a pre-interrupt would silently let the request proceed.
+        // NOTE: this test does not exercise the predicate's own
+        // Thread.currentThread().isInterrupted() short-circuit, since the JDK
+        // aborts before that branch can be reached; a true mid-flight
+        // interrupt is hard to test deterministically.
         String url = "/geoip/v2.1/insights/1.2.3.4";
         wireMock.stubFor(get(urlEqualTo(url))
             .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER)));