[ISSUE #10801] Surface unexpected topic route lookup errors - #10802
[ISSUE #10801] Surface unexpected topic route lookup errors#10802Aias00 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
This PR changes DefaultAdminService.topicExist to only return false for explicit “topic not found” conditions, and to surface unexpected NameServer route lookup failures by throwing instead of silently treating them as missing topics.
Changes:
- Update
topicExistexception handling to distinguish “not exist” errors from unexpected failures. - Throw an
IllegalStateExceptionfor unexpected route lookup failures. - Add regression tests covering both “not found” and unexpected failure behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| proxy/src/main/java/org/apache/rocketmq/proxy/service/admin/DefaultAdminService.java | Adjusts error handling in topicExist to only suppress explicit not-found and to surface unexpected lookup failures. |
| proxy/src/test/java/org/apache/rocketmq/proxy/service/admin/DefaultAdminServiceTest.java | Adds test coverage for the new topicExist behavior for not-found vs unexpected NameServer errors. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (TopicRouteHelper.isTopicNotExistError(e)) { | ||
| topicExist = false; | ||
| } else { | ||
| throw new IllegalStateException("get topic route " + topic + " failed", e); |
| @Test(expected = IllegalStateException.class) | ||
| public void testTopicExistThrowsForUnexpectedRouteLookupFailure() throws Exception { | ||
| when(mqClientAPIExt.getTopicRouteInfoFromNameServer(eq("brokenTopic"), anyLong())) | ||
| .thenThrow(new MQClientException(ResponseCode.SYSTEM_ERROR, "namesrv unavailable")); | ||
|
|
||
| defaultAdminService.topicExist("brokenTopic"); | ||
| } |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #10802 +/- ##
=============================================
- Coverage 48.34% 48.25% -0.10%
+ Complexity 13527 13495 -32
=============================================
Files 1380 1380
Lines 101104 101140 +36
Branches 13107 13121 +14
=============================================
- Hits 48882 48802 -80
- Misses 46267 46346 +79
- Partials 5955 5992 +37 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Fixes #10801
Tests