Add RFC 9110-aware idempotent HTTP client retry filter - #12640
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an opt-in, RFC 9110-aware HTTP client retry filter that retries only idempotent requests on transient failures, including Retry-After support, with accompanying configuration, docs, and tests.
Changes:
- Introduces
IdempotentRetryClientFilterplus request/response retry predicate extension points andRetry-Afterparsing. - Adds
HttpMethod.isSafe()/isIdempotent()and client retry configuration wiring. - Updates docs TOC + retry docs, and adds unit/integration/spec coverage and doc-example tests.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test-suite/src/test/java/io/micronaut/docs/http/client/retry/CustomResponseRetryPredicateSpec.java | Adds end-to-end doc-example test verifying a custom response retry predicate behavior. |
| test-suite/src/test/java/io/micronaut/docs/http/client/retry/CustomResponseRetryPredicate.java | Provides Java doc-example showing how to replace the default response retry predicate. |
| test-suite-kotlin/src/test/kotlin/io/micronaut/docs/http/client/retry/CustomResponseRetryPredicate.kt | Provides Kotlin doc-example equivalent of the custom response retry predicate. |
| src/main/docs/guide/toc.yml | Registers the new idempotent retry documentation section in the guide TOC. |
| src/main/docs/guide/httpClient/idempotentRetry.adoc | Documents the new retry filter, defaults, configuration, and timeout/mesh considerations. |
| src/main/docs/guide/httpClient/clientAnnotation/clientRetry.adoc | Reframes client retry docs to distinguish @Retryable vs the new transport-level filter. |
| http/src/test/groovy/io/micronaut/http/HttpMethodSpec.groovy | Adds coverage for HttpMethod.isSafe() and isIdempotent(). |
| http/src/main/java/io/micronaut/http/HttpMethod.java | Adds RFC 9110 semantics helpers: isSafe() and isIdempotent(). |
| http-client/src/test/groovy/io/micronaut/http/client/retry/IdempotentRetryIntegrationSpec.groovy | Adds EmbeddedServer integration tests for retry behavior across GET/POST and exhaustion. |
| http-client/src/test/groovy/io/micronaut/http/client/retry/IdempotentRetryClientFilterSpec.groovy | Adds unit tests for filter retry logic, Retry-After, bypass paths, and disablement. |
| http-client-core/src/test/groovy/io/micronaut/http/client/retry/RetryAfterParserSpec.groovy | Adds unit tests for parsing delta-seconds / HTTP-date Retry-After forms and edge cases. |
| http-client-core/src/test/groovy/io/micronaut/http/client/retry/HttpResponseRetryPredicateSpec.groovy | Adds unit tests for default retryable status matrix and predicate behavior. |
| http-client-core/src/main/java/io/micronaut/http/client/retry/RetryAfterParser.java | Implements internal Retry-After parsing per RFC 9110 §10.2.3. |
| http-client-core/src/main/java/io/micronaut/http/client/retry/IdempotentRetryClientFilter.java | Implements the opt-in retry filter with exponential backoff/jitter and Retry-After handling. |
| http-client-core/src/main/java/io/micronaut/http/client/retry/HttpResponseRetryPredicate.java | Adds public functional interface for response/exception-based retry decisions + defaults. |
| http-client-core/src/main/java/io/micronaut/http/client/retry/HttpRequestRetryPredicate.java | Adds public functional interface for request idempotency eligibility + defaults. |
| http-client-core/src/main/java/io/micronaut/http/client/retry/DefaultRetryPredicateFactory.java | Provides default predicate beans when users don’t supply replacements. |
| http-client-core/src/main/java/io/micronaut/http/client/HttpClientConfiguration.java | Adds nested RetryConfiguration and new accessor for retry config. |
| http-client-core/src/main/java/io/micronaut/http/client/DefaultHttpClientConfiguration.java | Wires default retry configuration into the default HTTP client configuration implementation. |
| /** | ||
| * Obtains the idempotent retry configuration. | ||
| * | ||
| * @return The retry configuration, or {@code null} if retry is not configured. | ||
| * @since 5.0.0 | ||
| */ | ||
| public HttpClientConfiguration.@Nullable RetryConfiguration getRetryConfiguration() { | ||
| return null; |
There was a problem hiding this comment.
The base HttpClientConfiguration#getRetryConfiguration() implementation always returns null, which makes the new API easy to misuse (callers must always null-check) and makes it impossible for callers to observe configured defaults unless they happen to have a concrete subtype override. Consider returning a non-null default RetryConfiguration instance from the base class (and letting configuration binding override it), aligning with other nested client config patterns and simplifying consumption.
| /** | |
| * Obtains the idempotent retry configuration. | |
| * | |
| * @return The retry configuration, or {@code null} if retry is not configured. | |
| * @since 5.0.0 | |
| */ | |
| public HttpClientConfiguration.@Nullable RetryConfiguration getRetryConfiguration() { | |
| return null; | |
| private final RetryConfiguration retryConfiguration = new RetryConfiguration(); | |
| /** | |
| * Obtains the idempotent retry configuration. | |
| * | |
| * @return The retry configuration. | |
| * @since 5.0.0 | |
| */ | |
| public HttpClientConfiguration.RetryConfiguration getRetryConfiguration() { | |
| return retryConfiguration; |
There was a problem hiding this comment.
Pushing back here. The null-at-base pattern is established for getHttp2Configuration()
and getWebSocketCompressionConfiguration() in this same file, and serves a specific
purpose: it lets user-defined HttpClientConfiguration subclasses (e.g.
ClientTwoHttpConfiguration in
http-client/src/test/groovy/io/micronaut/http/client/ClientSpecificLoggerSpec.groovy)
opt into individual nested configs without inheriting unrelated defaults. Switching
getRetryConfiguration() to a non-null base would silently inject a default-valued
RetryConfiguration into every existing custom subclass — that's an implicit behavior
change in user code, not an ergonomic win.
A second wrinkle specific to retry: this feature is disabled-by-default (intentional,
to keep zero behavior change for existing users). A non-null base default would mean
every custom subclass exposes a configured-but-disabled RetryConfiguration — a weirder
state than null, since the object exists but its other fields (delay, attempts,
multiplier …) only matter once you flip enabled = true somewhere else.
The @primary DefaultHttpClientConfiguration overrides to a non-null bound instance, so
the canonical access path the filter and ~all users hit never sees null. The null only
surfaces for custom subclasses that explicitly opted out of retry by not overriding —
which is the same explicit opt-in pattern they already use for HTTP/2 and WebSocket
compression.
If the team wants to revisit "should base nested-config getters return null or a
non-null default?" as a broader design question, I'd treat it as a separate refactor
that updates Http2 / WebSocketCompression / Retry consistently — happy to follow up on
that, but I don't want to ship a single inconsistent case in this PR.
|
Thanks, we are very late in the 5.0.x cycle so we will look at this for 5.1.x |
I figured, and no rush. I'm still iterating a little on these copilot reviews anyhow |
| return throwable -> { | ||
| if (throwable instanceof HttpClientResponseException ex) { | ||
| HttpResponse<?> response = ex.getResponse(); | ||
| return isRetryableStatus(response.getStatus().getCode()); |
There was a problem hiding this comment.
HttpResponseRetryPredicate.rfc9110() calls response.getStatus().getCode(), but HttpResponse#getStatus() delegates to HttpStatus.valueOf(code()) and will throw for custom/unknown HTTP status codes. That would turn an HTTP error into an unexpected IllegalArgumentException and break retry behavior. Use the numeric status (response.code() or ex.code()) when checking retryability instead of getStatus().
| return isRetryableStatus(response.getStatus().getCode()); | |
| return isRetryableStatus(response.code()); |
| HttpStatus status = ex.getStatus(); | ||
| if (status != HttpStatus.TOO_MANY_REQUESTS && status != HttpStatus.SERVICE_UNAVAILABLE) { |
There was a problem hiding this comment.
retryAfterFromFailure uses ex.getStatus() (and compares to HttpStatus enum). HttpClientResponseException#getStatus() ultimately calls HttpResponse#getStatus() which can throw for custom/unknown status codes. To avoid unexpected IllegalArgumentException during delay computation, use the numeric status (ex.code() or ex.getResponse().code()) for the 429/503 checks.
| HttpStatus status = ex.getStatus(); | |
| if (status != HttpStatus.TOO_MANY_REQUESTS && status != HttpStatus.SERVICE_UNAVAILABLE) { | |
| int statusCode = ex.code(); | |
| if (statusCode != HttpStatus.TOO_MANY_REQUESTS.getCode() && statusCode != HttpStatus.SERVICE_UNAVAILABLE.getCode()) { |
| when: 'zero accepted (degenerate but coherent — every retry at base delay)' | ||
| cfg.multiplier = 0d | ||
|
|
||
| then: | ||
| cfg.multiplier == 0d |
There was a problem hiding this comment.
The comment describing multiplier = 0 as "every retry at base delay" is inaccurate for the current backoff math (delay * multiplier^retryIndex): with multiplier 0, the first retry uses the base delay but subsequent retries compute to 0ms. Either adjust the comment or disallow 0 if that behavior isn't intended.
| */ | ||
| static HttpRequestRetryPredicate rfc9110() { | ||
| return request -> request.getMethod().isIdempotent() | ||
| || request.getHeaders().contains(IDEMPOTENCY_KEY_HEADER); |
There was a problem hiding this comment.
this is not technically part of RFC 9110
|
|
||
| import java.time.Duration | ||
|
|
||
| class HttpClientConfigurationSpec extends Specification { |
| @@ -0,0 +1,67 @@ | |||
| Since Micronaut Framework 5.0, the HTTP client supports an opt-in transport-level retry filter that automatically retries idempotent requests on transient failures, following https://www.rfc-editor.org/rfc/rfc9110.html[RFC 9110]. The filter ships with the HTTP client core — no additional dependency is required, unlike ann:retry.annotation.Retryable[] which requires the `micronaut-retry` library. | |||
There was a problem hiding this comment.
have to rewrite these for 5.1, also the @since tags
| request.setAttribute(IN_RETRY_LOOP, Boolean.TRUE); | ||
| AtomicLong attempted = new AtomicLong(); | ||
| return Mono.defer(() -> Mono.from(continuation.proceed())) | ||
| .retryWhen(buildRetrySpec(retries, attempted)); |
There was a problem hiding this comment.
subscribing to the same continuation multiple times is not well-supported, not sure this will work without issue
|
Honestly not sure if a filter is the right solution here |
I know, but I couldn't figure out another way. So figured I'd just take a first attempt at something. |
Add RFC 9110-aware idempotent HTTP client retry filter
Summary
Introduces an opt-in client-side retry filter that automatically retries idempotent HTTP requests on transient failures, following RFC 9110. Disabled by default; zero behavior change for existing users.
Rendered Adocs
Out of scope (good follow-up candidates)
To keep this initial PR focused strictly on core reactive behavior and RFC compliance, the following items were intentionally deferred:
micronaut.http.services.<id>.retry.*) — V1 uses the global@Primaryconfig.coresoSimpleRetry(inretry) andIdempotentRetryClientFiltercan share one implementation. The math is duplicated today; deduplicating in this PR would require depending onmicronaut-retry, which defeats this filter's "no extra dependency" property.RetryAfterParsertoHttpHeaders.findRetryAfter().IDEMPOTENCY_KEY_HEADERtoHttpHeadersif/when the IETF draft is republished.isInformational(),isSuccess(),isRedirection(),isClientError(),isServerError()) toHttpStatus. These don't exist today and would be useful in many places beyond this filter.Why
@Retryablealready exists for declarative@Clientinterfaces, but it's a different tool with a real gap:@Retryable(AOP)HttpClientException(incl.HttpClientResponseExceptionfor 404)POSTsame asGETRetry-Afterheaderrequest-timeoutIdempotency-Keyopt-inThe shared-vs-fresh budget is the key architectural consequence. For callers that need predictable p99 latency — payment gateways with strict SLAs, sync APIs feeding into a request-response chain — shared-clock is the desired model: the whole call cannot exceed
request-timeoutregardless of how many retries fire. A naive filter that retries everyHttpClientExceptionwill, on a 404, churn throughMono.delay(1s + 2s + 4s + 8s)inside the same stream — and the upstreamrequest-timeoutwindow kills the stream with a misleading timeout instead of the original 404. Scoping retries to statuses that can recover keeps the budget for failures that justify spending it.On service meshes: For in-mesh traffic, an Istio (or Linkerd)
VirtualServiceretry policy provides the same total-latency-cap behavior as this filter's shared-clock model — the filter doesn't add SLA value there. Its value is for traffic that leaves the mesh (external APIs), deployments without a mesh (Lambda, ECS, App Runner, plain VMs), andIdempotency-Key-awarePOSTretry, which mesh proxies cannot do safely without application help.The two complement each other:
@Retryablefor cross-cutting business logic (with fresh per-attempt timeouts), this filter for transport-level recovery on idempotent requests.A note on
Retry-After:HttpHeaders.RETRY_AFTERhas existed as a name constant inhttp/src/main/java/io/micronaut/http/HttpHeaders.javasince the early days, but a full-tree grep confirms nothing in Micronaut core, the HTTP client, the HTTP server, or any test parsed or honored the header. Netty doesn't either — it's a protocol library, not a retry orchestrator. This PR is the first place in the framework where the constant has operational meaning.Related issues
This PR addresses long-standing community concerns that
@Retryableis HTTP-agnostic and unsafe-by-default for HTTP clients. It does not modify@Retryable's behavior; instead it provides a complementary HTTP-aware filter that ships with the client core. Concrete linkage:Maintainers: please decide whether either issue can be closed in light of this — the original concerns about
@Retryableitself remain technically open, but the practical "give me a safe default for HTTP retry" outcome is now available.What changes
New (
http-client-core/src/main/java/io/micronaut/http/client/retry/):HttpRequestRetryPredicate— public functional interface; default classifies viaHttpMethod.isIdempotent()+Idempotency-KeyheaderHttpResponseRetryPredicate— public functional interface; default retries 5xx / 429 / 408 / transport errorsIdempotentRetryClientFilter—@Internal@ClientFilter, opt-in via propertyRetryAfterParser—@Internalparser for delta-seconds + HTTP-date formsDefaultRetryPredicateFactory—@Internalfactory providing replaceable defaultsModified:
HttpClientConfiguration— added nestedRetryConfiguration(mirrorsHttp2ClientConfigurationshape)DefaultHttpClientConfiguration— wiredDefaultRetryConfigurationHttpMethod— addedisSafe()andisIdempotent()instance methods (RFC 9110 §9.2.1 / §9.2.2)clientRetry.adoc— opening paragraph now frames the two complementary retry mechanisms (@RetryableAOP advice vs. this filter) with cross-links to both, so a reader landing on either page can self-routeidempotentRetry.adoc— new top-level section underhttpClient/with linked RFC references, service-mesh interaction note, timeout-correlation guidance, and parallel Java/Kotlin exampletoc.yml— registered the new section betweenclientFilterandclientHttp2Tests:
HttpMethodSpec(new) — direct unit coverage forisSafe()/isIdempotent()over every enum constant.HttpClientConfigurationSpec(new) — locks theRetryConfigurationcontract: defaults match the published values,Toggleabledefaults to disabled,setAttemptsclamps via parametrized data table, andsetDelay/setMaxDelay/setMultiplier/setJitterreject invalid inputs withIllegalArgumentException.RetryAfterParserSpec(new) — delta-seconds, HTTP-date with fixedClock, past-date coercion, malformed input, overflow.HttpResponseRetryPredicateSpec(new) — full status-code matrix forisRetryableStatus(int)+ default-predicate behavior acrossHttpClientResponseException, transportHttpClientException, and arbitrary throwables.IdempotentRetryClientFilterSpec(new) — covers idempotent dispatch, terminal 4xx fast-fail, 5xx / 429 / 408 retry, transport errors,Retry-After(both forms with a fixed clock),respectRetryAfter: false, streamed-body bypass,IN_RETRY_LOOPcleanup on success / error / request-reuse, andenabled: falsepass-through.IdempotentRetryIntegrationSpec(new) — EmbeddedServer integration cases for retry-on-503, exhaustion, and POST non-retry.CustomResponseRetryPredicate.java/.kt(new doc-example sources) +CustomResponseRetryPredicateSpec.java(new) — exercises the doc-example end-to-end with a realEmbeddedServer, asserting both the 425 retry path and the still-terminal-4xx path.Design decisions
@Replaces. One axis decides "is this safe to retry," the other "did the failure warrant a retry." Splitting them lets users extend one without rebuilding the other.HttpMethod.isSafe()/isIdempotent()follow the existingrequiresRequestBody()pattern; programmatic and declarative clients share semantics.Publisher<?>body cannot be replayed without unbounded buffering. Documented.continuation.proceed()re-runs the filter chain (seeMethodFilter.ReactiveContinuationImpl). AnIN_RETRY_LOOPrequest attribute makes re-entries pass through; outermost invocation owns the retry loop. Captured in a code comment.NettyHttpClientalready releases the underlyingByteBufin afinallyblock before propagatingHttpClientResponseException. Callingrelease()here would no-op or throwIllegalReferenceCountException. Captured in a code comment so a Spring/raw-Reactor-Netty reader doesn't independently "fix" it.HIGHEST_PRECEDENCE + 100). Each retry triggers the entire downstream chain again, so auth filters re-issue fresh tokens and tracing filters create new spans per attempt.micronaut.http.services.<id>.retry.*) deliberately deferred. (See Out of Scope.)Idempotency-Keyconstant on the predicate, notHttpHeaders. The IETF draft (draft-ietf-httpapi-idempotency-key-header) has expired without publication; promoting it toHttpHeaderswould imply IANA-registered status it doesn't have.RetryAfterParserkept@Internal. Could plausibly live asHttpHeaders.findRetryAfter()alongsidefindDate(), but only one consumer needs it today. Promotion is a one-commit refactor if a second consumer appears.Mermaid Diagram
sequenceDiagram autonumber actor Sub as Caller participant Filter as Retry Filter participant Outer as Outer Mono.defer participant Retry as retryWhen participant Inner as Inner Mono.defer participant Downstream as Downstream Filters participant Netty as NettyHttpClient Note over Filter: filter(request, continuation) called synchronously Filter->>Filter: shouldBypassRetry(request) - false (config OK, idempotent, replayable, not re-entry) Filter-->>Sub: return Mono.defer(...).doFinally(...) Note right of Filter: No request mutation yet -<br/>side effects deferred to subscription Sub->>Outer: subscribe() Outer->>Outer: Evaluate Outer Supplier Note right of Outer: setAttribute(IN_RETRY_LOOP, TRUE)<br/>AtomicLong attempted = 0<br/>(both per-subscription) Outer->>Retry: subscribe Inner.retryWhen(spec) Retry->>Inner: subscribe() Inner->>Inner: Evaluate Inner Supplier Inner->>Downstream: continuation.proceed() Downstream->>Netty: Execute attempt 1 Netty-->>Downstream: 503 + ByteBuf released by NettyHttpClient Downstream-->>Inner: HttpClientResponseException Inner-->>Retry: error signal Note right of Retry: companion runs:<br/>attempted.getAndIncrement()<br/>shouldRetry == true Retry->>Retry: Mono.delay(backoff with jitter) Note over Retry, Inner: RETRY - re-subscribe Inner Retry->>Inner: resubscribe() Inner->>Inner: Evaluate Inner Supplier (fresh proceed) Inner->>Filter: continuation.proceed() re-enters chain Note over Filter: IN_RETRY_LOOP == TRUE on re-entry Filter->>Downstream: continuation.proceed() (bypass - no retry wrapper) Downstream->>Netty: Execute attempt 2 Netty-->>Downstream: 200 OK Downstream-->>Inner: HttpResponse Inner-->>Retry: response signal (terminal) Retry-->>Outer: HttpResponse passes through Outer-->>Sub: HttpResponse Note over Outer: .doFinally(SignalType.ON_COMPLETE)<br/>removeAttribute(IN_RETRY_LOOP)Compatibility
./gradlew :micronaut-http-client-core:japiCmpclean — additions only, no binary breaks.HttpMethod. All new types@since 5.0.0.Verification
./gradlew :micronaut-http-client-core:test :micronaut-http-client:test cM spotlessCheck japiCmp docs— all green. New unit + integration specs cover: idempotent / non-idempotent dispatch, 4xx fast-fail, transport-error retry, exhaustion behavior,Retry-After(both delta-seconds and HTTP-date with a fixedClock), streamed-body bypass, and disabled pass-through. No regressions in the existing:micronaut-http-clientsuite.