unbridled-41 opened a new pull request, #11015: URL: https://github.com/apache/rocketmq/pull/11015
### Which Issue(s) This PR Fixes - Fixes #11010 ### Problem / Evidence When `writeAndFlush` fails, `NettyRemotingAbstract` drops the netty-side `f.cause()` in four places: 1. `invoke0`'s `writeAndFlush` listener calls `requestFail(opaque)` and logs without the throwable. 2. `requestFail(int opaque)` never calls `responseFuture.setCause(...)`, so `ResponseFuture#executeInvokeCallback` builds `new RemotingSendRequestException(channel.remoteAddress().toString(), getCause())` with a **null** cause — the plumbing for the cause exists on the consumer side, only the assignment is missing. 3. `invokeOnewayImpl`'s listener logs the oneway send failure without `f.cause()`. 4. `invokeOnewayImpl`'s catch block wraps `e` into the thrown exception but never logs it. Consequently every send failure (sync, async and oneway; also the `failFast` channel-close path shares `requestFail`) surfaces without the underlying network error, so the reason a send failed is unrecoverable both programmatically and from the logs. ### Root cause / Fix - Pass `f.cause()` from the `invoke0` listener into a new `requestFail(int opaque, Throwable cause)` overload which calls `responseFuture.setCause(cause)` before executing the callback; the no-cause variant used by `failFast` now delegates to it with `null`. - Include `f.cause()` / the caught exception in the two `invokeOnewayImpl` warn logs. `failFast` behavior is unchanged (it has no cause to record); no public API changes. ### Priority PRIORITY = 70: impact 24 (diagnosability of every remoting send failure, no data loss) + scope 14 (all sync/async/oneway send paths of every client built on the remoting layer) + reproducibility 18 (deterministic unit test) + maintenance 14 (small plumbing fix that completes the existing `ResponseFuture.cause` design). FIX_CONFIDENCE = 95: the consumer side already reads `getCause()`; only the assignment was missing. ### How Did You Test This Change? Regression tests in `NettyRemotingAbstractTest`: - `testInvokeAsyncSendFailureCarriesCause`: mocks a channel whose `writeAndFlush` fails with `ClosedChannelException` (immediate executor, so the listener fires deterministically) and asserts the async callback's `operationFail` receives a `RemotingSendRequestException` whose cause is that exact exception. Fails on unfixed develop (cause is null), passes with this change. - `testInvokeSyncSendFailureCarriesCause`: same failure injection through `invokeSyncImpl`; asserts the thrown `RemotingSendRequestException` cause chain reaches the original `ClosedChannelException`. Commands and results: - `mvn -pl remoting test -Dtest=NettyRemotingAbstractTest` → 7/7 pass (fails at the cause assertion before the fix). - `mvn -pl remoting test` → 173 tests, 0 failures, 1 pre-existing environment error in `TlsTest` (`OpenJdkSelfSignedCertGenerator not supported on the used JDK version`, reproduced on unmodified develop with JDK 21, unrelated to this change). ### Risk Very low: the only behavior change is that recorded/logged exceptions now carry the real failure cause. `requestFail(int)` callers are unchanged in behavior (`failFast` passes `null`, as before). -- 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]
