[ISSUE #9028] Adjust some error code for SYSTEM_ERROR (#9027)

* feat: change some SYSTEM_ERROR to more meaningful error code Change-Id: I4b6ffa5aa18325eeadc29941c5788244c2770423 * feat: change some SYSTEM_ERROR to more meaningful error code Change-Id: I0c6ff75c5a2f7adde73261da93608781260e09da * test: adjust test case for error code Change-Id: I302ff5ad204280b55c8427ba4e8563b042263aeb * test: adjust test case for error code Change-Id: I7fc958c865c53b2a66b7bd77b6fb69f1546d2826
apache · Dec 14, 2024 · b5cf3ca · b5cf3ca
1 parent 9aa081b
commit b5cf3ca
Show file tree

Hide file tree

Showing 13 changed files with 38 additions and 36 deletions.
diff --git a/broker/src/main/java/org/apache/rocketmq/broker/client/net/Broker2Client.java b/broker/src/main/java/org/apache/rocketmq/broker/client/net/Broker2Client.java
@@ -113,7 +113,7 @@ public RemotingCommand resetOffset(String topic, String group, long timeStamp, b
         TopicConfig topicConfig = this.brokerController.getTopicConfigManager().selectTopicConfig(topic);
         if (null == topicConfig) {
             log.error("[reset-offset] reset offset failed, no topic in this broker. topic={}", topic);
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.TOPIC_NOT_EXIST);
             response.setRemark("[reset-offset] reset offset failed, no topic in this broker. topic=" + topic);
             return response;
         }

diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/AbstractSendMessageProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/AbstractSendMessageProcessor.java
@@ -467,7 +467,7 @@ protected RemotingCommand msgCheck(final ChannelHandlerContext ctx,
 
         TopicValidator.ValidateTopicResult result = TopicValidator.validateTopic(requestHeader.getTopic());
         if (!result.isValid()) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(result.getRemark());
             return response;
         }
@@ -522,7 +522,7 @@ protected RemotingCommand msgCheck(final ChannelHandlerContext ctx,
                 RemotingHelper.parseChannelRemoteAddr(ctx.channel()));
 
             LOGGER.warn(errorInfo);
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(errorInfo);
 
             return response;

diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessor.java
@@ -425,7 +425,7 @@ private RemotingCommand getSubscriptionGroup(ChannelHandlerContext ctx,
         SubscriptionGroupConfig groupConfig = this.brokerController.getSubscriptionGroupManager().getSubscriptionGroupTable().get(requestHeader.getGroup());
         if (groupConfig == null) {
             LOGGER.error("No group in this broker, client: {} group: {}", ctx.channel().remoteAddress(), requestHeader.getGroup());
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.SUBSCRIPTION_GROUP_NOT_EXIST);
             response.setRemark("No group in this broker");
             return response;
         }
@@ -514,13 +514,13 @@ private synchronized RemotingCommand updateAndCreateTopic(ChannelHandlerContext
         try {
             TopicValidator.ValidateTopicResult result = TopicValidator.validateTopic(topic);
             if (!result.isValid()) {
-                response.setCode(ResponseCode.SYSTEM_ERROR);
+                response.setCode(ResponseCode.INVALID_PARAMETER);
                 response.setRemark(result.getRemark());
                 return response;
             }
             if (brokerController.getBrokerConfig().isValidateSystemTopicWhenUpdateTopic()) {
                 if (TopicValidator.isSystemTopic(topic)) {
-                    response.setCode(ResponseCode.SYSTEM_ERROR);
+                    response.setCode(ResponseCode.INVALID_PARAMETER);
                     response.setRemark("The topic[" + topic + "] is conflict with system topic.");
                     return response;
                 }
@@ -541,7 +541,7 @@ private synchronized RemotingCommand updateAndCreateTopic(ChannelHandlerContext
                 String msgTypeAttrKey = AttributeParser.ATTR_ADD_PLUS_SIGN + TopicAttributes.TOPIC_MESSAGE_TYPE_ATTRIBUTE.getName();
                 String msgTypeAttrValue = topicConfig.getAttributes().get(msgTypeAttrKey);
                 if (msgTypeAttrValue != null && msgTypeAttrValue.equals(TopicMessageType.MIXED.getValue())) {
-                    response.setCode(ResponseCode.SYSTEM_ERROR);
+                    response.setCode(ResponseCode.INVALID_PARAMETER);
                     response.setRemark("MIXED message type is not supported.");
                     return response;
                 }
@@ -604,13 +604,13 @@ private synchronized RemotingCommand updateAndCreateTopicList(ChannelHandlerCont
                 String topic = topicConfig.getTopicName();
                 TopicValidator.ValidateTopicResult result = TopicValidator.validateTopic(topic);
                 if (!result.isValid()) {
-                    response.setCode(ResponseCode.SYSTEM_ERROR);
+                    response.setCode(ResponseCode.INVALID_PARAMETER);
                     response.setRemark(result.getRemark());
                     return response;
                 }
                 if (brokerController.getBrokerConfig().isValidateSystemTopicWhenUpdateTopic()) {
                     if (TopicValidator.isSystemTopic(topic)) {
-                        response.setCode(ResponseCode.SYSTEM_ERROR);
+                        response.setCode(ResponseCode.INVALID_PARAMETER);
                         response.setRemark("The topic[" + topic + "] is conflict with system topic.");
                         return response;
                     }
@@ -620,7 +620,7 @@ private synchronized RemotingCommand updateAndCreateTopicList(ChannelHandlerCont
                     String msgTypeAttrKey = AttributeParser.ATTR_ADD_PLUS_SIGN + TopicAttributes.TOPIC_MESSAGE_TYPE_ATTRIBUTE.getName();
                     String msgTypeAttrValue = topicConfig.getAttributes().get(msgTypeAttrKey);
                     if (msgTypeAttrValue != null && msgTypeAttrValue.equals(TopicMessageType.MIXED.getValue())) {
-                        response.setCode(ResponseCode.SYSTEM_ERROR);
+                        response.setCode(ResponseCode.INVALID_PARAMETER);
                         response.setRemark("MIXED message type is not supported.");
                         return response;
                     }
@@ -674,13 +674,13 @@ private synchronized RemotingCommand updateAndCreateStaticTopic(ChannelHandlerCo
 
         TopicValidator.ValidateTopicResult result = TopicValidator.validateTopic(topic);
         if (!result.isValid()) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(result.getRemark());
             return response;
         }
         if (brokerController.getBrokerConfig().isValidateSystemTopicWhenUpdateTopic()) {
             if (TopicValidator.isSystemTopic(topic)) {
-                response.setCode(ResponseCode.SYSTEM_ERROR);
+                response.setCode(ResponseCode.INVALID_PARAMETER);
                 response.setRemark("The topic[" + topic + "] is conflict with system topic.");
                 return response;
             }
@@ -721,14 +721,14 @@ private synchronized RemotingCommand deleteTopic(ChannelHandlerContext ctx,
         String topic = requestHeader.getTopic();
 
         if (UtilAll.isBlank(topic)) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark("The specified topic is blank.");
             return response;
         }
 
         if (brokerController.getBrokerConfig().isValidateSystemTopicWhenUpdateTopic()) {
             if (TopicValidator.isSystemTopic(topic)) {
-                response.setCode(ResponseCode.SYSTEM_ERROR);
+                response.setCode(ResponseCode.INVALID_PARAMETER);
                 response.setRemark("The topic[" + topic + "] is conflict with system topic.");
                 return response;
             }
@@ -1092,7 +1092,7 @@ private RemotingCommand setCommitLogReadaheadMode(ChannelHandlerContext ctx, Rem
             }
             int mode = Integer.parseInt(extFields.get(FIleReadaheadMode.READ_AHEAD_MODE));
             if (mode != LibC.MADV_RANDOM && mode != LibC.MADV_NORMAL) {
-                response.setCode(ResponseCode.SYSTEM_ERROR);
+                response.setCode(ResponseCode.INVALID_PARAMETER);
                 response.setRemark("set commitlog readahead mode param value error");
                 return response;
             }
@@ -3081,7 +3081,7 @@ private RemotingCommand createUser(ChannelHandlerContext ctx,
 
         CreateUserRequestHeader requestHeader = request.decodeCommandCustomHeader(CreateUserRequestHeader.class);
         if (StringUtils.isEmpty(requestHeader.getUsername())) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark("The username is blank");
             return response;
         }
@@ -3113,7 +3113,7 @@ private RemotingCommand updateUser(ChannelHandlerContext ctx,
 
         UpdateUserRequestHeader requestHeader = request.decodeCommandCustomHeader(UpdateUserRequestHeader.class);
         if (StringUtils.isEmpty(requestHeader.getUsername())) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark("The username is blank");
             return response;
         }
@@ -3177,7 +3177,7 @@ private RemotingCommand getUser(ChannelHandlerContext ctx,
         GetUserRequestHeader requestHeader = request.decodeCommandCustomHeader(GetUserRequestHeader.class);
 
         if (StringUtils.isBlank(requestHeader.getUsername())) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark("The username is blank");
             return response;
         }

diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/ConsumerManageProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/ConsumerManageProcessor.java
@@ -177,13 +177,13 @@ private RemotingCommand updateConsumerOffset(ChannelHandlerContext ctx, Remoting
         }
 
         if (queueId == null) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark("QueueId is null, topic is " + topic);
             return response;
         }
 
         if (offset == null) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark("Offset is null, topic is " + topic);
             return response;
         }

diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/NotificationProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/NotificationProcessor.java
@@ -112,7 +112,7 @@ public RemotingCommand processRequest(final ChannelHandlerContext ctx,
             String errorInfo = String.format("queueId[%d] is illegal, topic:[%s] topicConfig.readQueueNums:[%d] consumer:[%s]",
                 requestHeader.getQueueId(), requestHeader.getTopic(), topicConfig.getReadQueueNums(), channel.remoteAddress());
             POP_LOGGER.warn(errorInfo);
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(errorInfo);
             return response;
         }

diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/PeekMessageProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/PeekMessageProcessor.java
@@ -114,7 +114,7 @@ private RemotingCommand processRequest(final Channel channel, RemotingCommand re
             String errorInfo = String.format("queueId[%d] is illegal, topic:[%s] topicConfig.readQueueNums:[%d] consumer:[%s]",
                 requestHeader.getQueueId(), requestHeader.getTopic(), topicConfig.getReadQueueNums(), channel.remoteAddress());
             LOG.warn(errorInfo);
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(errorInfo);
             return response;
         }

diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/PollingInfoProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/PollingInfoProcessor.java
@@ -89,7 +89,7 @@ private RemotingCommand processRequest(final Channel channel, RemotingCommand re
             String errorInfo = String.format("queueId[%d] is illegal, topic:[%s] topicConfig.readQueueNums:[%d] consumer:[%s]",
                 requestHeader.getQueueId(), requestHeader.getTopic(), topicConfig.getReadQueueNums(), channel.remoteAddress());
             POP_LOGGER.warn(errorInfo);
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(errorInfo);
             return response;
         }

diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/PopMessageProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/PopMessageProcessor.java
@@ -252,7 +252,7 @@ public RemotingCommand processRequest(final ChannelHandlerContext ctx, RemotingC
             return response;
         }
         if (requestHeader.getMaxMsgNums() > 32) {
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(String.format("the broker[%s] pop message's num is greater than 32",
                 this.brokerController.getBrokerConfig().getBrokerIP1()));
             return response;
@@ -288,7 +288,7 @@ public RemotingCommand processRequest(final ChannelHandlerContext ctx, RemotingC
                 requestHeader.getQueueId(), requestHeader.getTopic(), topicConfig.getReadQueueNums(),
                 channel.remoteAddress());
             POP_LOGGER.warn(errorInfo);
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(errorInfo);
             return response;
         }

diff --git a/broker/src/main/java/org/apache/rocketmq/broker/processor/PullMessageProcessor.java b/broker/src/main/java/org/apache/rocketmq/broker/processor/PullMessageProcessor.java
@@ -371,7 +371,7 @@ private RemotingCommand processRequest(final Channel channel, RemotingCommand re
             String errorInfo = String.format("queueId[%d] is illegal, topic:[%s] topicConfig.readQueueNums:[%d] consumer:[%s]",
                 requestHeader.getQueueId(), requestHeader.getTopic(), topicConfig.getReadQueueNums(), channel.remoteAddress());
             LOGGER.warn(errorInfo);
-            response.setCode(ResponseCode.SYSTEM_ERROR);
+            response.setCode(ResponseCode.INVALID_PARAMETER);
             response.setRemark(errorInfo);
             return response;
         }

diff --git a/broker/src/test/java/org/apache/rocketmq/broker/client/net/Broker2ClientTest.java b/broker/src/test/java/org/apache/rocketmq/broker/client/net/Broker2ClientTest.java
@@ -129,7 +129,7 @@ public void testCheckProducerTransactionStateException() throws Exception {
     public void testResetOffsetNoTopicConfig() throws RemotingCommandException {
         when(topicConfigManager.selectTopicConfig(defaultTopic)).thenReturn(null);
         RemotingCommand response = broker2Client.resetOffset(defaultTopic, defaultGroup, timestamp, isForce);
-        assertEquals(ResponseCode.SYSTEM_ERROR, response.getCode());
+        assertEquals(ResponseCode.TOPIC_NOT_EXIST, response.getCode());
     }
 
     @Test

diff --git a/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java b/broker/src/test/java/org/apache/rocketmq/broker/processor/AdminBrokerProcessorTest.java
@@ -317,15 +317,15 @@ public void testUpdateAndCreateTopic() throws Exception {
         for (String topic : systemTopicSet) {
             RemotingCommand request = buildCreateTopicRequest(topic);
             RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request);
-            assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+            assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
             assertThat(response.getRemark()).isEqualTo("The topic[" + topic + "] is conflict with system topic.");
         }
 
         //test validate error topic
         String topic = "";
         RemotingCommand request = buildCreateTopicRequest(topic);
         RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request);
-        assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+        assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
 
         topic = "TEST_CREATE_TOPIC";
         request = buildCreateTopicRequest(topic);
@@ -339,7 +339,7 @@ public void testUpdateAndCreateTopic() throws Exception {
         attributes.put("+message.type", "MIXED");
         request = buildCreateTopicRequest(topic, attributes);
         response = adminBrokerProcessor.processRequest(handlerContext, request);
-        assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+        assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
         // test allow MIXED topic type
         brokerController.getBrokerConfig().setEnableMixedMessageType(true);
         response = adminBrokerProcessor.processRequest(handlerContext, request);
@@ -351,14 +351,14 @@ public void testUpdateAndCreateTopicList() throws RemotingCommandException {
         List<String> systemTopicList = new ArrayList<>(systemTopicSet);
         RemotingCommand request = buildCreateTopicListRequest(systemTopicList);
         RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request);
-        assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+        assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
         assertThat(response.getRemark()).isEqualTo("The topic[" + systemTopicList.get(0) + "] is conflict with system topic.");
 
         List<String> inValidTopicList = new ArrayList<>();
         inValidTopicList.add("");
         request = buildCreateTopicListRequest(inValidTopicList);
         response = adminBrokerProcessor.processRequest(handlerContext, request);
-        assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+        assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
 
         List<String> topicList = new ArrayList<>();
         topicList.add("TEST_CREATE_TOPIC");
@@ -378,7 +378,7 @@ public void testUpdateAndCreateTopicList() throws RemotingCommandException {
         attributes.put("+message.type", "MIXED");
         request = buildCreateTopicListRequest(topicList, attributes);
         response = adminBrokerProcessor.processRequest(handlerContext, request);
-        assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+        assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
         // test allow MIXED topic type
         brokerController.getBrokerConfig().setEnableMixedMessageType(true);
         response = adminBrokerProcessor.processRequest(handlerContext, request);
@@ -400,7 +400,7 @@ public void testDeleteTopic() throws Exception {
         for (String topic : systemTopicSet) {
             RemotingCommand request = buildDeleteTopicRequest(topic);
             RemotingCommand response = adminBrokerProcessor.processRequest(handlerContext, request);
-            assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+            assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
             assertThat(response.getRemark()).isEqualTo("The topic[" + topic + "] is conflict with system topic.");
         }
 
@@ -1065,7 +1065,7 @@ public void testSetCommitLogReadAheadMode() throws RemotingCommandException {
         extfields.put(FIleReadaheadMode.READ_AHEAD_MODE, String.valueOf(LibC.MADV_DONTNEED));
         request.setExtFields(extfields);
         response = adminBrokerProcessor.processRequest(handlerContext, request);
-        assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+        assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
 
         extfields.clear();
         extfields.put(FIleReadaheadMode.READ_AHEAD_MODE, String.valueOf(LibC.MADV_NORMAL));

diff --git a/broker/src/test/java/org/apache/rocketmq/broker/processor/PeekMessageProcessorTest.java b/broker/src/test/java/org/apache/rocketmq/broker/processor/PeekMessageProcessorTest.java
@@ -154,7 +154,7 @@ public void testProcessRequest_SubscriptionGroupNotExist() throws RemotingComman
     public void testProcessRequest_QueueIdError() throws RemotingCommandException {
         RemotingCommand request = createPeekMessageRequest("group","topic",17);
         RemotingCommand response = peekMessageProcessor.processRequest(handlerContext, request);
-        assertThat(response.getCode()).isEqualTo(ResponseCode.SYSTEM_ERROR);
+        assertThat(response.getCode()).isEqualTo(ResponseCode.INVALID_PARAMETER);
     }
 
     private RemotingCommand createPeekMessageRequest(String group,String topic,int queueId) {

diff --git a/remoting/src/main/java/org/apache/rocketmq/remoting/protocol/ResponseCode.java b/remoting/src/main/java/org/apache/rocketmq/remoting/protocol/ResponseCode.java
@@ -55,6 +55,8 @@ public class ResponseCode extends RemotingSysResponseCode {
 
     public static final int FILTER_DATA_NOT_LATEST = 28;
 
+    public static final int INVALID_PARAMETER = 29;
+
     public static final int TRANSACTION_SHOULD_COMMIT = 200;
 
     public static final int TRANSACTION_SHOULD_ROLLBACK = 201;