Aias00 commented on PR #6414: URL: https://github.com/apache/shenyu/pull/6414#issuecomment-5193465044
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. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
