Skip to content

Commit 7cca002

Browse files
oschwaldclaude
andcommitted
STF-322: Add bounded transport-failure retry to WebServiceClient
When the JDK HttpClient pool reuses an idle connection that an intermediary (load balancer, proxy, NAT) has silently closed, the next send() fails with "Connection reset", "Broken pipe", or related transport errors. A single retry recovers transparently without exposing this race to callers. The default JDK keep-alive timeout exceeds many intermediaries' idle timeout, making this mismatch the common case. The retry predicate is permissive by exclusion: any IOException from httpClient.send() is retried EXCEPT HttpTimeoutException (covering both request-phase and connect-phase timeouts, since HttpConnectTimeoutException is a subclass) and InterruptedIOException. Both timeouts are customer-set budgets that retrying would silently extend; InterruptedIOException is a user-cancellation signal. HTTP 4xx and 5xx responses are surfaced as HttpException (and subclasses) from a separate code path -- they come back as HttpResponse objects rather than IOExceptions, so the predicate is structurally unable to retry them. Customers can opt out via .maxRetries(0). Default is 1 (one retry, two total attempts). The interrupt flag is restored before rewrapping InterruptedException, and a pre-set interrupt short-circuits the predicate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 68f7c7d commit 7cca002

1 file changed

Lines changed: 102 additions & 3 deletions

File tree

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

Lines changed: 102 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,26 @@
1616
import com.maxmind.minfraud.response.ScoreResponse;
1717
import java.io.IOException;
1818
import java.io.InputStream;
19+
import java.io.InterruptedIOException;
20+
import java.net.ConnectException;
1921
import java.net.ProxySelector;
2022
import java.net.URI;
2123
import java.net.URISyntaxException;
24+
import java.net.UnknownHostException;
2225
import java.net.http.HttpClient;
2326
import java.net.http.HttpRequest;
2427
import java.net.http.HttpResponse;
28+
import java.net.http.HttpTimeoutException;
2529
import java.nio.charset.StandardCharsets;
2630
import java.time.Duration;
31+
import java.util.ArrayList;
2732
import java.util.Base64;
2833
import java.util.Collections;
2934
import java.util.HashMap;
3035
import java.util.List;
3136
import java.util.Map;
37+
import javax.net.ssl.SSLHandshakeException;
38+
import javax.net.ssl.SSLPeerUnverifiedException;
3239

3340
/**
3441
* Client for MaxMind minFraud Score, Insights, and Factors
@@ -45,6 +52,7 @@ public final class WebServiceClient {
4552
private final boolean useHttps;
4653
private final List<String> locales;
4754
private final Duration requestTimeout;
55+
private final int maxRetries;
4856

4957
private final HttpClient httpClient;
5058

@@ -63,6 +71,7 @@ private WebServiceClient(WebServiceClient.Builder builder) {
6371
.getBytes(StandardCharsets.UTF_8));
6472

6573
requestTimeout = builder.requestTimeout;
74+
maxRetries = builder.maxRetries;
6675
if (builder.httpClient != null) {
6776
httpClient = builder.httpClient;
6877
} else {
@@ -106,6 +115,7 @@ public static final class Builder {
106115
List<String> locales = List.of("en");
107116
private ProxySelector proxy;
108117
private HttpClient httpClient;
118+
private int maxRetries = 1;
109119

110120
/**
111121
* @param accountId Your MaxMind account ID.
@@ -120,6 +130,7 @@ public Builder(int accountId, String licenseKey) {
120130
* @param val Timeout duration to establish a connection to the web service. There is no
121131
* timeout by default.
122132
* @return Builder object
133+
* @apiNote See {@link #maxRetries(int)} for how this timeout interacts with retries.
123134
*/
124135
public WebServiceClient.Builder connectTimeout(Duration val) {
125136
connectTimeout = val;
@@ -173,8 +184,9 @@ public WebServiceClient.Builder locales(List<String> val) {
173184

174185

175186
/**
176-
* @param val Request timeout duration. here is no timeout by default.
187+
* @param val Request timeout duration. There is no timeout by default.
177188
* @return Builder object
189+
* @apiNote See {@link #maxRetries(int)} for how this timeout interacts with retries.
178190
*/
179191
public Builder requestTimeout(Duration val) {
180192
requestTimeout = val;
@@ -195,13 +207,38 @@ public Builder proxy(ProxySelector val) {
195207
* @param val the HttpClient to use when making requests. When provided,
196208
* connectTimeout and proxy settings will be ignored as the
197209
* custom client should handle these configurations.
210+
* <p>
211+
* The SDK applies its own transport-failure retry on top of any supplied
212+
* client; customers can disable it via {@link #maxRetries(int)} with
213+
* {@code .maxRetries(0)}.
198214
* @return Builder object
199215
*/
200216
public Builder httpClient(HttpClient val) {
201217
httpClient = val;
202218
return this;
203219
}
204220

221+
/**
222+
* @param val Maximum number of retries on transport-level failures
223+
* (connection reset, broken pipe, EOF, ...).
224+
* Applies uniformly to all endpoints. Defaults to 1.
225+
* Set to 0 to disable.
226+
* @return Builder.
227+
* @throws IllegalArgumentException if {@code val} is negative.
228+
* @apiNote Retries fire only on transient transport failures.
229+
* Timeouts and other non-transient errors are not retried — see
230+
* the README for the complete list. When all attempts fail,
231+
* the prior {@code IOException}s are attached via
232+
* {@link Throwable#getSuppressed()} for debugging.
233+
*/
234+
public Builder maxRetries(int val) {
235+
if (val < 0) {
236+
throw new IllegalArgumentException("maxRetries must not be negative");
237+
}
238+
maxRetries = val;
239+
return this;
240+
}
241+
205242
/**
206243
* @return an instance of {@code WebServiceClient} created from the fields set on this
207244
* builder.
@@ -311,7 +348,7 @@ public void reportTransaction(TransactionReport transaction) throws IOException,
311348

312349
HttpResponse<InputStream> response = null;
313350
try {
314-
response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
351+
response = sendWithRetry(request);
315352
maybeThrowException(response, uri);
316353
exhaustBody(response);
317354
} catch (InterruptedException e) {
@@ -333,7 +370,7 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
333370

334371
HttpResponse<InputStream> response = null;
335372
try {
336-
response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
373+
response = sendWithRetry(request);
337374
return handleResponse(response, uri, cls);
338375
} catch (InterruptedException e) {
339376
throw new MinFraudException("Interrupted sending request", e);
@@ -344,6 +381,67 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
344381
}
345382
}
346383

384+
private HttpResponse<InputStream> sendWithRetry(HttpRequest request)
385+
throws IOException, InterruptedException {
386+
int attempts = 0;
387+
List<IOException> priors = new ArrayList<>();
388+
while (true) {
389+
try {
390+
return httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
391+
} catch (IOException e) {
392+
// Attach all prior IOExceptions directly so the final stack
393+
// trace carries the full retry history without nesting.
394+
for (IOException p : priors) {
395+
e.addSuppressed(p);
396+
}
397+
if (!isRetriableTransportFailure(e) || attempts >= maxRetries) {
398+
throw e;
399+
}
400+
priors.add(e);
401+
attempts++;
402+
}
403+
}
404+
}
405+
406+
private static boolean isRetriableTransportFailure(IOException e) {
407+
if (Thread.currentThread().isInterrupted()) {
408+
return false;
409+
}
410+
// Both connect-phase and request-phase timeouts are customer-set
411+
// budgets that retrying would silently extend.
412+
// HttpConnectTimeoutException extends HttpTimeoutException, so this
413+
// single check covers both.
414+
if (e instanceof HttpTimeoutException) {
415+
return false;
416+
}
417+
// The thread was interrupted during I/O; honor the cancellation.
418+
if (e instanceof InterruptedIOException) {
419+
return false;
420+
}
421+
// The four exclusions below are *occasionally* transient (DNS hiccup,
422+
// TCP RST race during cert rotation, brief LB outage), but treating
423+
// them as deterministic is a deliberate product decision: retrying
424+
// would mask config bugs behind 2x latency, and the customer-visible
425+
// cost of one extra failed call on a true transient is small.
426+
if (e instanceof UnknownHostException) {
427+
return false;
428+
}
429+
if (e instanceof ConnectException) {
430+
return false;
431+
}
432+
if (e instanceof SSLHandshakeException) {
433+
return false;
434+
}
435+
if (e instanceof SSLPeerUnverifiedException) {
436+
return false;
437+
}
438+
// Everything else from httpClient.send() is a transport failure
439+
// (connection reset, broken pipe, EOF, closed channel, ...).
440+
// HTTP 4xx and 5xx responses do not reach this predicate -- they come
441+
// back as HttpResponse objects rather than IOExceptions.
442+
return true;
443+
}
444+
347445
private HttpRequest requestFor(AbstractModel transaction, URI uri)
348446
throws MinFraudException, IOException {
349447
var builder = HttpRequest.newBuilder()
@@ -354,6 +452,7 @@ private HttpRequest requestFor(AbstractModel transaction, URI uri)
354452
.header("User-Agent", userAgent)
355453
// XXX - creating this JSON string is somewhat wasteful. We
356454
// could use an input stream instead.
455+
// BodyPublishers.ofString() is replayable; safe for retry attempts
357456
.POST(HttpRequest.BodyPublishers.ofString(transaction.toJson()));
358457

359458
if (requestTimeout != null) {

0 commit comments

Comments
 (0)