diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b6ef9b8..0789265b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ 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. 4.2.0 (2026-02-26) ------------------ diff --git a/README.md b/README.md index 7b0abd26..70bb3bbb 100644 --- a/README.md +++ b/README.md @@ -110,6 +110,50 @@ exception will be thrown. See the API documentation for more details. +### Connection pooling and transport retries ### + +`WebServiceClient` is thread-safe and reuses a pooled `HttpClient` across +requests. Idle connections in the pool can be silently closed by load +balancers or other intermediaries. When the next request reuses one of these +half-closed connections, the JDK reports the failure as a `Connection reset`, +`Broken pipe`, or related transport `IOException`. + +To smooth over these intermittent transport failures, the SDK retries once +by default. Any transport-level `IOException` raised by the underlying HTTP +send is retried, with the following exclusions: + +* `HttpTimeoutException` — a request-phase timeout. Connect-phase timeouts + (`HttpConnectTimeoutException`) are also excluded because they extend + `HttpTimeoutException`. The SDK honors the timeouts you configure. +* `InterruptedIOException` — the calling thread was interrupted; the SDK + honors the cancellation rather than override it. +* Typically deterministic failures: `UnknownHostException`, + `ConnectException`, `SSLHandshakeException`, `SSLPeerUnverifiedException`. + Retrying these would just delay surfacing a config bug. +* If the calling thread is already interrupted when the predicate runs, the + retry is short-circuited regardless of the exception type. + +HTTP 4xx and 5xx responses are not retried — they are returned as +`HttpResponse` objects (not `IOException`s) and surfaced through the existing +exception hierarchy. POST 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: diff --git a/src/main/java/com/maxmind/minfraud/WebServiceClient.java b/src/main/java/com/maxmind/minfraud/WebServiceClient.java index 0fb5e561..f9e4dade 100644 --- a/src/main/java/com/maxmind/minfraud/WebServiceClient.java +++ b/src/main/java/com/maxmind/minfraud/WebServiceClient.java @@ -16,12 +16,16 @@ 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; @@ -29,6 +33,8 @@ 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 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 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 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,6 +206,10 @@ 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. + *

+ * The SDK applies its own transport-failure retry on top of any supplied + * client; customers can disable it via {@link #maxRetries(int)} with + * {@code .maxRetries(0)}. * @return Builder object */ public Builder httpClient(HttpClient val) { @@ -202,6 +217,31 @@ public Builder httpClient(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 Timeouts are not retried ({@link java.net.http.HttpTimeoutException}, + * including the connect-phase subclass + * {@link java.net.http.HttpConnectTimeoutException}). When + * {@code maxRetries > 0}, retries are triggered only by fast transport + * failures, so each attempt is independently bounded by + * {@link #connectTimeout(Duration)} and {@link #requestTimeout(Duration)}. + * The multiplied worst-case wall clock a naive reading suggests is + * unreachable in practice, since hitting the timeout aborts the call + * rather than triggering a retry. + */ + 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 +351,11 @@ public void reportTransaction(TransactionReport transaction) throws IOException, HttpResponse 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 +374,10 @@ private T responseFor(String service, AbstractModel transaction, Class cl HttpResponse 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 +386,62 @@ private T responseFor(String service, AbstractModel transaction, Class cl } } + private HttpResponse 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) { + 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; + } + // Typically deterministic failures: retrying just delays surfacing the + // config bug without recovering the request. + 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 +452,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) { diff --git a/src/test/java/com/maxmind/minfraud/WebServiceClientTest.java b/src/test/java/com/maxmind/minfraud/WebServiceClientTest.java index f2cc57e9..48b3d6c3 100644 --- a/src/test/java/com/maxmind/minfraud/WebServiceClientTest.java +++ b/src/test/java/com/maxmind/minfraud/WebServiceClientTest.java @@ -22,8 +22,10 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.github.tomakehurst.wiremock.http.Fault; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; import com.github.tomakehurst.wiremock.junit5.WireMockTest; +import com.github.tomakehurst.wiremock.stubbing.Scenario; import com.maxmind.minfraud.exception.AuthenticationException; import com.maxmind.minfraud.exception.HttpException; import com.maxmind.minfraud.exception.InsufficientFundsException; @@ -41,9 +43,11 @@ import com.maxmind.minfraud.response.InsightsResponse; import com.maxmind.minfraud.response.IpRiskReason; import com.maxmind.minfraud.response.ScoreResponse; +import java.io.IOException; import java.net.InetAddress; import java.net.ProxySelector; import java.net.http.HttpClient; +import java.net.http.HttpTimeoutException; import java.time.Duration; import java.util.List; import org.junit.jupiter.api.Test; @@ -423,4 +427,198 @@ public void testHttpClientWithProxyThrowsException() { assertEquals("Cannot set both httpClient and proxy. " + "Configure proxy on the provided HttpClient instead.", ex.getMessage()); } + + @Test + public void testRetriesOnConnectionReset_score() throws Exception { + var responseContent = readJsonFile("score-response"); + var url = "/minfraud/v2.0/score"; + + wireMock.stubFor(post(urlEqualTo(url)) + .inScenario("retry-score") + .whenScenarioStateIs(Scenario.STARTED) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER)) + .willSetStateTo("succeeded")); + + wireMock.stubFor(post(urlEqualTo(url)) + .inScenario("retry-score") + .whenScenarioStateIs("succeeded") + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", + "application/vnd.maxmind.com-minfraud-score+json; charset=UTF-8; version=2.0\n") + .withBody(responseContent))); + + var client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + var response = client.score(fullTransaction()); + JSONAssert.assertEquals(responseContent, response.toJson(), true); + + wireMock.verify(2, postRequestedFor(urlEqualTo(url))); + } + + @Test + public void testRetriesOnConnectionReset_reportTransaction() throws Exception { + var url = "/minfraud/v2.0/transactions/report"; + + wireMock.stubFor(post(urlEqualTo(url)) + .inScenario("retry-report") + .whenScenarioStateIs(Scenario.STARTED) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER)) + .willSetStateTo("succeeded")); + + wireMock.stubFor(post(urlEqualTo(url)) + .inScenario("retry-report") + .whenScenarioStateIs("succeeded") + .willReturn(aResponse().withStatus(204))); + + var client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + client.reportTransaction(fullTransactionReport()); + + wireMock.verify(2, postRequestedFor(urlEqualTo(url))); + } + + @Test + public void testNoRetryOnHttpTimeoutException() { + var url = "/minfraud/v2.0/insights"; + wireMock.stubFor(post(urlEqualTo(url)) + .willReturn(aResponse() + .withStatus(200) + .withFixedDelay(2000) + .withBody("{}"))); + + var client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .requestTimeout(Duration.ofMillis(100)) + .build(); + + assertThrows(HttpTimeoutException.class, () -> client.insights(fullTransaction())); + + wireMock.verify(1, postRequestedFor(urlEqualTo(url))); + } + + @Test + public void testNoRetryOn5xx() { + var url = "/minfraud/v2.0/insights"; + wireMock.stubFor(post(urlEqualTo(url)) + .willReturn(aResponse() + .withStatus(500) + .withHeader("Content-Type", "application/json") + .withBody(""))); + + var client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + assertThrows(HttpException.class, () -> client.insights(fullTransaction())); + + wireMock.verify(1, postRequestedFor(urlEqualTo(url))); + } + + @Test + public void testNoRetryOn4xx() { + var url = "/minfraud/v2.0/insights"; + wireMock.stubFor(post(urlEqualTo(url)) + .willReturn(aResponse() + .withStatus(402) + .withHeader("Content-Type", "application/json") + .withBody("{\"code\":\"INSUFFICIENT_FUNDS\",\"error\":\"out of credit\"}"))); + + var client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + assertThrows(InsufficientFundsException.class, + () -> client.insights(fullTransaction())); + + wireMock.verify(1, postRequestedFor(urlEqualTo(url))); + } + + @Test + public void testMaxRetriesZeroDisablesRetry() { + var url = "/minfraud/v2.0/insights"; + wireMock.stubFor(post(urlEqualTo(url)) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); + + var client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .maxRetries(0) + .build(); + + assertThrows(IOException.class, () -> client.insights(fullTransaction())); + + wireMock.verify(1, postRequestedFor(urlEqualTo(url))); + } + + @Test + public void testRetriesExhausted() { + var url = "/minfraud/v2.0/insights"; + wireMock.stubFor(post(urlEqualTo(url)) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); + + var client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .maxRetries(2) + .build(); + + assertThrows(IOException.class, () -> client.insights(fullTransaction())); + + // 1 initial attempt + 2 retries. + wireMock.verify(3, postRequestedFor(urlEqualTo(url))); + } + + @Test + public void testNegativeMaxRetriesThrows() { + var builder = new WebServiceClient.Builder(6, "0123456789"); + assertThrows(IllegalArgumentException.class, () -> builder.maxRetries(-1)); + } + + @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 + // MinFraudException with the interrupt flag restored. The wire-count + // assertion (zero) guards against a regression where pre-interrupt + // would silently let the request proceed. + var url = "/minfraud/v2.0/insights"; + wireMock.stubFor(post(urlEqualTo(url)) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); + + var client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + Thread.currentThread().interrupt(); + try { + assertThrows(MinFraudException.class, () -> client.insights(fullTransaction())); + assertTrue(Thread.currentThread().isInterrupted(), + "interrupt flag should remain set after the call"); + } finally { + // Clear the interrupt flag so it does not leak to other tests + // (and so wireMock.verify below isn't affected by it). + Thread.interrupted(); + } + wireMock.verify(0, postRequestedFor(urlEqualTo(url))); + } }