fixup! STF-322: Add bounded transport-failure retry to WebServiceClient

Author: oschwald

src/main/java/com/maxmind/geoip2/WebServiceClient.java

@@ -31,11 +31,13 @@
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
 import java.net.http.HttpTimeoutException;
+import java.nio.channels.ClosedChannelException;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.util.Base64;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 /**
@@ -423,20 +425,17 @@ private <T> T responseFor(String path, InetAddress ipAddress, Class<T> cls)
 
     private HttpResponse<InputStream> sendWithRetry(HttpRequest request)
         throws IOException, InterruptedException {
-        IOException lastException = null;
-        int attempts = maxRetries + 1;
-        for (int i = 0; i < attempts; i++) {
+        int attempts = 0;
+        while (true) {
             try {
                 return httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
             } catch (IOException e) {
-                if (!isRetriableTransportFailure(e) || i == attempts - 1) {
+                if (!isRetriableTransportFailure(e) || attempts >= maxRetries) {
                     throw e;
                 }
-                lastException = e;
+                attempts++;
             }
         }
-        // Unreachable: loop either returns or throws.
-        throw lastException;
     }
 
     private static boolean isRetriableTransportFailure(IOException e) {
@@ -445,11 +444,14 @@ private static boolean isRetriableTransportFailure(IOException e) {
         }
         // Walk the cause chain: the JDK HttpClient wraps the underlying transport
         // failure in different ways depending on the protocol path. Over HTTP/1.1
-        // a "Connection reset" surfaces as a SocketException; over HTTP/2 (e.g.
-        // a SETTINGS-frame write failure) it may surface as a plain IOException
-        // with the same message. Match by message regardless of class to handle
-        // both, while keeping the type checks for connect-phase timeouts and
-        // request-phase timeouts (which must NEVER be retried).
+        // a "Connection reset" surfaces as a SocketException; over HTTP/2 a
+        // SETTINGS-frame write failure may surface as a plain IOException with
+        // the same message; an HTTP/2 upgrade against an already-closed socket
+        // surfaces as a (message-less) IOException caused by a
+        // ClosedChannelException. Match by class for the framing types and by
+        // message (case-insensitive) for the resets, while keeping the explicit
+        // checks for connect-phase timeouts and request-phase timeouts (which
+        // must NEVER be retried).
         Throwable t = e;
         while (t != null) {
             if (t instanceof HttpConnectTimeoutException) {
@@ -461,11 +463,16 @@ private static boolean isRetriableTransportFailure(IOException e) {
             if (t instanceof ConnectException) {
                 return true;
             }
-            String msg = t.getMessage();
-            if (msg != null
-                && (msg.contains("Connection reset") || msg.contains("Broken pipe"))) {
+            if (t instanceof ClosedChannelException) {
                 return true;
             }
+            String msg = t.getMessage();
+            if (msg != null) {
+                String lower = msg.toLowerCase(Locale.ROOT);
+                if (lower.contains("connection reset") || lower.contains("broken pipe")) {
+                    return true;
+                }
+            }
             t = t.getCause();
         }
         return false;