fix(httpclient): cache request body for replay during retry

[eye-gu]

Fixes #6413

When retry is enabled, exchange.getRequest().getBody() (a single-use FluxReceive) is exhausted after the first attempt. Retry subscriptions emit onComplete immediately, so upstreams receive an empty body — silent data loss with no error logged.

Add getCachedRequestBody(): DataBufferUtils.join → read byte[] → .cache() → Flux.defer(wrap) per subscription.

Make sure that:

• You have read the contribution guidelines.
• You submit test cases (unit or integration tests) that back your changes.
• Your local test passed ./mvnw clean install -Dmaven.javadoc.skip=true.

[Aias00]

I found one issue that should be addressed before merging.

AbstractHttpClientPlugin#getCachedRequestBody now uses DataBufferUtils.join(exchange.getRequest().getBody()) for every retriable non-GET/HEAD request, then copies the whole body into a cached byte[] before the first upstream attempt. That fixes replay, but it also changes retry-enabled requests from streaming to full in-memory buffering with no size limit.

For large/chunked uploads, this can hold the gateway request until the client finishes sending the whole body and can create an unbounded heap spike whenever httpRetry > 0. This path also does not appear to enforce the existing shenyu.httpclient.maxInMemorySize limit used by the WebClient configuration.

Could we gate this replay cache behind a max byte limit, disable retry/replay for streaming or unknown-size bodies, or otherwise reuse the module's existing max-body setting? I would also add a failover retry test, since the PR changes the DefaultRetryStrategy.resend(...) body path but the new test coverage only exercises the current retry strategy.

[eye-gu]

Check the body size based on Content-Length, and if it is greater than maxInCacheSize, it will not retry

[Aias00]

Reviewing #6414. The body-replay fix is the right idea and the test coverage for the POST retry path is solid, but there's a regression I think needs to be addressed before merge.

Blocker — GET/HEAD retry is silently disabled (regression from master).
AbstractHttpClientPlugin.execute gates retry on body replay: bodyReplayEnabled = configuredRetryTimes > 0 && isRequestBodyRequired(httpMethod) && isCacheable(exchange) (AbstractHttpClientPlugin.java:85-87) and then retryTimes = bodyReplayEnabled ? configuredRetryTimes : 0. isRequestBodyRequired returns false for GET/HEAD (AbstractHttpClientPlugin.java:184-186), so every GET/HEAD with HTTP_RETRY>0 gets retryTimes=0 and never retries — across all strategies (current, failover, exponential, fixed, custom). On master, retryTimes came straight from HTTP_RETRY (master AbstractHttpClientPlugin.java:74) and GET retried fine because NettyHttpClientPlugin.doRequest (master :82-87) never subscribes to the body for GET/HEAD, so retryWhen resubscribed cleanly. There is no GET-retry test in RequestBodyReplayRetryTest. Retry-enabled and body-needs-replay are two separate concerns; please decouple them: keep retryTimes = configuredRetryTimes and only use bodyReplayEnabled to choose between getCachedRequestBody(exchange) and exchange.getRequest().getBody().

Should fix — cache cap trusts the forgeable Content-Length, not actual bytes.
isCacheable (AbstractHttpClientPlugin.java:130-133) checks exchange.getRequest().getHeaders().getContentLength() <= maxInMemorySize, but getCachedRequestBody (AbstractHttpClientPlugin.java:152-160) then calls DataBufferUtils.join(body), which aggregates every emitted buffer into one heap buffer with no bound. A client can send Content-Length: 1 and stream gigabytes, passing the gate while join buffers the whole payload on heap — an OOM/DoS on any retry-enabled route. Please enforce the cap against actual accumulated bytes during aggregation (e.g. a bounded join that releases buffers and errors with DataBufferLimitException once readableByteCount exceeds maxInMemorySize), and treat the header as a hint only.

Should fix — binary/source-incompatible constructor removal.
NettyHttpClientPlugin.java:59 and WebClientPlugin.java:52 replace the public single-arg constructors with two-arg forms; only the starter is updated. These are public API in a framework jar — downstream code, custom plugins, and tests calling new NettyHttpClientPlugin(httpClient) / new WebClientPlugin(webClient) will break. Please keep the single-arg constructors as @Deprecated delegating overloads (defaulting maxInMemorySize).

Should fix — int overflow in maxInMemorySize * BYTES_PER_MB.
HttpClientPluginConfiguration.java:113 and :134 compute properties.getMaxInMemorySize() * Constants.BYTES_PER_MB in int (property is Integer at HttpClientProperties.java:381, BYTES_PER_MB is int at Constants.java:811), passed to the int field at AbstractHttpClientPlugin.java:64. For maxInMemorySize >= 2048, 2048 * 1048576 = 2^31 overflows to negative and isCacheable returns false for every body, silently disabling retry for all bodies; >= 4096 overflows to 0 with the same effect. An operator raising the cap to allow retry for a multi-MB body gets retry disabled everywhere. Please compute in long and store the field as long.

Nits. (1) The warn at AbstractHttpClientPlugin.java:88-91 fires on every GET with retry configured and blames body size for a bodyless method — once the blocker is fixed, GET shouldn't warn at all; for POST the message should reflect the actual reason. (2) getCachedRequestBody (AbstractHttpClientPlugin.java:165) wraps request bytes with exchange.getResponse().bufferFactory(); the request's own factory would be more correct and less fragile given NettyHttpClientPlugin.doRequest casts buffers to NettyDataBuffer (NettyHttpClientPlugin.java:83-84).

Happy to review a follow-up commit addressing the above.

[Aias00]

Solid, well-tested fix — caching the single-use Netty body Flux as byte[] so retries replay the full body is the right approach, and the test suite (replay on retry, multi-retry, replayable Flux, idempotency, failover resend, oversize-disables-retry) is thorough. The fail-safe behavior (disable retry + warn when the body is unknown-size or oversize, rather than silently retrying with an empty body) is a good defensive choice.

Two things worth addressing:

Constructor signature change is an API break for downstream consumers. NettyHttpClientPlugin(HttpClient) and WebClientPlugin(WebClient) each gained a required int maxInMemorySize param. In-repo callers are updated (CI green), but these are public plugin classes — any external code or custom plugin config that instantiates them directly would fail to compile. A backward-compat overload delegating with a sensible default (e.g. Constants.BYTES_PER_MB) would smooth this. (May already be part of the CHANGES_REQUESTED feedback; flagging in case it isn't.)

Cacheability is Content-Length-only. isCacheable reads getHeaders().getContentLength(), so any chunked / unknown-length request (no Content-Length) returns -1 → not cacheable → retry disabled, even for tiny bodies. That's a safe default, but it means a class of requests silently loses retry. An alternative would be to buffer up to maxInMemorySize and bail mid-stream if exceeded — more complex, but it would cover chunked small bodies. Fine to keep as-is if the trade-off is acceptable; just flagging.

Minor: where does the production maxInMemorySize come from? The test hardcodes Constants.BYTES_PER_MB; is it wired from ShenyuConfig (configurable) or hardcoded in the bean definition? If the latter, operators can't tune the cache limit. Worth confirming it's config-driven.