Skip to content

Commit 953d4b5

Browse files
oschwaldclaude
andcommitted
amend! 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 9ba81a5 commit 953d4b5

1 file changed

Lines changed: 17 additions & 37 deletions

File tree

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

Lines changed: 17 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,20 @@
1616
import com.maxmind.minfraud.response.ScoreResponse;
1717
import java.io.IOException;
1818
import java.io.InputStream;
19-
import java.net.ConnectException;
19+
import java.io.InterruptedIOException;
2020
import java.net.ProxySelector;
2121
import java.net.URI;
2222
import java.net.URISyntaxException;
2323
import java.net.http.HttpClient;
24-
import java.net.http.HttpConnectTimeoutException;
2524
import java.net.http.HttpRequest;
2625
import java.net.http.HttpResponse;
2726
import java.net.http.HttpTimeoutException;
28-
import java.nio.channels.ClosedChannelException;
2927
import java.nio.charset.StandardCharsets;
3028
import java.time.Duration;
3129
import java.util.Base64;
3230
import java.util.Collections;
3331
import java.util.HashMap;
3432
import java.util.List;
35-
import java.util.Locale;
3633
import java.util.Map;
3734

3835
/**
@@ -405,40 +402,23 @@ private static boolean isRetriableTransportFailure(IOException e) {
405402
if (Thread.currentThread().isInterrupted()) {
406403
return false;
407404
}
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();
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;
440415
}
441-
return false;
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;
442422
}
443423

444424
private HttpRequest requestFor(AbstractModel transaction, URI uri)

0 commit comments

Comments
 (0)