Skip to content

Commit af116dd

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" or "Broken pipe". A single retry recovers transparently without exposing this race to callers. The default keep-alive timeout in the JDK is longer than many intermediaries' idle timeout, so this mismatch is the common case. The retry predicate is intentionally narrow: SocketException ("Connection reset" / "Broken pipe"), ConnectException, and HttpConnectTimeoutException only. Request-phase HttpTimeoutException, 4xx, and 5xx responses are NOT retried -- the request may have been processed and retrying could cause unwanted side effects. The predicate walks the cause chain because the JDK frequently wraps the underlying SocketException in a generic IOException ("HTTP/1.1 header parser received no bytes"). 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 af116dd

1 file changed

Lines changed: 100 additions & 2 deletions

File tree

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

Lines changed: 100 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,18 +16,23 @@
1616
import com.maxmind.minfraud.response.ScoreResponse;
1717
import java.io.IOException;
1818
import java.io.InputStream;
19+
import java.net.ConnectException;
1920
import java.net.ProxySelector;
2021
import java.net.URI;
2122
import java.net.URISyntaxException;
2223
import java.net.http.HttpClient;
24+
import java.net.http.HttpConnectTimeoutException;
2325
import java.net.http.HttpRequest;
2426
import java.net.http.HttpResponse;
27+
import java.net.http.HttpTimeoutException;
28+
import java.nio.channels.ClosedChannelException;
2529
import java.nio.charset.StandardCharsets;
2630
import java.time.Duration;
2731
import java.util.Base64;
2832
import java.util.Collections;
2933
import java.util.HashMap;
3034
import java.util.List;
35+
import java.util.Locale;
3136
import java.util.Map;
3237

3338
/**
@@ -45,6 +50,7 @@ public final class WebServiceClient {
4550
private final boolean useHttps;
4651
private final List<String> locales;
4752
private final Duration requestTimeout;
53+
private final int maxRetries;
4854

4955
private final HttpClient httpClient;
5056

@@ -63,6 +69,7 @@ private WebServiceClient(WebServiceClient.Builder builder) {
6369
.getBytes(StandardCharsets.UTF_8));
6470

6571
requestTimeout = builder.requestTimeout;
72+
maxRetries = builder.maxRetries;
6673
if (builder.httpClient != null) {
6774
httpClient = builder.httpClient;
6875
} else {
@@ -106,6 +113,7 @@ public static final class Builder {
106113
List<String> locales = List.of("en");
107114
private ProxySelector proxy;
108115
private HttpClient httpClient;
116+
private int maxRetries = 1;
109117

110118
/**
111119
* @param accountId Your MaxMind account ID.
@@ -119,6 +127,12 @@ public Builder(int accountId, String licenseKey) {
119127
/**
120128
* @param val Timeout duration to establish a connection to the web service. There is no
121129
* timeout by default.
130+
* <p>
131+
* When {@code maxRetries > 0}, one API call may incur up to
132+
* {@code (maxRetries + 1)} connection attempts, each subject to
133+
* {@code connectTimeout} and {@code requestTimeout}. Worst-case wall-clock
134+
* duration is roughly
135+
* {@code (maxRetries + 1) x (connectTimeout + requestTimeout)}.
122136
* @return Builder object
123137
*/
124138
public WebServiceClient.Builder connectTimeout(Duration val) {
@@ -174,6 +188,12 @@ public WebServiceClient.Builder locales(List<String> val) {
174188

175189
/**
176190
* @param val Request timeout duration. here is no timeout by default.
191+
* <p>
192+
* When {@code maxRetries > 0}, one API call may incur up to
193+
* {@code (maxRetries + 1)} connection attempts, each subject to
194+
* {@code connectTimeout} and {@code requestTimeout}. Worst-case wall-clock
195+
* duration is roughly
196+
* {@code (maxRetries + 1) x (connectTimeout + requestTimeout)}.
177197
* @return Builder object
178198
*/
179199
public Builder requestTimeout(Duration val) {
@@ -195,13 +215,33 @@ public Builder proxy(ProxySelector val) {
195215
* @param val the HttpClient to use when making requests. When provided,
196216
* connectTimeout and proxy settings will be ignored as the
197217
* custom client should handle these configurations.
218+
* <p>
219+
* The SDK applies its own transport-failure retry on top of any supplied
220+
* client; customers can disable it via {@link #maxRetries(int)} with
221+
* {@code .maxRetries(0)}.
198222
* @return Builder object
199223
*/
200224
public Builder httpClient(HttpClient val) {
201225
httpClient = val;
202226
return this;
203227
}
204228

229+
/**
230+
* @param val Maximum number of retries on transport-level failures
231+
* (connection reset, broken pipe, connect timeout, ...).
232+
* Applies uniformly to all endpoints. Defaults to 1.
233+
* Set to 0 to disable.
234+
* @return Builder.
235+
* @throws IllegalArgumentException if {@code val} is negative.
236+
*/
237+
public Builder maxRetries(int val) {
238+
if (val < 0) {
239+
throw new IllegalArgumentException("maxRetries must not be negative");
240+
}
241+
maxRetries = val;
242+
return this;
243+
}
244+
205245
/**
206246
* @return an instance of {@code WebServiceClient} created from the fields set on this
207247
* builder.
@@ -311,10 +351,11 @@ public void reportTransaction(TransactionReport transaction) throws IOException,
311351

312352
HttpResponse<InputStream> response = null;
313353
try {
314-
response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
354+
response = sendWithRetry(request);
315355
maybeThrowException(response, uri);
316356
exhaustBody(response);
317357
} catch (InterruptedException e) {
358+
Thread.currentThread().interrupt();
318359
throw new MinFraudException("Interrupted sending request", e);
319360
} finally {
320361
if (response != null) {
@@ -333,9 +374,10 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
333374

334375
HttpResponse<InputStream> response = null;
335376
try {
336-
response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
377+
response = sendWithRetry(request);
337378
return handleResponse(response, uri, cls);
338379
} catch (InterruptedException e) {
380+
Thread.currentThread().interrupt();
339381
throw new MinFraudException("Interrupted sending request", e);
340382
} finally {
341383
if (response != null) {
@@ -344,6 +386,61 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
344386
}
345387
}
346388

389+
private HttpResponse<InputStream> sendWithRetry(HttpRequest request)
390+
throws IOException, InterruptedException {
391+
int attempts = 0;
392+
while (true) {
393+
try {
394+
return httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
395+
} catch (IOException e) {
396+
if (!isRetriableTransportFailure(e) || attempts >= maxRetries) {
397+
throw e;
398+
}
399+
attempts++;
400+
}
401+
}
402+
}
403+
404+
private static boolean isRetriableTransportFailure(IOException e) {
405+
if (Thread.currentThread().isInterrupted()) {
406+
return false;
407+
}
408+
// Walk the cause chain: the JDK HttpClient wraps the underlying transport
409+
// failure in different ways depending on the protocol path. Over HTTP/1.1
410+
// a "Connection reset" surfaces as a SocketException; over HTTP/2 a
411+
// SETTINGS-frame write failure may surface as a plain IOException with
412+
// the same message; an HTTP/2 upgrade against an already-closed socket
413+
// surfaces as a (message-less) IOException caused by a
414+
// ClosedChannelException. Match by class for the framing types and by
415+
// message (case-insensitive) for the resets, while keeping the explicit
416+
// checks for connect-phase timeouts and request-phase timeouts (which
417+
// must NEVER be retried).
418+
Throwable t = e;
419+
while (t != null) {
420+
if (t instanceof HttpConnectTimeoutException) {
421+
return true; // subclass of HttpTimeoutException - must be checked first
422+
}
423+
if (t instanceof HttpTimeoutException) {
424+
return false; // request-phase timeout: NOT retriable
425+
}
426+
if (t instanceof ConnectException) {
427+
return true;
428+
}
429+
if (t instanceof ClosedChannelException) {
430+
return true;
431+
}
432+
String msg = t.getMessage();
433+
if (msg != null) {
434+
String lower = msg.toLowerCase(Locale.ROOT);
435+
if (lower.contains("connection reset") || lower.contains("broken pipe")) {
436+
return true;
437+
}
438+
}
439+
t = t.getCause();
440+
}
441+
return false;
442+
}
443+
347444
private HttpRequest requestFor(AbstractModel transaction, URI uri)
348445
throws MinFraudException, IOException {
349446
var builder = HttpRequest.newBuilder()
@@ -354,6 +451,7 @@ private HttpRequest requestFor(AbstractModel transaction, URI uri)
354451
.header("User-Agent", userAgent)
355452
// XXX - creating this JSON string is somewhat wasteful. We
356453
// could use an input stream instead.
454+
// BodyPublishers.ofString() is replayable; safe for retry attempts
357455
.POST(HttpRequest.BodyPublishers.ofString(transaction.toJson()));
358456

359457
if (requestTimeout != null) {

0 commit comments

Comments
 (0)