Skip to content

Commit e7f2b7a

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 d0dcb2e commit e7f2b7a

1 file changed

Lines changed: 97 additions & 2 deletions

File tree

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

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,20 +20,26 @@
2020
import com.maxmind.geoip2.model.InsightsResponse;
2121
import java.io.IOException;
2222
import java.io.InputStream;
23+
import java.io.InterruptedIOException;
24+
import java.net.ConnectException;
2325
import java.net.InetAddress;
2426
import java.net.InetSocketAddress;
2527
import java.net.ProxySelector;
2628
import java.net.URI;
2729
import java.net.URISyntaxException;
30+
import java.net.UnknownHostException;
2831
import java.net.http.HttpClient;
2932
import java.net.http.HttpRequest;
3033
import java.net.http.HttpResponse;
34+
import java.net.http.HttpTimeoutException;
3135
import java.nio.charset.StandardCharsets;
3236
import java.time.Duration;
3337
import java.util.Base64;
3438
import java.util.HashMap;
3539
import java.util.List;
3640
import java.util.Map;
41+
import javax.net.ssl.SSLHandshakeException;
42+
import javax.net.ssl.SSLPeerUnverifiedException;
3743

3844
/**
3945
* <p>
@@ -112,6 +118,7 @@ public class WebServiceClient implements WebServiceProvider {
112118
private final boolean useHttps;
113119
private final int port;
114120
private final Duration requestTimeout;
121+
private final int maxRetries;
115122
private final String userAgent = "GeoIP2/"
116123
+ getClass().getPackage().getImplementationVersion()
117124
+ " (Java/" + System.getProperty("java.version") + ")";
@@ -125,6 +132,7 @@ private WebServiceClient(Builder builder) {
125132
this.port = builder.port;
126133
this.useHttps = builder.useHttps;
127134
this.locales = builder.locales;
135+
this.maxRetries = builder.maxRetries;
128136

129137
// HttpClient supports basic auth, but it will only send it after the
130138
// server responds with an unauthorized. As such, we just make the
@@ -182,6 +190,7 @@ public static final class Builder {
182190
List<String> locales = List.of("en");
183191
private ProxySelector proxy = null;
184192
private HttpClient httpClient = null;
193+
private int maxRetries = 1;
185194

186195
/**
187196
* @param accountId Your MaxMind account ID.
@@ -197,6 +206,7 @@ public Builder(int accountId, String licenseKey) {
197206
* @param val Timeout duration to establish a connection to the
198207
* web service. The default is 3 seconds.
199208
* @return Builder object
209+
* @apiNote See {@link #maxRetries(int)} for how this timeout interacts with retries.
200210
*/
201211
public Builder connectTimeout(Duration val) {
202212
this.connectTimeout = val;
@@ -251,6 +261,7 @@ public Builder locales(List<String> val) {
251261
/**
252262
* @param val Request timeout duration. The default is 20 seconds.
253263
* @return Builder object
264+
* @apiNote See {@link #maxRetries(int)} for how this timeout interacts with retries.
254265
*/
255266
public Builder requestTimeout(Duration val) {
256267
this.requestTimeout = val;
@@ -271,13 +282,42 @@ public Builder proxy(ProxySelector val) {
271282
* @param val the custom HttpClient to use for requests. When providing a
272283
* custom HttpClient, you cannot also set connectTimeout or proxy
273284
* parameters as these should be configured on the provided client.
285+
* <p>
286+
* The SDK applies its own transport-failure retry on top of any
287+
* supplied client; customers can disable it via
288+
* {@link #maxRetries(int)} with {@code .maxRetries(0)}.
274289
* @return Builder object
275290
*/
276291
public Builder httpClient(HttpClient val) {
277292
this.httpClient = val;
278293
return this;
279294
}
280295

296+
/**
297+
* @param val Maximum number of retries on transport-level failures
298+
* (connection reset, broken pipe, EOF, ...).
299+
* Applies uniformly to all endpoints. Defaults to 1.
300+
* Set to 0 to disable.
301+
* @return Builder.
302+
* @throws IllegalArgumentException if {@code val} is negative.
303+
* @apiNote Timeouts are not retried ({@link java.net.http.HttpTimeoutException},
304+
* including the connect-phase subclass
305+
* {@link java.net.http.HttpConnectTimeoutException}). When
306+
* {@code maxRetries > 0}, retries are triggered only by fast transport
307+
* failures, so each attempt is independently bounded by
308+
* {@link #connectTimeout(Duration)} and {@link #requestTimeout(Duration)}.
309+
* The multiplied worst-case wall clock a naive reading suggests is
310+
* unreachable in practice, since hitting the timeout aborts the call
311+
* rather than triggering a retry.
312+
*/
313+
public Builder maxRetries(int val) {
314+
if (val < 0) {
315+
throw new IllegalArgumentException("maxRetries must not be negative");
316+
}
317+
maxRetries = val;
318+
return this;
319+
}
320+
281321
/**
282322
* @return an instance of {@code WebServiceClient} created from the
283323
* fields set on this builder.
@@ -371,8 +411,7 @@ private <T> T responseFor(String path, InetAddress ipAddress, Class<T> cls)
371411
.GET()
372412
.build();
373413
try {
374-
var response = this.httpClient
375-
.send(request, HttpResponse.BodyHandlers.ofInputStream());
414+
var response = sendWithRetry(request);
376415
try {
377416
return handleResponse(response, cls);
378417
} finally {
@@ -383,6 +422,62 @@ private <T> T responseFor(String path, InetAddress ipAddress, Class<T> cls)
383422
}
384423
}
385424

425+
private HttpResponse<InputStream> sendWithRetry(HttpRequest request)
426+
throws IOException, InterruptedException {
427+
int attempts = 0;
428+
IOException prior = null;
429+
while (true) {
430+
try {
431+
return httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream());
432+
} catch (IOException e) {
433+
if (prior != null) {
434+
e.addSuppressed(prior);
435+
}
436+
if (!isRetriableTransportFailure(e) || attempts >= maxRetries) {
437+
throw e;
438+
}
439+
prior = e;
440+
attempts++;
441+
}
442+
}
443+
}
444+
445+
private static boolean isRetriableTransportFailure(IOException e) {
446+
if (Thread.currentThread().isInterrupted()) {
447+
return false;
448+
}
449+
// Both connect-phase and request-phase timeouts are customer-set
450+
// budgets that retrying would silently extend.
451+
// HttpConnectTimeoutException extends HttpTimeoutException, so this
452+
// single check covers both.
453+
if (e instanceof HttpTimeoutException) {
454+
return false;
455+
}
456+
// The thread was interrupted during I/O; honor the cancellation.
457+
if (e instanceof InterruptedIOException) {
458+
return false;
459+
}
460+
// Typically deterministic failures: retrying just delays surfacing the
461+
// config bug without recovering the request.
462+
if (e instanceof UnknownHostException) {
463+
return false;
464+
}
465+
if (e instanceof ConnectException) {
466+
return false;
467+
}
468+
if (e instanceof SSLHandshakeException) {
469+
return false;
470+
}
471+
if (e instanceof SSLPeerUnverifiedException) {
472+
return false;
473+
}
474+
// Everything else from httpClient.send() is a transport failure
475+
// (connection reset, broken pipe, EOF, closed channel, ...).
476+
// HTTP 4xx and 5xx responses do not reach this predicate -- they come
477+
// back as HttpResponse objects rather than IOExceptions.
478+
return true;
479+
}
480+
386481
private <T> T handleResponse(HttpResponse<InputStream> response, Class<T> cls)
387482
throws GeoIp2Exception, IOException {
388483
var status = response.statusCode();

0 commit comments

Comments
 (0)