Merge pull request ClickHouse#1582 from zalj/main

chernser · web-flow · commit 2ca52fc91a57 · 2024-04-04T07:08:45.000-07:00
Using a more elegant Pattern-Matcher method to extract error code from error message
diff --git a/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseException.java b/clickhouse-client/src/main/java/com/clickhouse/client/ClickHouseException.java
@@ -5,6 +5,8 @@
 import java.net.UnknownHostException;
 import java.util.Locale;
 import java.util.concurrent.TimeoutException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 /**
  * Exception thrown from ClickHouse server. See full list at
@@ -28,6 +30,7 @@ public class ClickHouseException extends Exception {
 
     static final String MSG_CODE = "Code: ";
     static final String MSG_CONNECT_TIMED_OUT = "connect timed out";
+    static final Pattern ERROR_CODE_PATTERN = Pattern.compile("Code:[ ]*(\\d+)");
 
     private final int errorCode;
 
@@ -60,27 +63,10 @@ private static String buildErrorMessage(int code, String message, ClickHouseNode
     private static int extractErrorCode(String errorMessage) {
         if (errorMessage == null || errorMessage.isEmpty()) {
             return ERROR_UNKNOWN;
-        } else if (errorMessage.startsWith("Poco::Exception. Code: 1000, ")) {
-            return ERROR_POCO;
         }
-
-        int startIndex = errorMessage.indexOf(' ');
-        if (startIndex >= 0) {
-            for (int i = ++startIndex, len = errorMessage.length(); i < len; i++) {
-                char ch = errorMessage.charAt(i);
-                if (ch == '.' || ch == ',' || Character.isWhitespace(ch)) {
-                    try {
-                        return Integer.parseInt(errorMessage.substring(startIndex, i));
-                    } catch (NumberFormatException e) {
-                        // ignore
-                    }
-                    break;
-                }
-            }
-        }
-
-        // this is confusing as usually it's a client-side exception
-        return ERROR_UNKNOWN;
+        Matcher matcher = ERROR_CODE_PATTERN.matcher(errorMessage);
+        // when not match, this is confusing as usually it's a client-side exception
+        return matcher.find() ? Integer.parseInt(matcher.group(1)) : ERROR_UNKNOWN;
     }
 
     static Throwable getRootCause(Throwable t) {
diff --git a/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseExceptionTest.java b/clickhouse-client/src/test/java/com/clickhouse/client/ClickHouseExceptionTest.java
@@ -106,6 +106,31 @@ public void testHandleException() {
         Assert.assertEquals(e.getErrorCode(), 12345);
         Assert.assertEquals(e.getCause(), cause.getCause());
         Assert.assertEquals(e.getMessage(), cause.getCause().getMessage() + ", server " + server);
+
+        cause = new ExecutionException(new IllegalArgumentException("Code:12345. Something goes wrong..."));
+        e = ClickHouseException.of(cause, server);
+        Assert.assertEquals(e.getErrorCode(), 12345);
+        Assert.assertEquals(e.getCause(), cause.getCause());
+        Assert.assertEquals(e.getMessage(), cause.getCause().getMessage() + ", server " + server);
+
+        cause = new ExecutionException(new IllegalArgumentException("Poco::Exception. Code: 1000, "));
+        e = ClickHouseException.of(cause, server);
+        Assert.assertEquals(e.getErrorCode(), 1000);
+        Assert.assertEquals(e.getCause(), cause.getCause());
+        Assert.assertEquals(e.getMessage(), cause.getCause().getMessage() + ", server " + server);
+
+        cause = new ExecutionException(new IllegalArgumentException("Code:       123"));
+        e = ClickHouseException.of(cause, server);
+        Assert.assertEquals(e.getErrorCode(), 123);
+        Assert.assertEquals(e.getCause(), cause.getCause());
+        Assert.assertEquals(e.getMessage(), cause.getCause().getMessage() + ", server " + server);
+
+        cause = new ExecutionException(new IllegalArgumentException("Code: ab123"));
+        e = ClickHouseException.of(cause, server);
+        Assert.assertEquals(e.getErrorCode(), 1002);
+        Assert.assertEquals(e.getCause(), cause.getCause());
+        Assert.assertEquals(e.getMessage(), cause.getCause().getMessage() + ", server " + server);
+
     }
 
     @Test(groups = { "unit" })
