[Bug] MqClientAdmin futures never complete when remoting invocation fails
- Dominant language
- Java
- Stars
- 22.6k
- Forks
- 12k
- Avg merge
- 3d 1h
- Merged PRs (30d)
- 27
Description
### Before Creating the Bug Report
- [x] This is a reproducible bug rather than a usage question.
- [x] I searched open and closed issues, pull requests, commits, and Discussions and found no duplicate or current claimant.
- [x] The affected code is in this RocketMQ repository.
### Runtime platform environment
Linux x86_64. The reproduction uses a mocked `RemotingClient` and does not require a running RocketMQ cluster.
### RocketMQ version
- Branch: `develop`
- Commit: `00e45b8a6db23efbe756d0306f10716156cfd4dd`
### JDK Version
OpenJDK 8
### Describe the Bug
All 19 asynchronous operations in `MqClientAdminImpl` create a public result `CompletableFuture` and attach their response logic with:
```java
remotingClient.invoke(...).thenAccept(response -> {
// complete the separate result future
});
```
The dependent stage returned by `thenAccept` is ignored. If the remoting future completes exceptionally (timeout, connect failure, send failure, and similar paths), the consumer is not invoked and the separate result future is never completed. The same pending state occurs if a response decoder/handler throws: only the ignored dependent stage observes that exception.
Consequently, callers can wait indefinitely on the returned admin future even though the remoting layer has already completed with a failure. The operation's `timeoutMillis` no longer produces an observable completion at this API boundary.
### Steps to Reproduce
Two deterministic unit-test paths reproduce the problem without sockets, sleeps, or random scheduling:
1. Mock `RemotingClient.invoke` to return a manually controlled `CompletableFuture`.
2. Invoke each of the 19 `MqClientAdminImpl` operations and retain their returned futures.
3. Complete the remoting future exceptionally with a `RemotingException`.
4. Observe that every returned admin future remains incomplete.
For the handler path, return a SUCCESS response with a null body to `viewMessage`; the decoder throws, but the public future again remains incomplete.
On the unmodified commit above, the remoting-failure test and handler-failure test each failed 5/5 times.
### What Did You Expect to See?
- A remoting exception should complete the public admin future exceptionally with the same cause.
- A response handler or decoder exception should complete the public admin future exceptionally rather than leave it pending.
- Existing SUCCESS and non-SUCCESS response-code behavior should remain unchanged.
### What Did You See Instead?
The remoting/dependent stage completes exceptionally, while the distinct future returned to the caller remains pending indefinitely.
### Additional Context
A small bridge around `whenComplete` can propagate the upstream throwable and run the existing response handler inside a `try/catch`, completing the result exceptionally for handler failures. This can be applied consistently to all 19 methods without changing public interfaces or wire protocol.
With that implementation, the two focused tests pass 20/20, the complete `MqClientAdminImplTest` passes 41/41, and the full JDK 8 client reactor passes 995 tests with 0 failures, 0 errors, and 1 skipped test. Checkstyle reports 0 violations, and SpotBugs reports 0 findings/errors.
Historical issue/PR #6644/#6646 introduced the async admin API, while #8375/#8376 added normal response coverage; neither covers exceptional remoting completion or handler failures. A review on #6646 also established catching `Throwable` at the remoting completion boundary as intended behavior: https://github.com/apache/rocketmq/pull/6646#discussion_r1176380128.
Contributor guide
Research direction
Start in MqClientAdminImpl and inspect all 19 asynchronous operations, especially how each uses the RemotingClient future and separate public future. Run MqClientAdminImplTest with mocked exceptional remoting completion and the viewMessage SUCCESS/null-body handler case. Done means both failures complete the public futures exceptionally while existing success and non-SUCCESS behavior remains unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- api
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100