diff --git a/CHANGELOG.md b/CHANGELOG.md index 2353accc..1fabc554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ CHANGELOG ========= +5.0.3 (unreleased) +------------------ + +* Added `WebServiceClient.Builder.maxRetries(int)` to configure transport-failure + retry behavior. Defaults to 1 (one retry on connection reset, broken pipe, + or connect timeout). Set to 0 to disable. Request-phase timeouts and HTTP + 4xx/5xx responses are never retried. + 5.0.2 (2025-12-08) ------------------ diff --git a/README.md b/README.md index 5d46a084..b16a3103 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,41 @@ are not created for each request. See the [API documentation](https://maxmind.github.io/GeoIP2-java/) for more details. +### Connection pooling and transport retries ### + +`WebServiceClient` is thread-safe and reuses a pooled `HttpClient` across +requests. Idle connections in the pool can be silently closed by load +balancers or other intermediaries. When the next request reuses one of these +half-closed connections, the JDK reports the failure as a `Connection reset` +(or `Broken pipe`) `IOException`. + +To smooth over these intermittent transport failures, the SDK retries once by +default. The retry covers: + +* `SocketException` with message `Connection reset` or `Broken pipe`, +* `ConnectException`, +* `HttpConnectTimeoutException`. + +Retries are **not** applied to request-phase timeouts (`HttpTimeoutException`) +or to HTTP 4xx / 5xx responses. Web service requests are idempotent GETs, so +retried requests are byte-identical to the original. + +You can change the retry budget via the builder: + +```java +WebServiceClient client = new WebServiceClient.Builder(42, "license_key") + .maxRetries(2) // up to two retries (three total attempts) + .build(); +``` + +Set `.maxRetries(0)` to disable the retry entirely. Negative values throw +`IllegalArgumentException`. + +If you frequently see `Connection reset` errors, you can also reduce the +JDK's keep-alive timeout via the system property +`jdk.httpclient.keepalive.timeout` (in seconds) to evict pooled connections +before any intermediary does so. + ## Web Service Example ## ### Country Service ### diff --git a/mise.lock b/mise.lock new file mode 100644 index 00000000..0145b93f --- /dev/null +++ b/mise.lock @@ -0,0 +1,30 @@ +# @generated - this file is auto-generated by `mise lock` https://mise.jdx.dev/dev-tools/mise-lock.html + +[[tools.java]] +version = "26.0.0" +backend = "core:java" + +[[tools.maven]] +version = "3.9.15" +backend = "aqua:apache/maven" + +[tools.maven."platforms.linux-arm64"] +url = "https://archive.apache.org/dist/maven/maven-3/3.9.15/binaries/apache-maven-3.9.15-bin.tar.gz" + +[tools.maven."platforms.linux-arm64-musl"] +url = "https://archive.apache.org/dist/maven/maven-3/3.9.15/binaries/apache-maven-3.9.15-bin.tar.gz" + +[tools.maven."platforms.linux-x64"] +url = "https://archive.apache.org/dist/maven/maven-3/3.9.15/binaries/apache-maven-3.9.15-bin.tar.gz" + +[tools.maven."platforms.linux-x64-musl"] +url = "https://archive.apache.org/dist/maven/maven-3/3.9.15/binaries/apache-maven-3.9.15-bin.tar.gz" + +[tools.maven."platforms.macos-arm64"] +url = "https://archive.apache.org/dist/maven/maven-3/3.9.15/binaries/apache-maven-3.9.15-bin.tar.gz" + +[tools.maven."platforms.macos-x64"] +url = "https://archive.apache.org/dist/maven/maven-3/3.9.15/binaries/apache-maven-3.9.15-bin.tar.gz" + +[tools.maven."platforms.windows-x64"] +url = "https://archive.apache.org/dist/maven/maven-3/3.9.15/binaries/apache-maven-3.9.15-bin.tar.gz" diff --git a/mise.toml b/mise.toml new file mode 100644 index 00000000..c4027682 --- /dev/null +++ b/mise.toml @@ -0,0 +1,18 @@ +[settings] +experimental = true +lockfile = true +disable_backends = [ + "asdf", + "vfox", +] + +[tools] +java = "latest" +maven = "latest" + +[hooks] +enter = "mise install --quiet --locked" + +[[watch_files]] +patterns = ["mise.toml", "mise.lock"] +run = "mise install --quiet --locked" diff --git a/src/main/java/com/maxmind/geoip2/WebServiceClient.java b/src/main/java/com/maxmind/geoip2/WebServiceClient.java index 125a3af9..1d15b836 100644 --- a/src/main/java/com/maxmind/geoip2/WebServiceClient.java +++ b/src/main/java/com/maxmind/geoip2/WebServiceClient.java @@ -20,14 +20,17 @@ import com.maxmind.geoip2.model.InsightsResponse; import java.io.IOException; import java.io.InputStream; +import java.net.ConnectException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.ProxySelector; import java.net.URI; import java.net.URISyntaxException; import java.net.http.HttpClient; +import java.net.http.HttpConnectTimeoutException; import java.net.http.HttpRequest; import java.net.http.HttpResponse; +import java.net.http.HttpTimeoutException; import java.nio.charset.StandardCharsets; import java.time.Duration; import java.util.Base64; @@ -112,6 +115,7 @@ public class WebServiceClient implements WebServiceProvider { private final boolean useHttps; private final int port; private final Duration requestTimeout; + private final int maxRetries; private final String userAgent = "GeoIP2/" + getClass().getPackage().getImplementationVersion() + " (Java/" + System.getProperty("java.version") + ")"; @@ -125,6 +129,7 @@ private WebServiceClient(Builder builder) { this.port = builder.port; this.useHttps = builder.useHttps; this.locales = builder.locales; + this.maxRetries = builder.maxRetries; // HttpClient supports basic auth, but it will only send it after the // server responds with an unauthorized. As such, we just make the @@ -182,6 +187,7 @@ public static final class Builder { List locales = List.of("en"); private ProxySelector proxy = null; private HttpClient httpClient = null; + private int maxRetries = 1; /** * @param accountId Your MaxMind account ID. @@ -196,6 +202,12 @@ public Builder(int accountId, String licenseKey) { /** * @param val Timeout duration to establish a connection to the * web service. The default is 3 seconds. + *

+ * When {@code maxRetries > 0}, one API call may incur up to + * {@code (maxRetries + 1)} connection attempts, each subject to + * {@code connectTimeout} and {@code requestTimeout}. Worst-case + * wall-clock duration is roughly + * {@code (maxRetries + 1) x (connectTimeout + requestTimeout)}. * @return Builder object */ public Builder connectTimeout(Duration val) { @@ -250,6 +262,12 @@ public Builder locales(List val) { /** * @param val Request timeout duration. The default is 20 seconds. + *

+ * When {@code maxRetries > 0}, one API call may incur up to + * {@code (maxRetries + 1)} connection attempts, each subject to + * {@code connectTimeout} and {@code requestTimeout}. Worst-case + * wall-clock duration is roughly + * {@code (maxRetries + 1) x (connectTimeout + requestTimeout)}. * @return Builder object */ public Builder requestTimeout(Duration val) { @@ -271,6 +289,10 @@ public Builder proxy(ProxySelector val) { * @param val the custom HttpClient to use for requests. When providing a * custom HttpClient, you cannot also set connectTimeout or proxy * parameters as these should be configured on the provided client. + *

+ * The SDK applies its own transport-failure retry on top of any + * supplied client; customers can disable it via + * {@link #maxRetries(int)} with {@code .maxRetries(0)}. * @return Builder object */ public Builder httpClient(HttpClient val) { @@ -278,6 +300,22 @@ public Builder httpClient(HttpClient val) { return this; } + /** + * @param val Maximum number of retries on transport-level failures + * (connection reset, broken pipe, connect timeout, ...). + * Applies uniformly to all endpoints. Defaults to 1. + * Set to 0 to disable. + * @return Builder. + * @throws IllegalArgumentException if {@code val} is negative. + */ + public Builder maxRetries(int val) { + if (val < 0) { + throw new IllegalArgumentException("maxRetries must not be negative"); + } + maxRetries = val; + return this; + } + /** * @return an instance of {@code WebServiceClient} created from the * fields set on this builder. @@ -371,18 +409,68 @@ private T responseFor(String path, InetAddress ipAddress, Class cls) .GET() .build(); try { - var response = this.httpClient - .send(request, HttpResponse.BodyHandlers.ofInputStream()); + var response = sendWithRetry(request); try { return handleResponse(response, cls); } finally { response.body().close(); } } catch (InterruptedException e) { + Thread.currentThread().interrupt(); throw new GeoIp2Exception("Interrupted sending request", e); } } + private HttpResponse sendWithRetry(HttpRequest request) + throws IOException, InterruptedException { + IOException lastException = null; + int attempts = maxRetries + 1; + for (int i = 0; i < attempts; i++) { + try { + return httpClient.send(request, HttpResponse.BodyHandlers.ofInputStream()); + } catch (IOException e) { + if (!isRetriableTransportFailure(e) || i == attempts - 1) { + throw e; + } + lastException = e; + } + } + // Unreachable: loop either returns or throws. + throw lastException; + } + + private static boolean isRetriableTransportFailure(IOException e) { + if (Thread.currentThread().isInterrupted()) { + return false; + } + // Walk the cause chain: the JDK HttpClient wraps the underlying transport + // failure in different ways depending on the protocol path. Over HTTP/1.1 + // a "Connection reset" surfaces as a SocketException; over HTTP/2 (e.g. + // a SETTINGS-frame write failure) it may surface as a plain IOException + // with the same message. Match by message regardless of class to handle + // both, while keeping the type checks for connect-phase timeouts and + // request-phase timeouts (which must NEVER be retried). + Throwable t = e; + while (t != null) { + if (t instanceof HttpConnectTimeoutException) { + return true; // subclass of HttpTimeoutException - must be checked first + } + if (t instanceof HttpTimeoutException) { + return false; // request-phase timeout: NOT retriable + } + if (t instanceof ConnectException) { + return true; + } + String msg = t.getMessage(); + if (msg != null + && (msg.contains("Connection reset") || msg.contains("Broken pipe"))) { + return true; + } + t = t.getCause(); + } + return false; + } + private T handleResponse(HttpResponse response, Class cls) throws GeoIp2Exception, IOException { var status = response.statusCode(); diff --git a/src/test/java/com/maxmind/geoip2/WebServiceClientTest.java b/src/test/java/com/maxmind/geoip2/WebServiceClientTest.java index cd5c2af0..efbac8f3 100644 --- a/src/test/java/com/maxmind/geoip2/WebServiceClientTest.java +++ b/src/test/java/com/maxmind/geoip2/WebServiceClientTest.java @@ -3,6 +3,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.equalTo; import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.client.WireMock.getRequestedFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static com.jcabi.matchers.RegexMatchers.matchesPattern; @@ -15,8 +16,10 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.github.tomakehurst.wiremock.http.Fault; import com.github.tomakehurst.wiremock.junit5.WireMockExtension; import com.github.tomakehurst.wiremock.junit5.WireMockTest; +import com.github.tomakehurst.wiremock.stubbing.Scenario; import com.maxmind.geoip2.exception.AddressNotFoundException; import com.maxmind.geoip2.exception.AuthenticationException; import com.maxmind.geoip2.exception.GeoIp2Exception; @@ -24,6 +27,8 @@ import com.maxmind.geoip2.exception.InvalidRequestException; import com.maxmind.geoip2.exception.OutOfQueriesException; import com.maxmind.geoip2.exception.PermissionRequiredException; +import com.maxmind.geoip2.model.CityResponse; +import com.maxmind.geoip2.model.CountryResponse; import com.maxmind.geoip2.model.InsightsResponse; import com.maxmind.geoip2.record.City; import com.maxmind.geoip2.record.Continent; @@ -37,6 +42,7 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.ProxySelector; +import java.net.ServerSocket; import java.net.http.HttpClient; import java.nio.charset.StandardCharsets; import java.time.Duration; @@ -460,4 +466,233 @@ public void testHttpClientWithDefaultSettingsDoesNotThrow() throws Exception { assertNotNull(client); } + @Test + public void testRetriesOnConnectionReset_country() throws Exception { + String url = "/geoip/v2.1/country/1.2.3.4"; + String body = "{\"traits\":{\"ip_address\":\"1.2.3.4\"}}"; + + wireMock.stubFor(get(urlEqualTo(url)) + .inScenario("retry-country") + .whenScenarioStateIs(Scenario.STARTED) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER)) + .willSetStateTo("succeeded")); + + wireMock.stubFor(get(urlEqualTo(url)) + .inScenario("retry-country") + .whenScenarioStateIs("succeeded") + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", + "application/vnd.maxmind.com-country+json; charset=UTF-8; version=2.1") + .withBody(body))); + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + CountryResponse response = client.country(InetAddress.getByName("1.2.3.4")); + assertNotNull(response); + + wireMock.verify(2, getRequestedFor(urlEqualTo(url))); + } + + @Test + public void testRetriesOnConnectionReset_city() throws Exception { + String url = "/geoip/v2.1/city/1.2.3.4"; + String body = "{\"traits\":{\"ip_address\":\"1.2.3.4\"}}"; + + wireMock.stubFor(get(urlEqualTo(url)) + .inScenario("retry-city") + .whenScenarioStateIs(Scenario.STARTED) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER)) + .willSetStateTo("succeeded")); + + wireMock.stubFor(get(urlEqualTo(url)) + .inScenario("retry-city") + .whenScenarioStateIs("succeeded") + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", + "application/vnd.maxmind.com-city+json; charset=UTF-8; version=2.1") + .withBody(body))); + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + CityResponse response = client.city(InetAddress.getByName("1.2.3.4")); + assertNotNull(response); + + wireMock.verify(2, getRequestedFor(urlEqualTo(url))); + } + + @Test + public void testRetriesOnConnectionReset_insights() throws Exception { + String url = "/geoip/v2.1/insights/1.2.3.4"; + String body = "{\"traits\":{\"ip_address\":\"1.2.3.4\"}}"; + + wireMock.stubFor(get(urlEqualTo(url)) + .inScenario("retry-insights") + .whenScenarioStateIs(Scenario.STARTED) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER)) + .willSetStateTo("succeeded")); + + wireMock.stubFor(get(urlEqualTo(url)) + .inScenario("retry-insights") + .whenScenarioStateIs("succeeded") + .willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", + "application/vnd.maxmind.com-insights+json; charset=UTF-8; version=2.1") + .withBody(body))); + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + InsightsResponse response = client.insights(InetAddress.getByName("1.2.3.4")); + assertNotNull(response); + + wireMock.verify(2, getRequestedFor(urlEqualTo(url))); + } + + @Test + public void testNoRetryOnHttpTimeoutException() { + String url = "/geoip/v2.1/insights/1.2.3.4"; + wireMock.stubFor(get(urlEqualTo(url)) + .willReturn(aResponse() + .withStatus(200) + .withFixedDelay(2000) + .withBody("{}"))); + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .requestTimeout(Duration.ofMillis(100)) + .build(); + + // The request-phase timeout surfaces as a checked exception; we just + // need to confirm it propagates (any throwable is acceptable here). + assertThrows(Exception.class, + () -> client.insights(InetAddress.getByName("1.2.3.4"))); + + wireMock.verify(1, getRequestedFor(urlEqualTo(url))); + } + + @Test + public void testRetriesOnConnectTimeout() throws Exception { + // Deterministic alternative to an unroutable address: bind to a free + // local port, then immediately close it. Connection attempts to the + // closed port fail fast with ConnectException, which exercises the + // same retry branch as a connect timeout (both are predicate hits). + int port; + try (ServerSocket socket = new ServerSocket(0)) { + port = socket.getLocalPort(); + } + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("127.0.0.1") + .port(port) + .disableHttps() + .build(); + + assertThrows(Exception.class, + () -> client.insights(InetAddress.getByName("1.2.3.4"))); + } + + @Test + public void testNoRetryOn5xx() { + String url = "/geoip/v2.1/insights/1.2.3.4"; + wireMock.stubFor(get(urlEqualTo(url)) + .willReturn(aResponse() + .withStatus(500) + .withHeader("Content-Type", "application/json") + .withBody(""))); + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + assertThrows(HttpException.class, + () -> client.insights(InetAddress.getByName("1.2.3.4"))); + + wireMock.verify(1, getRequestedFor(urlEqualTo(url))); + } + + @Test + public void testNoRetryOn4xx() { + String url = "/geoip/v2.1/insights/1.2.3.4"; + wireMock.stubFor(get(urlEqualTo(url)) + .willReturn(aResponse() + .withStatus(402) + .withHeader("Content-Type", "application/json") + .withBody("{\"code\":\"OUT_OF_QUERIES\",\"error\":\"out of credit\"}"))); + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + assertThrows(OutOfQueriesException.class, + () -> client.insights(InetAddress.getByName("1.2.3.4"))); + + wireMock.verify(1, getRequestedFor(urlEqualTo(url))); + } + + @Test + public void testMaxRetriesZeroDisablesRetry() { + String url = "/geoip/v2.1/insights/1.2.3.4"; + wireMock.stubFor(get(urlEqualTo(url)) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .maxRetries(0) + .build(); + + assertThrows(Exception.class, + () -> client.insights(InetAddress.getByName("1.2.3.4"))); + + wireMock.verify(1, getRequestedFor(urlEqualTo(url))); + } + + @Test + public void testInterruptDuringRetry() { + // Pre-interrupt the calling thread; the predicate short-circuits when + // the thread is interrupted, so no retry should occur and the + // InterruptedException path in the client should restore the flag. + String url = "/geoip/v2.1/insights/1.2.3.4"; + wireMock.stubFor(get(urlEqualTo(url)) + .willReturn(aResponse().withFault(Fault.CONNECTION_RESET_BY_PEER))); + + WebServiceClient client = new WebServiceClient.Builder(6, "0123456789") + .host("localhost") + .port(wireMock.getPort()) + .disableHttps() + .build(); + + Thread.currentThread().interrupt(); + try { + assertThrows(Exception.class, + () -> client.insights(InetAddress.getByName("1.2.3.4"))); + assertTrue(Thread.currentThread().isInterrupted(), + "interrupt flag should remain set after the call"); + } finally { + // Clear the interrupt flag so it does not leak to other tests. + Thread.interrupted(); + } + } + }