Use 'GeoIP'/'GeoLite' branding in documentation and prose

MaxMind no longer ships the legacy products, so refer to the products as
'GeoIP' and 'GeoLite' rather than 'GeoIP2'/'GeoLite2' in prose. Technical
identifiers (packages, class names, filenames, edition IDs, hostnames,
URLs) are unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: oschwald

CLAUDE.md

@@ -427,7 +427,7 @@ String json = response.toJson();  // Pretty-printed JSON output
 
 The project depends on:
 - **Jackson** (core, databind, annotations, datatype-jsr310): JSON handling
-- **GeoIP2 Java API**: Sibling library providing GeoIP2 models used in responses
+- **GeoIP Java API**: Sibling library providing GeoIP models used in responses
 - **JUnit 5** (Jupiter): Testing framework
 - **WireMock**: HTTP mocking for tests
 - **jsonassert**: JSON comparison in tests

review.md

@@ -0,0 +1,62 @@
+# PR #619 Review Summary — `greg/stf-322`: bounded transport-failure retry
+
+## Critical Issues (must address before merge)
+
+1. **Worst-case timeout formula is incorrect** *(code-reviewer + comment-analyzer agree)*
+   - `WebServiceClient.java:128-132`, `:188-193`
+   - Javadoc states `(maxRetries + 1) x (connectTimeout + requestTimeout)`. But `requestTimeout` already covers the entire send (connect + send + receive); the two don't sum. Per-attempt is bounded by `max(connectTimeout, requestTimeout)`, total is `(maxRetries + 1) x requestTimeout` when both set.
+   - **Fix:** Reword to `(maxRetries + 1) x requestTimeout`, or describe each attempt as bounded by the configured timeouts independently.
+
+2. **`isRetriableTransportFailure()` retry deny-list is overly broad** *(silent-failure-hunter)*
+   - `WebServiceClient.java:401-422`
+   - Default-true means `UnknownHostException`, `SSLHandshakeException`, `SSLPeerUnverifiedException`, and `ConnectException` (refused) are all retried — but these are typically *deterministic* failures. Retrying just delays surfacing real config bugs.
+   - **Fix:** Either (a) flip to an allow-list (`SocketException`, `EOFException`, generic `IOException`), or (b) add explicit exclusions for `UnknownHostException` and `SSLHandshakeException`.
+
+## Important Issues (should address)
+
+3. **README missing the third exclusion** *(comment-analyzer)*
+   - `README.md:118-127` lists two exceptions; the code also short-circuits when `Thread.currentThread().isInterrupted()` is true regardless of exception type. Add a bullet.
+
+4. **No test for retry exhaustion or `maxRetries >= 2`** *(pr-test-analyzer)*
+   - The loop's terminating branch (`attempts >= maxRetries → throw`) at `WebServiceClient.java:391-394` is never exercised. A regression flipping `>=` to `>` (infinite loop) would not be caught.
+   - **Fix:** Add `.maxRetries(2)` + three faults → `assertThrows` + `wireMock.verify(3, ...)`.
+
+5. **`testInterruptDuringRetry` doesn't verify "no retry"** *(pr-test-analyzer)*
+   - `WebServiceClientTest.java:574-593` — add `wireMock.verify(1, postRequestedFor(...))`. Without it, the test would still pass if the interrupt-short-circuit branch were deleted.
+
+6. **Intermediate retry failures are silently dropped** *(silent-failure-hunter)*
+   - `WebServiceClient.java:386-397` — when retries fail and the final attempt throws, prior `IOException` instances are discarded.
+   - **Fix:** `e.addSuppressed(prior)` on rethrow so the final stack trace carries the full retry history. Project guidance ("do not ignore unexpected errors") supports this.
+
+7. **Pre-existing typo bundled in but not fixed** *(code-reviewer)*
+   - `WebServiceClient.java:187`: "Request timeout duration. **here** is no timeout by default." — should be "There". Worth fixing while editing this Javadoc anyway.
+
+8. **Interrupt-flag restoration belongs in a separate commit** *(code-reviewer)*
+   - `WebServiceClient.java:355, 375` — adding `Thread.currentThread().interrupt()` to existing `catch (InterruptedException)` blocks is an independent, legitimate bug fix. Per project hygiene, separate it from the retry feature.
+
+## Suggestions (nice to have)
+
+9. **Negative `maxRetries(-1)` validation is untested** — one-line `assertThrows(IllegalArgumentException.class, ...)`.
+10. **Tighten `assertThrows(Exception.class, ...)`** in `testNoRetryOnHttpTimeoutException`, `testMaxRetriesZeroDisablesRetry`, `testInterruptDuringRetry` — assert specific exception types.
+11. **Move retry-budget Javadoc** off `@param val` blocks into the method-level body — currently the same paragraph is duplicated on `connectTimeout()` and `requestTimeout()`.
+12. **Trim CHANGELOG entry** — currently duplicates README; one sentence + link is sufficient.
+13. **`maxRetries` Javadoc lists "connect timeout" as retriable** *(comment-analyzer)* — but it isn't (`HttpConnectTimeoutException extends HttpTimeoutException`). Drop from example list.
+14. **Soften phrasing**: "HTTP 4xx and 5xx responses cannot reach this predicate" → "do not reach" (same meaning, less hostage-to-fortune).
+15. **Confirm intentional default of `maxRetries = 1`** — silently changes upgrade behavior. Documented, but worth a sanity check that the team wants opt-out rather than opt-in.
+
+## Strengths
+
+- Builder API design follows existing conventions (`int val`, `IllegalArgumentException`, fluent return).
+- Thread-safety preserved: `final int`, local `attempts` counter, no shared state.
+- Classification logic catches the `HttpConnectTimeoutException extends HttpTimeoutException` JDK contract correctly with a single `instanceof` check.
+- The `BodyPublishers.ofString()` replayability comment is accurate (verified against JDK source).
+- `wireMock.verify(N, ...)` calls in 5 of 7 tests provide regression-resistant attempt-count assertions.
+- README is unusually clear — names actual JDK error messages users see and points at `jdk.httpclient.keepalive.timeout` as a complementary tuning knob.
+- The block comment in `isRetriableTransportFailure` explaining *why* timeouts are excluded ("customer-set budgets that retrying would silently extend") captures intent code alone can't.
+
+## Recommended action sequence
+
+1. Fix #1, #2 (critical) — they're behavioral correctness issues.
+2. Address #3-#8 — most can land as fixup commits per the project's commit-hygiene rule, *except* #8 itself which should be split out.
+3. Consider suggestions #9-#15 in the same fixup batch.
+4. Re-run `mvn test` and `mvn checkstyle:check`; re-request review on the deny-list-vs-allow-list discussion (#2) since it's a design judgment call rather than a clear-cut bug.

src/main/java/com/maxmind/minfraud/response/GeoIp2Location.java

@@ -6,7 +6,7 @@
 import java.time.ZonedDateTime;
 
 /**
- * This class contains minFraud response data related to the GeoIP2 Insights location.
+ * This class contains minFraud response data related to the GeoIP Insights location.
  *
  * @param accuracyRadius    The approximate accuracy radius in kilometers around the latitude and
  *                          longitude for the geographical entity (country, subdivision, city or

src/test/java/com/maxmind/minfraud/WebServiceClientTest.java

@@ -102,7 +102,7 @@ public void testFullInsightsTransaction() throws Exception {
         var response = client.insights(request);
 
         // We use non-strict checking as there is some extra stuff in the serialized
-        // object, most notably the "name" field in the GeoIP2 InsightsResponse subobjects.
+        // object, most notably the "name" field in the GeoIP InsightsResponse subobjects.
         // We cannot change this as it would be a breaking change to the GeoIP2 API.
         JSONAssert.assertEquals(responseContent, response.toJson(), false);
         verifyRequestFor(wireMock, "insights", "full-request");
@@ -153,7 +153,7 @@ public void testFullFactorsTransaction() throws Exception {
         var response = client.factors(request);
 
         // We use non-strict checking as there is some extra stuff in the serialized
-        // object, most notably the "name" field in the GeoIP2 InsightsResponse subobjects.
+        // object, most notably the "name" field in the GeoIP InsightsResponse subobjects.
         // We cannot change this as it would be a breaking change to the GeoIP2 API.
         JSONAssert.assertEquals(responseContent, response.toJson(), false);
         verifyRequestFor(wireMock, "factors", "full-request");