[ISSUE #10747] Fix InvokeCallback completion order

[ai-yang]

Which Issue(s) This PR Fixes

• Fixes [Bug] invokeAsyncImpl violates InvokeCallback completion order #10747

Brief Description

invokeAsyncImpl() chained whenComplete, thenAccept, and exceptionally. This caused operationComplete to run before operationSucceed or operationFail, contrary to the InvokeCallback contract, and allowed exceptions thrown by callback methods to flow into the invocation-failure stage.

The fix handles the original invocation result in one terminal whenComplete callback:

• invoke operationSucceed or operationFail before operationComplete;
• preserve unwrapping of the original remoting failure;
• keep callback failures separate from the remoting invocation outcome;
• invoke final completion from finally, including when the outcome callback throws.

Callback exceptions are caught and logged locally because invokeAsyncImpl() returns void and does not expose the dependent completion stage. This guarantees that operationComplete still runs and prevents callback failures from being misclassified as remoting invocation failures.

How Did You Test This Change?

• Unmodified develop: the deterministic callback-order regression failed in 5/5 isolated JDK 8 Maven processes.
• Affected callback regressions: 20 isolated Maven/JVM processes at 5/5 each (100/100 total).
• Complete client reactor: common 241/241, remoting 177/177, and client 578 tests with 1 skip, 0 failures, and 0 errors.
• Checkstyle: 0 violations.
• SpotBugs: 0 bug instances and 0 errors.
• Maven validate: passed.
• git diff --check: passed.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Restructures invokeAsyncImpl to guarantee that outcome callbacks (operationSucceed/operationFail) are always invoked before the completion callback (operationComplete), and wraps both in try-catch to prevent one failure from blocking the other.

Findings

• [Info] NettyRemotingAbstract.java:680-707 — The single whenComplete handler replaces the previous thenAccept/exceptionally chain, eliminating the ordering ambiguity. Outcome callback is now explicitly called first, then completion callback.
• [Info] The try-catch around each callback invocation ensures that a misbehaving callback cannot prevent the other from executing. This is important for cleanup paths.
• [Info] NettyRemotingClientTest.java — Tests now verify callback ordering with InOrder and cover the case where a callback throws. The new testInvokeAsyncCompleteFailureDoesNotInvokeFailureCallback test validates that operationFail receives the correct exception instance.

Suggestions

• Consider adding a brief comment in the whenComplete handler explaining why outcome must precede completion (contract: callers may depend on outcome state in operationComplete).

---

Automated review by github-manager-bot

[RockteMQ-AI]

Summary

Defensive fix with proper validation and test coverage. LGTM.

---

Automated review by github-manager-bot

[ai-yang]

Status refresh (2026-08-28):

• Rebased the PR onto the latest upstream develop (e348efa66). New head: 4a6b17c6c.

• The new upstream head includes [ISSUE #10575] Fix race condition between scanResponseTable and processResponseCommand #10576, which changes response-table removal in the same class. I rechecked the interaction: it addresses a different race and does not correct the InvokeCallback outcome/completion ordering fixed here.

• Re-ran the targeted remoting reactor on the rebased head:

  mvn -Dmaven.repo.local=/tmp/codex-maven-repository -pl remoting -am -DskipITs -Dtest=org.apache.rocketmq.remoting.netty.NettyRemotingClientTest -Dsurefire.failIfNoSpecifiedTests=false test

  Result: BUILD SUCCESS; 17 tests run, 0 failures, 0 errors. Checkstyle and SpotBugs also passed in the reactor.

The PR remains compatible with current develop, and no additional implementation changes were needed.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 48.50%. Comparing base (e348efa) to head (4a6b17c).

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10753      +/-   ##
=============================================
- Coverage      48.58%   48.50%   -0.08%     
+ Complexity     13676    13648      -28     
=============================================
  Files           1381     1381              
  Lines         101475   101479       +4     
  Branches       13190    13191       +1     
=============================================
- Hits           49299    49220      -79     
- Misses         46174    46246      +72     
- Partials        6002     6013      +11     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ai-yang]

All 10 CI checks are green on the refreshed single signed-off commit. @drpmma @aaron-ai, could you please take a human review when convenient? The key point is preserving the InvokeCallback contract that outcome callbacks run before operationComplete, including callback exceptions and the concurrent timeout/response race.