Skip to content

Commit b1705ef

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 b1705ef

1 file changed

Lines changed: 90 additions & 2 deletions

File tree

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

Lines changed: 90 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,16 @@
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;
21+
import java.net.SocketException;
2022
import java.net.URI;
2123
import java.net.URISyntaxException;
2224
import java.net.http.HttpClient;
25+
import java.net.http.HttpConnectTimeoutException;
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;
2731
import java.util.Base64;
@@ -45,6 +49,7 @@ public final class WebServiceClient {
4549
private final boolean useHttps;
4650
private final List<String> locales;
4751
private final Duration requestTimeout;
52+
private final int maxRetries;
4853

4954
private final HttpClient httpClient;
5055

@@ -63,6 +68,7 @@ private WebServiceClient(WebServiceClient.Builder builder) {
6368
.getBytes(StandardCharsets.UTF_8));
6469

6570
requestTimeout = builder.requestTimeout;
71+
maxRetries = builder.maxRetries;
6672
if (builder.httpClient != null) {
6773
httpClient = builder.httpClient;
6874
} else {
@@ -106,6 +112,7 @@ public static final class Builder {
106112
List<String> locales = List.of("en");
107113
private ProxySelector proxy;
108114
private HttpClient httpClient;
115+
private int maxRetries = 1;
109116

110117
/**
111118
* @param accountId Your MaxMind account ID.
@@ -119,6 +126,12 @@ public Builder(int accountId, String licenseKey) {
119126
/**
120127
* @param val Timeout duration to establish a connection to the web service. There is no
121128
* timeout by default.
129+
* <p>
130+
* When {@code maxRetries > 0}, one API call may incur up to
131+
* {@code (maxRetries + 1)} connection attempts, each subject to
132+
* {@code connectTimeout} and {@code requestTimeout}. Worst-case wall-clock
133+
* duration is roughly
134+
* {@code (maxRetries + 1) x (connectTimeout + requestTimeout)}.
122135
* @return Builder object
123136
*/
124137
public WebServiceClient.Builder connectTimeout(Duration val) {
@@ -174,6 +187,12 @@ public WebServiceClient.Builder locales(List<String> val) {
174187

175188
/**
176189
* @param val Request timeout duration. here is no timeout by default.
190+
* <p>
191+
* When {@code maxRetries > 0}, one API call may incur up to
192+
* {@code (maxRetries + 1)} connection attempts, each subject to
193+
* {@code connectTimeout} and {@code requestTimeout}. Worst-case wall-clock
194+
* duration is roughly
195+
* {@code (maxRetries + 1) x (connectTimeout + requestTimeout)}.
177196
* @return Builder object
178197
*/
179198
public Builder requestTimeout(Duration val) {
@@ -195,13 +214,33 @@ public Builder proxy(ProxySelector val) {
195214
* @param val the HttpClient to use when making requests. When provided,
196215
* connectTimeout and proxy settings will be ignored as the
197216
* custom client should handle these configurations.
217+
* <p>
218+
* The SDK applies its own transport-failure retry on top of any supplied
219+
* client; customers can disable it via {@link #maxRetries(int)} with
220+
* {@code .maxRetries(0)}.
198221
* @return Builder object
199222
*/
200223
public Builder httpClient(HttpClient val) {
201224
httpClient = val;
202225
return this;
203226
}
204227

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

312351
HttpResponse<InputStream> response = null;
313352
try {
314-
response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
353+
response = sendWithRetry(request);
315354
maybeThrowException(response, uri);
316355
exhaustBody(response);
317356
} catch (InterruptedException e) {
357+
Thread.currentThread().interrupt();
318358
throw new MinFraudException("Interrupted sending request", e);
319359
} finally {
320360
if (response != null) {
@@ -333,9 +373,10 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
333373

334374
HttpResponse<InputStream> response = null;
335375
try {
336-
response = httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
376+
response = sendWithRetry(request);
337377
return handleResponse(response, uri, cls);
338378
} catch (InterruptedException e) {
379+
Thread.currentThread().interrupt();
339380
throw new MinFraudException("Interrupted sending request", e);
340381
} finally {
341382
if (response != null) {
@@ -344,6 +385,52 @@ private <T> T responseFor(String service, AbstractModel transaction, Class<T> cl
344385
}
345386
}
346387

388+
private HttpResponse<InputStream> sendWithRetry(HttpRequest request)
389+
throws IOException, InterruptedException {
390+
IOException lastException = null;
391+
int attempts = maxRetries + 1;
392+
for (int i = 0; i < attempts; i++) {
393+
try {
394+
return httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
395+
} catch (IOException e) {
396+
if (!isRetriableTransportFailure(e) || i == attempts - 1) {
397+
throw e;
398+
}
399+
lastException = e;
400+
}
401+
}
402+
// Unreachable: loop either returns or throws.
403+
throw lastException;
404+
}
405+
406+
private static boolean isRetriableTransportFailure(IOException e) {
407+
if (Thread.currentThread().isInterrupted()) {
408+
return false;
409+
}
410+
// Walk the cause chain: the JDK HttpClient often wraps the underlying
411+
// transport failure (e.g. SocketException "Connection reset") in a
412+
// generic IOException ("HTTP/1.1 header parser received no bytes").
413+
Throwable t = e;
414+
while (t != null) {
415+
if (t instanceof HttpConnectTimeoutException) {
416+
return true; // subclass of HttpTimeoutException - must be checked first
417+
}
418+
if (t instanceof HttpTimeoutException) {
419+
return false; // request-phase timeout: NOT retriable
420+
}
421+
if (t instanceof ConnectException) {
422+
return true;
423+
}
424+
if (t instanceof SocketException) {
425+
String msg = t.getMessage();
426+
return msg != null
427+
&& (msg.contains("Connection reset") || msg.contains("Broken pipe"));
428+
}
429+
t = t.getCause();
430+
}
431+
return false;
432+
}
433+
347434
private HttpRequest requestFor(AbstractModel transaction, URI uri)
348435
throws MinFraudException, IOException {
349436
var builder = HttpRequest.newBuilder()
@@ -354,6 +441,7 @@ private HttpRequest requestFor(AbstractModel transaction, URI uri)
354441
.header("User-Agent", userAgent)
355442
// XXX - creating this JSON string is somewhat wasteful. We
356443
// could use an input stream instead.
444+
// BodyPublishers.ofString() is replayable; safe for retry attempts
357445
.POST(HttpRequest.BodyPublishers.ofString(transaction.toJson()));
358446

359447
if (requestTimeout != null) {

0 commit comments

Comments
 (0)