STF-322: Bounded transport-failure retry in WebServiceClient

[oschwald]

Summary

• Adds WebServiceClient.Builder.maxRetries(int) (defaults to 1) so a single transient transport failure (Connection reset, Broken pipe, EOF, closed channel, etc.) is retried transparently.
• Predicate is permissive by exclusion: any IOException from httpClient.send() is retried EXCEPT HttpTimeoutException (which covers both request-phase and connect-phase timeouts, since HttpConnectTimeoutException extends it) and InterruptedIOException. Both timeouts are customer-set budgets that retrying would silently extend; InterruptedIOException is a user-cancellation signal we honor. HTTP 4xx/5xx come back as HttpResponse objects (not IOExceptions) and are surfaced through the existing exception hierarchy, so the predicate is structurally unable to retry them.
• Restores the interrupt flag in the InterruptedException rewrap path. Updates README and CHANGELOG; opt out via .maxRetries(0).
• Adds a mise.toml / mise.lock to pin the Java + Maven versions used locally, mirroring minfraud-api-java.

Fixes STF-322. Parallel PR for minfraud-api-java uses a byte-identical predicate (maxmind/minfraud-api-java#619).

Test plan

• mvn checkstyle:check clean
• mvn verify -Dgpg.skip=true — 102/102 pass (28 existing + 9 new WebServiceClientTest cases)
• Reviewer confirms README guidance points customers at jdk.httpclient.keepalive.timeout as the complementary workaround

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a configurable retry mechanism for transport-level failures in the WebServiceClient, defaulting to one retry for issues like connection resets or broken pipes. The implementation includes updates to the Builder for maxRetries configuration, detailed documentation in the README, and comprehensive test coverage for various failure scenarios. Feedback suggests refactoring the retry loop to prevent potential integer overflow and adopting case-insensitive string matching for exception messages to improve robustness across different environments.

[oschwald]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a configurable retry mechanism for transport-level failures in the WebServiceClient. It adds a maxRetries option to the builder (defaulting to 1), implements logic to retry on specific exceptions like connection resets and connect timeouts, and includes comprehensive tests to verify the behavior. Additionally, it adds tool configuration files for mise and updates the documentation and changelog. I have no feedback to provide as the changes are well-implemented and the existing review comments were purely evaluative.

[oschwald]

@gemini-code-assist review

[gemini-code-assist]

Code Review

This pull request introduces a transport-failure retry mechanism to the WebServiceClient, defaulting to one retry for transient errors like connection resets or broken pipes. It includes updates to the Builder for configuring maxRetries, expanded documentation in the README and CHANGELOG, and new test cases. Review feedback identifies a discrepancy in the retry logic regarding HttpConnectTimeoutException, which is currently excluded because it inherits from HttpTimeoutException. Additionally, a test case for connection timeouts was found to be misnamed and lacks verification that retries are actually being attempted.

I am having trouble creating individual review comments. Click here to see my feedback.

src/main/java/com/maxmind/geoip2/WebServiceClient.java (446-448)

There is a discrepancy between the code and the PR summary regarding HttpConnectTimeoutException. The summary states that connect timeouts are retried, but since HttpConnectTimeoutException extends HttpTimeoutException, it is currently excluded by this check. Additionally, the summary describes a "narrow" predicate that "walks the cause chain," whereas the implementation is broad, retrying all IOExceptions except for timeouts and interruptions. If the intention is to exclude all timeouts (as suggested by the README and CHANGELOG), the PR summary should be corrected. If connect timeouts should be retried, this logic needs to be adjusted to specifically allow them while still excluding request-phase timeouts.

src/test/java/com/maxmind/geoip2/WebServiceClientTest.java (590-608)

The test testRetriesOnConnectTimeout is misnamed and lacks verification of the retry logic. It triggers a ConnectException (connection refused) by connecting to a closed port, which is distinct from a "Connect Timeout" (HttpConnectTimeoutException). Furthermore, the test does not verify that a retry actually occurred (e.g., by checking the number of attempts). Consider renaming the test to reflect that it tests connection refusal and adding a verification step (such as using a mock or checking WireMock logs if applicable) to ensure the retry mechanism is triggered as expected.