Merge pull request #619 from maxmind/greg/stf-322

STF-322: Bounded transport-failure retry in WebServiceClient

Author: horgh

CHANGELOG.md

@@ -7,6 +7,12 @@ CHANGELOG
 * Added `FAT_ZEBRA` to the `Payment.Processor` enum.
 * Added `CLEAR` to the `TransactionReport.Tag` enum for use with the Report
   Transaction API.
+* Added `WebServiceClient.Builder.maxRetries(int)` to bound transport-failure
+  retries (default 1; set 0 to disable). See the README for retry semantics.
+  **Behavior change:** previously, transient transport failures (connection
+  reset, broken pipe, etc.) surfaced to callers immediately. They are now
+  retried once by default; pass `.maxRetries(0)` to restore the prior
+  behavior.
 
 4.2.0 (2026-02-26)
 ------------------

README.md

@@ -110,6 +110,46 @@ exception will be thrown.
 
 See the API documentation for more details.
 
+### Connection pooling and transport retries ###
+
+`WebServiceClient` reuses pooled HTTP connections for performance. Idle
+connections can be silently closed by load balancers or other
+intermediaries; when the next request reuses such a half-closed connection,
+the JDK reports the failure as a `Connection reset`, `Broken pipe`, or
+similar transport error.
+
+To smooth over these intermittent failures, the SDK retries once by
+default. Most transport-level `IOException`s are retried; the SDK does
+**not** retry:
+
+* **Timeouts** (`HttpTimeoutException`, including connect-phase timeouts).
+  The SDK honors the timeouts you configure rather than extending them.
+* **Cancellation** (`InterruptedIOException`, or any interrupt observed
+  before the request runs).
+* **Typically deterministic failures** — `UnknownHostException`,
+  `ConnectException`, `SSLHandshakeException`, `SSLPeerUnverifiedException`.
+  Retrying these would just delay surfacing a config bug.
+
+HTTP 4xx and 5xx responses are surfaced through the existing exception
+hierarchy and are never retried. Request bodies are replayable, so retried
+requests are byte-identical to the original.
+
+You can change the retry budget via the builder:
+
+```java
+WebServiceClient client = new WebServiceClient.Builder(6, "ABCD567890")
+    .maxRetries(2) // up to two retries (three total attempts)
+    .build();
+```
+
+Set `.maxRetries(0)` to disable the retry entirely. Negative values throw
+`IllegalArgumentException`.
+
+If you frequently see `Connection reset` errors, you can also reduce the
+JDK's keep-alive timeout via the system property
+`jdk.httpclient.keepalive.timeout` (in seconds) to evict pooled connections
+before any intermediary does so.
+
 ### Exceptions ###
 
 Runtime exceptions:

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

@@ -16,19 +16,25 @@
 import com.maxmind.minfraud.response.ScoreResponse;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.InterruptedIOException;
+import java.net.ConnectException;
 import java.net.ProxySelector;
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.UnknownHostException;
 import java.net.http.HttpClient;
 import java.net.http.HttpRequest;
 import java.net.http.HttpResponse;
+import java.net.http.HttpTimeoutException;
 import java.nio.charset.StandardCharsets;
 import java.time.Duration;
 import java.util.Base64;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import javax.net.ssl.SSLHandshakeException;
+import javax.net.ssl.SSLPeerUnverifiedException;
 
 /**
  * Client for MaxMind minFraud Score, Insights, and Factors
@@ -45,6 +51,7 @@ public final class WebServiceClient {
     private final boolean useHttps;
     private final List<String> locales;
     private final Duration requestTimeout;
+    private final int maxRetries;
 
     private final HttpClient httpClient;
 
@@ -63,6 +70,7 @@ private WebServiceClient(WebServiceClient.Builder builder) {
                 .getBytes(StandardCharsets.UTF_8));
 
         requestTimeout = builder.requestTimeout;
+        maxRetries = builder.maxRetries;
         if (builder.httpClient != null) {
             httpClient = builder.httpClient;
         } else {
@@ -106,6 +114,7 @@ public static final class Builder {
         List<String> locales = List.of("en");
         private ProxySelector proxy;
         private HttpClient httpClient;
+        private int maxRetries = 1;
 
         /**
          * @param accountId  Your MaxMind account ID.
@@ -120,6 +129,7 @@ public Builder(int accountId, String licenseKey) {
          * @param val Timeout duration to establish a connection to the web service. There is no
          *            timeout by default.
          * @return Builder object
+         * @apiNote See {@link #maxRetries(int)} for how this timeout interacts with retries.
          */
         public WebServiceClient.Builder connectTimeout(Duration val) {
             connectTimeout = val;
@@ -173,8 +183,9 @@ public WebServiceClient.Builder locales(List<String> val) {
 
 
         /**
-         * @param val Request timeout duration. here is no timeout by default.
+         * @param val Request timeout duration. There is no timeout by default.
          * @return Builder object
+         * @apiNote See {@link #maxRetries(int)} for how this timeout interacts with retries.
          */
         public Builder requestTimeout(Duration val) {
             requestTimeout = val;
@@ -195,13 +206,37 @@ public Builder proxy(ProxySelector val) {
          * @param val the HttpClient to use when making requests. When provided,
          *            connectTimeout and proxy settings will be ignored as the
          *            custom client should handle these configurations.
+         *            <p>
+         *            The SDK applies its own transport-failure retry on top of any supplied
+         *            client; pass {@code 0} to {@link #maxRetries(int)} to disable.
          * @return Builder object
          */
         public Builder httpClient(HttpClient val) {
             httpClient = val;
             return this;
         }
 
+        /**
+         * @param val Maximum number of retries on transport-level failures
+         *            (connection reset, broken pipe, EOF, ...).
+         *            Applies uniformly to all endpoints. Defaults to 1.
+         *            Set to 0 to disable.
+         * @return Builder.
+         * @throws IllegalArgumentException if {@code val} is negative.
+         * @apiNote Retries fire only on transient transport failures.
+         *     Timeouts and other non-transient errors are not retried — see
+         *     the README for the complete list. When all attempts fail,
+         *     the prior {@code IOException}s are attached via
+         *     {@link Throwable#getSuppressed()} for debugging.
+         */
+        public Builder maxRetries(int val) {
+            if (val < 0) {
+                throw new IllegalArgumentException("maxRetries must not be negative");
+            }
+            maxRetries = val;
+            return this;
+        }
+
         /**
          * @return an instance of {@code WebServiceClient} created from the fields set on this
          *     builder.
@@ -311,10 +346,11 @@ public void reportTransaction(TransactionReport transaction) throws IOException,
 
         HttpResponse<InputStream> response = null;
         try {
-            response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
+            response = sendWithRetry(request);
             maybeThrowException(response, uri);
             exhaustBody(response);
         } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
             throw new MinFraudException("Interrupted sending request", e);
         } finally {
             if (response != null) {
@@ -333,9 +369,10 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
 
         HttpResponse<InputStream> response = null;
         try {
-            response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
+            response = sendWithRetry(request);
             return handleResponse(response, uri, cls);
         } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
             throw new MinFraudException("Interrupted sending request", e);
         } finally {
             if (response != null) {
@@ -344,6 +381,68 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
         }
     }
 
+    private HttpResponse<InputStream> sendWithRetry(HttpRequest request)
+        throws IOException, InterruptedException {
+        int attempts = 0;
+        IOException prior = null;
+        while (true) {
+            try {
+                return httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
+            } catch (IOException e) {
+                // Attach the immediate predecessor so the suppressed chain
+                // carries the full retry history (each link is the previous
+                // attempt's failure; walk via Throwable#getSuppressed).
+                if (prior != null) {
+                    e.addSuppressed(prior);
+                }
+                if (!isRetriableTransportFailure(e) || attempts >= maxRetries) {
+                    throw e;
+                }
+                prior = e;
+                attempts++;
+            }
+        }
+    }
+
+    private static boolean isRetriableTransportFailure(IOException e) {
+        if (Thread.currentThread().isInterrupted()) {
+            return false;
+        }
+        // Both connect-phase and request-phase timeouts are customer-set
+        // budgets that retrying would silently extend.
+        // HttpConnectTimeoutException extends HttpTimeoutException, so this
+        // single check covers both.
+        if (e instanceof HttpTimeoutException) {
+            return false;
+        }
+        // The thread was interrupted during I/O; honor the cancellation.
+        if (e instanceof InterruptedIOException) {
+            return false;
+        }
+        // The four exclusions below are *occasionally* transient (DNS hiccup,
+        // TCP RST race during cert rotation, brief LB outage), but treating
+        // them as deterministic is a deliberate product decision: retrying
+        // would mask config bugs behind 2x latency, and the customer-visible
+        // cost of one extra failed call on a true transient is small.
+        if (e instanceof UnknownHostException) {
+            return false;
+        }
+        if (e instanceof ConnectException) {
+            return false;
+        }
+        if (e instanceof SSLHandshakeException) {
+            return false;
+        }
+        if (e instanceof SSLPeerUnverifiedException) {
+            return false;
+        }
+        // Everything else from httpClient.send() is a transport failure
+        // (connection reset, broken pipe, EOF, closed channel, ...).
+        // HTTP 4xx and 5xx responses do not reach this predicate -- they come
+        // back as HttpResponse objects rather than IOExceptions.
+        return true;
+    }
+
     private HttpRequest requestFor(AbstractModel transaction, URI uri)
         throws MinFraudException, IOException {
         var builder = HttpRequest.newBuilder()
@@ -354,6 +453,7 @@ private HttpRequest requestFor(AbstractModel transaction, URI uri)
             .header("User-Agent", userAgent)
             // XXX - creating this JSON string is somewhat wasteful. We
             // could use an input stream instead.
+            // BodyPublishers.ofString() is replayable; safe for retry attempts
             .POST(HttpRequest.BodyPublishers.ofString(transaction.toJson()));
 
         if (requestTimeout != null) {