Copilot commented on code in PR #10802:
URL: https://github.com/apache/rocketmq/pull/10802#discussion_r3705402347
##########
proxy/src/test/java/org/apache/rocketmq/proxy/service/admin/DefaultAdminServiceTest.java:
##########
@@ -87,6 +88,22 @@ public void testCreateTopic() throws Exception {
assertEquals(8,
topicConfigArgumentCaptor.getValue().getReadQueueNums());
}
+ @Test
+ public void testTopicExistReturnsFalseForNotFound() throws Exception {
+
when(mqClientAPIExt.getTopicRouteInfoFromNameServer(eq("missingTopic"),
anyLong()))
+ .thenThrow(new MQClientException(ResponseCode.TOPIC_NOT_EXIST,
"topic not exist"));
+
+ assertFalse(defaultAdminService.topicExist("missingTopic"));
+ }
+
+ @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");
+ }
Review Comment:
Using `@Test(expected = ...)` only asserts the exception type and does not
verify the exception message/cause. Since this PR is specifically about
surfacing unexpected lookup failures, it would be stronger to assert that the
thrown `IllegalStateException` preserves the original `MQClientException` as
the cause (and optionally that the message contains the topic). Prefer
`assertThrows` + assertions on `getCause()`/`getMessage()`.
##########
proxy/src/main/java/org/apache/rocketmq/proxy/service/admin/DefaultAdminService.java:
##########
@@ -49,8 +49,12 @@ public boolean topicExist(String topic) {
try {
topicRouteData =
this.getTopicRouteDataDirectlyFromNameServer(topic);
topicExist = topicRouteData != null;
- } catch (Throwable e) {
- topicExist = false;
+ } catch (Exception e) {
+ if (TopicRouteHelper.isTopicNotExistError(e)) {
+ topicExist = false;
+ } else {
+ throw new IllegalStateException("get topic route " + topic + "
failed", e);
Review Comment:
The thrown error message is missing quoting/context around the topic name,
which can make debugging harder if the topic contains whitespace/special
characters. Consider formatting it more explicitly (e.g., include quotes or a
key/value style like `topic='...'`) to improve log searchability and
readability.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
