[ISSUE #10806] Log proxy remoting cleanup failures

[Aias00]

Summary

• log unexpected failures from Proxy remoting expired request cleanup with compact queue context
• stop the current cleanup pass after such failures to avoid silent loop churn
• add regression coverage for queue access failures

Tests

• JAVA_HOME=$(/usr/libexec/java_home -v 1.8) mvn -pl proxy -DskipITs -DskipCheckStyle -Dtest=RemotingProtocolServerTest test
• git diff --check

Closes #10806

[Copilot]

Pull request overview

This PR fixes Proxy remoting expired-request queue cleanup to avoid silently swallowing unexpected failures by logging a warning with compact queue context and breaking out of the current cleanup pass. It also adds a regression test to ensure the cleanup does not spin when queue access fails.

Changes:

• Log unexpected exceptions during cleanExpiredRequestInQueue with queue size context and stop the current cleanup pass.
• Add safeQueueSize(...) helper to safely obtain queue size for logging.
• Add a regression unit test covering queue access failure behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
proxy/src/main/java/org/apache/rocketmq/proxy/remoting/RemotingProtocolServer.java	Logs cleanup failures with queue context and breaks the cleanup loop to prevent silent churn.
proxy/src/test/java/org/apache/rocketmq/proxy/remoting/RemotingProtocolServerTest.java	Adds regression coverage ensuring cleanup stops promptly when getQueue() throws.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 60.00000% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.23%. Comparing base (eddb235) to head (1536de3).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
...ocketmq/proxy/remoting/RemotingProtocolServer.java	60.00%	4 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10807      +/-   ##
=============================================
- Coverage      48.34%   48.23%   -0.12%     
+ Complexity     13527    13488      -39     
=============================================
  Files           1380     1380              
  Lines         101104   101113       +9     
  Branches       13107    13108       +1     
=============================================
- Hits           48882    48772     -110     
- Misses         46267    46355      +88     
- Partials        5955     5986      +31     

☔ 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.

[RockteMQ-AI]

Summary

Replaces silent catch (Throwable ignored) blocks with proper warning-level logging in proxy remoting cleanup paths. This improves debuggability for connection lifecycle issues without changing behavior. Good test coverage verifying both success and failure logging.

LGTM.

---

Automated review by github-manager-bot

[RockteMQ-AI]

🤖 Automated Review by RockteMQ-AI

PR Review: #10807

🔴 Critical

None.

🟡 Warning

• RemotingProtocolServer.java:391-393 — Breaking the loop on any Throwable changes behavior: previously the loop silently continued, which could skip a single bad item and still process others. Now a single failure aborts the entire cleanup pass. This is acceptable if the method is invoked periodically, but verify that leaving remaining expired items until the next scheduled run is intentional.
• RemotingProtocolServerTest.java:72 — Mockito.verify(executor, Mockito.atMost(2)).getQueue() is a weak assertion; it permits 0, 1, or 2 invocations and does not strongly verify the break-on-failure behavior. Prefer Mockito.times(2) because the loop calls getQueue() once, fails, then safeQueueSize() calls it once more during logging.
• RemotingProtocolServer.java:391 — Ensure the log field exists in RemotingProtocolServer (not visible in the diff). Otherwise the change will not compile.

🟢 Suggestion

• Add a test for the happy path to confirm normal expired-request cleanup still functions after the catch/block change.
• Consider whether this failure should be logged at error rather than warn, since it represents an unexpected state in the cleanup path.

---

_{Automated review. Please verify findings before acting on them.}