[PR] [ISSUE #11377] fix(proxy): grade a failed VerifyMessage by the outcome the client reported [rocketmq]

["iloveeyjafjalla (via GitHub)" <gi...@apache.org>]

iloveeyjafjalla opened a new pull request, #11378:
URL: https://github.com/apache/rocketmq/pull/11378

   ### Which Issue(s) This PR Fixes
   
   - Fixes #11377
   
   ### Brief Description
   
   `VerifyMessage` answered `MESSAGE_CORRUPTED` for every outcome that was not `CR_SUCCESS`, although the consumption outcome is
   graded on both sides of the call:
   
   - `ClientActivity#buildConsumeMessageDirectlyResult` maps a gRPC client's answer: `Code.OK` → `CR_SUCCESS`,
     `Code.FAILED_TO_CONSUME_MESSAGE` → `CR_LATER`, `Code.MESSAGE_CORRUPTED` → `CR_RETURN_NULL`;
   - when nothing is consumed, the relay result itself carries the reason (e.g. `SYSTEM_ERROR` with
     "client … is not connected to this cluster").
   
   So a client that refused to consume (or was offline) made the surface report a healthy message as corrupted, and a caller
   that reacts to `MESSAGE_CORRUPTED` did the wrong thing instead of retrying or fixing the client.
   
   The change picks the code from the reported outcome: `MESSAGE_CORRUPTED` only for `CR_RETURN_NULL` — the single outcome a
   client reports for a corrupt message (`Code.MESSAGE_CORRUPTED`) — and `FAILED_TO_CONSUME_MESSAGE` (the proto's generic code for
   failed consumption) for the others, keeping the relay's remark in the message when the client never answered.
   
   ### How Did You Test This Change?
   
   `ProxyAdminGrpcServiceTest`:
   
   - `verifyMessageReportsFailedConsumeWhenTheClientAsksForRetryTest` (`CR_LATER`) and
     `verifyMessageReportsFailedConsumeWhenTheClientThrowsTest` (`CR_THROW_EXCEPTION`, this replaced
     `verifyMessageReportsCorruptedOnFailureTest`, which asserted the old collapse) → `FAILED_TO_CONSUME_MESSAGE`;
   - `verifyMessageReportsCorruptedWhenTheClientReturnsNullTest` (`CR_RETURN_NULL`) → `MESSAGE_CORRUPTED`;
   - `verifyMessageReportsFailedConsumeWhenTheClientNeverAnsweredTest` (no consume result, relay remark
     "client … is not connected") → `FAILED_TO_CONSUME_MESSAGE` and the remark is carried into the message.
   
   Results on Temurin JDK 8 (same JDK as CI), base `9ea2ccdc2` plus this patch:
   
   - before the fix: `expected:<FAILED_TO_CONSUME_MESSAGE> but was:<MESSAGE_CORRUPTED>`
   - after the fix: `ProxyAdminGrpcServiceTest — Tests run: 63, Failures: 0, Errors: 0, Skipped: 0`
   - `mvn -B -ntp -pl proxy package` with checkstyle + spotbugs enabled: `BUILD SUCCESS`, spotbugs `BugInstance size is 0`
   
   Note for the reviewer: the removed test asserted the old one-code-for-everything behavior (its name said so); the three new
   tests cover the outcomes it lumped together.
   
   This change was prepared with AI assistance (analysis, patch and test drafting); the diff and every result above were
   reproduced locally by running the module's tests against the base commit and the patched tree.
   


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org