Skip to content

Commit ac241e8

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 183d7cd commit ac241e8

1 file changed

Lines changed: 80 additions & 2 deletions

File tree

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

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616
import com.maxmind.minfraud.response.ScoreResponse;
1717
import java.io.IOException;
1818
import java.io.InputStream;
19+
import java.io.InterruptedIOException;
1920
import java.net.ProxySelector;
2021
import java.net.URI;
2122
import java.net.URISyntaxException;
2223
import java.net.http.HttpClient;
2324
import java.net.http.HttpRequest;
2425
import java.net.http.HttpResponse;
26+
import java.net.http.HttpTimeoutException;
2527
import java.nio.charset.StandardCharsets;
2628
import java.time.Duration;
2729
import java.util.Base64;
@@ -45,6 +47,7 @@ public final class WebServiceClient {
4547
private final boolean useHttps;
4648
private final List<String> locales;
4749
private final Duration requestTimeout;
50+
private final int maxRetries;
4851

4952
private final HttpClient httpClient;
5053

@@ -63,6 +66,7 @@ private WebServiceClient(WebServiceClient.Builder builder) {
6366
.getBytes(StandardCharsets.UTF_8));
6467

6568
requestTimeout = builder.requestTimeout;
69+
maxRetries = builder.maxRetries;
6670
if (builder.httpClient != null) {
6771
httpClient = builder.httpClient;
6872
} else {
@@ -106,6 +110,7 @@ public static final class Builder {
106110
List<String> locales = List.of("en");
107111
private ProxySelector proxy;
108112
private HttpClient httpClient;
113+
private int maxRetries = 1;
109114

110115
/**
111116
* @param accountId Your MaxMind account ID.
@@ -119,6 +124,12 @@ public Builder(int accountId, String licenseKey) {
119124
/**
120125
* @param val Timeout duration to establish a connection to the web service. There is no
121126
* timeout by default.
127+
* <p>
128+
* When {@code maxRetries > 0}, one API call may incur up to
129+
* {@code (maxRetries + 1)} connection attempts, each subject to
130+
* {@code connectTimeout} and {@code requestTimeout}. Worst-case wall-clock
131+
* duration is roughly
132+
* {@code (maxRetries + 1) x (connectTimeout + requestTimeout)}.
122133
* @return Builder object
123134
*/
124135
public WebServiceClient.Builder connectTimeout(Duration val) {
@@ -174,6 +185,12 @@ public WebServiceClient.Builder locales(List<String> val) {
174185

175186
/**
176187
* @param val Request timeout duration. here is no timeout by default.
188+
* <p>
189+
* When {@code maxRetries > 0}, one API call may incur up to
190+
* {@code (maxRetries + 1)} connection attempts, each subject to
191+
* {@code connectTimeout} and {@code requestTimeout}. Worst-case wall-clock
192+
* duration is roughly
193+
* {@code (maxRetries + 1) x (connectTimeout + requestTimeout)}.
177194
* @return Builder object
178195
*/
179196
public Builder requestTimeout(Duration val) {
@@ -195,13 +212,33 @@ public Builder proxy(ProxySelector val) {
195212
* @param val the HttpClient to use when making requests. When provided,
196213
* connectTimeout and proxy settings will be ignored as the
197214
* custom client should handle these configurations.
215+
* <p>
216+
* The SDK applies its own transport-failure retry on top of any supplied
217+
* client; customers can disable it via {@link #maxRetries(int)} with
218+
* {@code .maxRetries(0)}.
198219
* @return Builder object
199220
*/
200221
public Builder httpClient(HttpClient val) {
201222
httpClient = val;
202223
return this;
203224
}
204225

226+
/**
227+
* @param val Maximum number of retries on transport-level failures
228+
* (connection reset, broken pipe, connect timeout, ...).
229+
* Applies uniformly to all endpoints. Defaults to 1.
230+
* Set to 0 to disable.
231+
* @return Builder.
232+
* @throws IllegalArgumentException if {@code val} is negative.
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,10 +348,11 @@ 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) {
355+
Thread.currentThread().interrupt();
318356
throw new MinFraudException("Interrupted sending request", e);
319357
} finally {
320358
if (response != null) {
@@ -333,9 +371,10 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
333371

334372
HttpResponse<InputStream> response = null;
335373
try {
336-
response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
374+
response = sendWithRetry(request);
337375
return handleResponse(response, uri, cls);
338376
} catch (InterruptedException e) {
377+
Thread.currentThread().interrupt();
339378
throw new MinFraudException("Interrupted sending request", e);
340379
} finally {
341380
if (response != null) {
@@ -344,6 +383,44 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
344383
}
345384
}
346385

386+
private HttpResponse<InputStream> sendWithRetry(HttpRequest request)
387+
throws IOException, InterruptedException {
388+
int attempts = 0;
389+
while (true) {
390+
try {
391+
return httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
392+
} catch (IOException e) {
393+
if (!isRetriableTransportFailure(e) || attempts >= maxRetries) {
394+
throw e;
395+
}
396+
attempts++;
397+
}
398+
}
399+
}
400+
401+
private static boolean isRetriableTransportFailure(IOException e) {
402+
if (Thread.currentThread().isInterrupted()) {
403+
return false;
404+
}
405+
// Both connect-phase and request-phase timeouts are customer-set
406+
// budgets that retrying would silently extend.
407+
// HttpConnectTimeoutException extends HttpTimeoutException, so this
408+
// single check covers both.
409+
if (e instanceof HttpTimeoutException) {
410+
return false;
411+
}
412+
// The thread was interrupted during I/O; honor the cancellation.
413+
if (e instanceof InterruptedIOException) {
414+
return false;
415+
}
416+
// Everything else from httpClient.send() is a transport failure
417+
// (connection reset, broken pipe, EOF, closed channel, connect refused,
418+
// DNS, TLS, protocol). HTTP 4xx and 5xx responses cannot reach this
419+
// predicate -- they come back as HttpResponse objects rather than
420+
// IOExceptions.
421+
return true;
422+
}
423+
347424
private HttpRequest requestFor(AbstractModel transaction, URI uri)
348425
throws MinFraudException, IOException {
349426
var builder = HttpRequest.newBuilder()
@@ -354,6 +431,7 @@ private HttpRequest requestFor(AbstractModel transaction, URI uri)
354431
.header("User-Agent", userAgent)
355432
// XXX - creating this JSON string is somewhat wasteful. We
356433
// could use an input stream instead.
434+
// BodyPublishers.ofString() is replayable; safe for retry attempts
357435
.POST(HttpRequest.BodyPublishers.ofString(transaction.toJson()));
358436

359437
if (requestTimeout != null) {

0 commit comments

Comments
 (0)