[GitHub] [ignite-3] valepakh commented on a diff in pull request #1024: IGNITE-17090 Show user-friendly message in sql command

[GitBox <gi...@apache.org>]

valepakh commented on code in PR #1024:
URL: https://github.com/apache/ignite-3/pull/1024#discussion_r954738272


##########
modules/cli/src/main/java/org/apache/ignite/cli/core/exception/handler/SqlExceptionHandler.java:
##########
@@ -60,13 +109,24 @@ public int handle(ExceptionWriter err, SQLException e) {
                 break;
             default:
                 LOG.error("Unrecognized error", e);
-                errorComponentBuilder.header("Unrecognized error while process SQL query");
+                errorComponentBuilder.header("SQL query execution error").details(e.getMessage());
         }
 
         err.write(errorComponentBuilder.build().render());
         return 1;
     }
 
+    /** Handles IgniteException that has more information like error code and trace id. */
+    public int handleIgniteException(ExceptionWriter err, IgniteException e) {
+        Function<IgniteException, ErrorComponentBuilder> errorComponentBuilder = sqlExceptionMappers.getOrDefault(
+                e.code(), defaultErrorMapper);

Review Comment:
   ```suggestion
           var errorComponentBuilder = sqlExceptionMappers.getOrDefault(e.code(), defaultErrorMapper);
   ```



##########
modules/cli/src/main/java/org/apache/ignite/cli/core/exception/handler/SqlExceptionHandler.java:
##########
@@ -39,8 +48,48 @@ public class SqlExceptionHandler implements ExceptionHandler<SQLException> {
 
     public static final String CONNECTION_BROKE_MESSAGE = "Connection error";
 
+    private final Map<Integer, Function<IgniteException, ErrorComponentBuilder>> sqlExceptionMappers = new HashMap<>();
+
+    private final Function<IgniteException, ErrorComponentBuilder> defaultErrorMapper =
+            (e) -> ErrorUiComponent.builder().header("Unrecognized error while process SQL query" + e.getMessage());
+
+    /** Default constructor. */
+    public SqlExceptionHandler() {
+        sqlExceptionMappers.put(Client.CONNECTION_ERR, this::connectionErrUiComponent);
+        sqlExceptionMappers.put(Sql.QUERY_INVALID_ERR, this::invalidQueryErrUiComponent);
+    }
+
+    private ErrorComponentBuilder invalidQueryErrUiComponent(IgniteException e) {
+        return ErrorUiComponent.builder()
+                .header(PARSING_ERROR_MESSAGE)
+                .errorCode(e.codeAsString())
+                .traceId(e.traceId())
+                .details(ErrorGroup.extractCauseMessage(e.getMessage()));
+    }
+
+    private ErrorComponentBuilder connectionErrUiComponent(IgniteException e) {
+        if (e.getCause() instanceof IgniteClientConnectionException) {
+            IgniteClientConnectionException clientConnectionException = (IgniteClientConnectionException) e.getCause();
+            return ErrorUiComponent.builder()
+                    .header(CLIENT_CONNECTION_FAILED_MESSAGE)
+                    .errorCode(clientConnectionException.codeAsString())
+                    .traceId(clientConnectionException.traceId())
+                    .details(ErrorGroup.extractCauseMessage(clientConnectionException.getMessage()));
+        }
+
+        return ErrorUiComponent.builder()
+                .header(CLIENT_CONNECTION_FAILED_MESSAGE)
+                .errorCode(e.codeAsString())
+                .traceId(e.traceId())
+                .details(ErrorGroup.extractCauseMessage(e.getMessage()));

Review Comment:
   Looks like there is a possibility to extract a method for IgniteCliException with custom header.



##########
modules/cli/src/main/java/org/apache/ignite/cli/core/exception/handler/SqlExceptionHandler.java:
##########
@@ -39,8 +48,48 @@ public class SqlExceptionHandler implements ExceptionHandler<SQLException> {
 
     public static final String CONNECTION_BROKE_MESSAGE = "Connection error";
 
+    private final Map<Integer, Function<IgniteException, ErrorComponentBuilder>> sqlExceptionMappers = new HashMap<>();
+
+    private final Function<IgniteException, ErrorComponentBuilder> defaultErrorMapper =
+            (e) -> ErrorUiComponent.builder().header("Unrecognized error while process SQL query" + e.getMessage());

Review Comment:
   Maybe use a method similar to invalidQueryErrUiComponent to extract code, traceId and cause message?
   Also "process"->"processing"



-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org