Add a per-request retry-on-exception hook to HxClient (lib-httpx 2.6.0) [release]

[pditommaso]

Why

HxConfig.retryCondition is a Predicate<Throwable>, so it answers the same way for every request. That is not enough for a caller with non-idempotent endpoints: a retry is safe when the request is idempotent or when it provably never reached the server, and only the first of those is visible in the request.

A bare HttpTimeoutException is the case that forces the distinction — it means the response never arrived, not that the request never did, so the same exception warrants opposite answers for a GET and a POST. A Predicate<Throwable> cannot express that on its own.

What

• Add HxClient.shouldRetryOnException(HttpRequest, Throwable) — the hook the retry policy now consults on both the sync and async send paths.
• The existing shouldRetryOnException(Throwable) becomes its default implementation, so current overrides and any configured HxConfig.retryCondition keep working unchanged.
• Document the pattern in the README, including how the response side already covers this through HttpResponse.request().

Compatibility

No API break. The new hook delegates to the old one by default, and a search across platform, wave, nextflow and sched found no consumer overriding shouldRetryOnException(Throwable), so the added indirection is invisible to existing callers.

Note that the 2-arg default calling into the overridable 1-arg method is load-bearing, not incidental: it is the mechanism by which a legacy override still takes effect. Restructuring it to bypass the 1-arg method via a private helper was tried and rejected — it makes should honour a shouldRetryOnException override from a subclass fail, because the retry sites call the 2-arg form. The flip side is that an override of the 1-arg method must not delegate back to the 2-arg form, which would recurse; that is now called out in the 1-arg javadoc.

Release

Bumps lib-httpx to 2.6.0 — VERSION, changelog.txt and the README dependency coordinate are updated together.

Test plan

./gradlew :lib-httpx:test passes — 22 specs in HxClientRetryIntegrationTest. Four are new:

1. Timed-out GET is re-sent — a subclass override retries a bare HttpTimeoutException that the default condition gives up on: 3 attempts where the default yields 1.
2. Reset POST is not re-sent — the same subclass refuses a CONNECTION_RESET_BY_PEER that the default condition would retry: 1 attempt where the default yields 3.
3. Async path reaches the hook — the same refusal holds through sendAsync, confirming both Retryable construction sites were rewired.
4. Composition with a configured retryCondition — a subclass that vetoes POSTs but defers to super for everything else, on top of a retryCondition that retries request timeouts. The GET half proves super reaches the configured condition (3 attempts; the default condition alone would stop at 1), the POST half proves the per-request veto still wins.

Backward-compatible delegation of an existing 1-arg override is covered by the pre-existing should honour a shouldRetryOnException override from a subclass spec, which is unchanged and still passes.

🤖 Generated with Claude Code

[pditommaso]

@claude check and review this PR

[claude]

Claude finished @pditommaso's task in 2m 13s —— View job

---

Review: per-request retry-on-exception hook

• Gather context (read diff + surrounding code)
• Review HxClient retry hook changes
• Review tests, README, version/changelog
• Post review

Verdict: looks good — no correctness, security or compatibility problem found. The change is additive and narrowly scoped: one new overload, both retry sites rewired to it, everything else untouched.

---

What I checked

Both retry paths are covered. Retryable is constructed in exactly two places in the module — sendWithRetry (HxClient.java:484) and sendWithRetryAsync (HxClient.java:557) — and both now pass t -> shouldRetryOnException(request, t). No third path was missed.

No shared-state hazard. The Retryable is built per call, so the captured request is per-invocation; concurrent sends on the same client can't cross-contaminate.

Backward compatibility holds. The default implementation delegates to shouldRetryOnException(Throwable) (HxClient.java:653), which still routes through config.getRetryCondition(). The pre-existing NeverRetryClient spec (HxClientRetryIntegrationTest.groovy:595) exercises exactly that delegation path and is unchanged. The only in-repo consumers of the old method are HxClientTest.groovy:42-45, which call the 1-arg form directly.

Overload resolution is clean. The old this::shouldRetryOnException method reference is gone, replaced by an explicit lambda — so the newly added 2-arg overload can't make the reference ambiguous. Arity differs anyway, but removing the method ref is the right defensive move.

Release hygiene is consistent. VERSION → 2.6.0, changelog.txt has a matching entry, and the README dependency coordinate is bumped in the same commit — matching the repo's release convention. lib-cloudinfo is the only in-repo dependent and needs no change since nothing is removed.

Docs. The javadoc and README both call out the one real footgun: overriding the new 2-arg hook bypasses a configured retryCondition unless the subclass delegates. The README's wrapper recipe even documents the tokenStore caveat with the 3-arg constructor — good catch, that's the kind of thing that usually bites someone later.

---

Non-blocking notes

1. The PR description's test plan doesn't match the tests. It says the three specs cover "the per-request decision, the default delegation, and the interaction with a configured retryCondition." What's actually added is: timed-out GET retried, reset POST not retried, and the async path. Default delegation is covered by the pre-existing spec, and no new spec asserts the new hook composing with a configured retryCondition. Worth either fixing the description or adding the third case — a subclass whose 2-arg override calls super on top of a custom retryCondition would pin down the composition that the javadoc promises.

2. Recursion footgun, docs-only. A subclass that overrides shouldRetryOnException(Throwable) by delegating to shouldRetryOnException(request, throwable) will recurse infinitely, since the default 2-arg impl calls straight back into the 1-arg. Unlikely, but a one-line "do not delegate the other way" in the 1-arg javadoc would close it.

3. The hook sees the pre-auth request. Correctly documented as such, and the right call — but it also means the 2-arg hook and shouldRetryOnResponse's HttpResponse.request() (which the README points at as the response-side equivalent) observe different request objects: the latter sees the actual sent request, post-Authorization and post-redirect. Not a bug, just an asymmetry a reader following the README from one paragraph to the next might not expect.

4. Timing-based spec. 'should let a subclass re-send a timed-out GET…' leans on a 150 ms request timeout against a 1000 ms fixed delay. That's the established pattern in this file, so it's consistent — flagging only as the usual CI-flake surface.

---

Not verified

I could not run ./gradlew :lib-httpx:test in this environment (the Gradle invocation wasn't permitted), so the "tests pass" claim rests on the PR author's run and CI, not on my own execution. I reviewed the new specs for compile-correctness instead — imports for Fault, ExecutionException, HttpTimeoutException and Duration are all already present at the top of the file, getHttpClient()/getConfig() are public (HxClient.java:732,736), and both HxClient constructors used are protected, which a subclass can reach via super(...) from any package.

I'm not able to submit a formal GitHub approval — this action posts review feedback as a comment only. Treat the above as an approving review; see the FAQ for the formal-review workaround.
• branch feat/per-request-retry-condition