(kafka) branch trunk updated: KAFKA-18061 AddRaftVoter responds with error message "NONE" instead of null (#17930)

[chia7712]

This is an automated email from the ASF dual-hosted git repository.

chia7712 pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 9c9f1446a16 KAFKA-18061 AddRaftVoter responds with error message 
"NONE" instead of null (#17930)
9c9f1446a16 is described below

commit 9c9f1446a16124c177bf7412869e14ded9032f88
Author: Linsiyuan9 <154567180+linsiyu...@users.noreply.github.com>
AuthorDate: Tue Sep 9 13:58:10 2025 +0800

    KAFKA-18061 AddRaftVoter responds with error message "NONE" instead of null 
(#17930)
    
    In `RaftUtil.addVoterResponse` and `RaftUtil.removeVoterResponse`
    methods, when the input `errorMessage` is `null`, the returned string is
    not actually null but `NONE`.
    
    This introduces an inconsistency: semantically, `null` should represent
    “no error message,” while `NONE` looks like a real string value and may
    confuse clients.
    
    Reviewers: Alyssa Huang <ahu...@confluent.io>, José Armando García
     Sancio <jsan...@apache.org>, Anton Agestam <anton.ages...@aiven.io>,
     Chia-Ping Tsai <chia7...@gmail.com>
---
 .../main/java/org/apache/kafka/raft/RaftUtil.java  | 12 +++++++----
 .../apache/kafka/raft/RaftClientTestContext.java   | 17 +++++++++++----
 .../java/org/apache/kafka/raft/RaftUtilTest.java   | 25 ++++++++++++++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/raft/src/main/java/org/apache/kafka/raft/RaftUtil.java 
b/raft/src/main/java/org/apache/kafka/raft/RaftUtil.java
index f3f411885a7..d3a4f760968 100644
--- a/raft/src/main/java/org/apache/kafka/raft/RaftUtil.java
+++ b/raft/src/main/java/org/apache/kafka/raft/RaftUtil.java
@@ -542,8 +542,10 @@ public class RaftUtil {
         Errors error,
         String errorMessage
     ) {
-        errorMessage = errorMessage == null ? error.message() : errorMessage;
-
+        // return the provided errorMessage if it exists, Errors.NONE should 
have a null message
+        if (errorMessage == null && error != Errors.NONE) {
+            errorMessage = error.message();
+        }
         return new AddRaftVoterResponseData()
             .setErrorCode(error.code())
             .setErrorMessage(errorMessage);
@@ -563,8 +565,10 @@ public class RaftUtil {
         Errors error,
         String errorMessage
     ) {
-        errorMessage = errorMessage == null ? error.message() : errorMessage;
-
+        // return the provided errorMessage if it exists, Errors.NONE should 
have a null message
+        if (errorMessage == null && error != Errors.NONE) {
+            errorMessage = error.message();
+        }
         return new RemoveRaftVoterResponseData()
             .setErrorCode(error.code())
             .setErrorMessage(errorMessage);
diff --git 
a/raft/src/test/java/org/apache/kafka/raft/RaftClientTestContext.java 
b/raft/src/test/java/org/apache/kafka/raft/RaftClientTestContext.java
index a98fb79d09a..42fdde0fa0e 100644
--- a/raft/src/test/java/org/apache/kafka/raft/RaftClientTestContext.java
+++ b/raft/src/test/java/org/apache/kafka/raft/RaftClientTestContext.java
@@ -108,6 +108,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public final class RaftClientTestContext {
@@ -1341,8 +1342,12 @@ public final class RaftClientTestContext {
         assertInstanceOf(AddRaftVoterResponseData.class, response.data());
 
         AddRaftVoterResponseData addVoterResponse = (AddRaftVoterResponseData) 
response.data();
-        assertEquals(error, Errors.forCode(addVoterResponse.errorCode()));
-
+        if (Errors.NONE.equals(error)) {
+            assertEquals(error, Errors.forCode(addVoterResponse.errorCode()));
+            assertNull(addVoterResponse.errorMessage());
+        } else {
+            assertEquals(error, Errors.forCode(addVoterResponse.errorCode()));
+        }
         return addVoterResponse;
     }
 
@@ -1371,8 +1376,12 @@ public final class RaftClientTestContext {
         assertInstanceOf(RemoveRaftVoterResponseData.class, response.data());
 
         RemoveRaftVoterResponseData removeVoterResponse = 
(RemoveRaftVoterResponseData) response.data();
-        assertEquals(error, Errors.forCode(removeVoterResponse.errorCode()));
-
+        if (Errors.NONE.equals(error)) {
+            assertEquals(error, 
Errors.forCode(removeVoterResponse.errorCode()));
+            assertNull(removeVoterResponse.errorMessage());
+        } else {
+            assertEquals(error, 
Errors.forCode(removeVoterResponse.errorCode()));
+        }
         return removeVoterResponse;
     }
 
diff --git a/raft/src/test/java/org/apache/kafka/raft/RaftUtilTest.java 
b/raft/src/test/java/org/apache/kafka/raft/RaftUtilTest.java
index cdbd728cb1e..5aacab01df6 100644
--- a/raft/src/test/java/org/apache/kafka/raft/RaftUtilTest.java
+++ b/raft/src/test/java/org/apache/kafka/raft/RaftUtilTest.java
@@ -72,6 +72,7 @@ import java.util.Map;
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class RaftUtilTest {
@@ -646,6 +647,30 @@ public class RaftUtilTest {
         );
     }
 
+    @Test
+    public void testAddVoterResponse() {
+        for (Errors error : Errors.values()) {
+            AddRaftVoterResponseData addRaftVoterResponseData = 
RaftUtil.addVoterResponse(error, null);
+            assertEquals(error.code(), addRaftVoterResponseData.errorCode());
+            if (Errors.NONE.equals(error))
+                assertNull(addRaftVoterResponseData.errorMessage());
+            else
+                assertEquals(error.message(), 
addRaftVoterResponseData.errorMessage());
+        }
+    }
+
+    @Test
+    public void testRemoveVoterResponse() {
+        for (Errors error : Errors.values()) {
+            RemoveRaftVoterResponseData removeRaftVoterResponseData = 
RaftUtil.removeVoterResponse(error, null);
+            assertEquals(error.code(), 
removeRaftVoterResponseData.errorCode());
+            if (Errors.NONE.equals(error))
+                assertNull(removeRaftVoterResponseData.errorMessage());
+            else
+                assertEquals(error.message(), 
removeRaftVoterResponseData.errorMessage());
+        }
+    }
+
     private Records createRecords() {
         ByteBuffer allocate = ByteBuffer.allocate(1024);