[GitHub] [ignite-3] ibessonov commented on a diff in pull request #904: IGNITE-14931 Added error groups

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

ibessonov commented on code in PR #904:
URL: https://github.com/apache/ignite-3/pull/904#discussion_r911706242


##########
modules/core/src/main/java/org/apache/ignite/lang/IgniteInternalCheckedException.java:
##########
@@ -26,29 +34,182 @@ public class IgniteInternalCheckedException extends Exception {
     /** Serial version uid. */
     private static final long serialVersionUID = 0L;
 
+    /** Name of the error group. */
+    private final String groupName;
+
+    /**
+     * Error code which contains information about error group and code, where code is unique within the group.
+     * The structure of a code is shown in the following diagram:
+     * +------------+--------------+
+     * |  16 bits   |    16 bits   |
+     * +------------+--------------+
+     * | Group Code |  Error Code  |
+     * +------------+--------------+
+     */
+    private final int code;
+
+    /** Unique identifier of this exception that should help locating the error message in a log file. */
+    private final UUID traceId;
+
+    /**
+     * Creates a new exception with the given group and error code.
+     *
+     * @param groupName Group name.
+     * @param code Full error code.
+     */
+    public IgniteInternalCheckedException(String groupName, int code) {

Review Comment:
   I believe that internal exception could have ErrorGroup as a parameter, what do you think?



##########
modules/client/src/test/java/org/apache/ignite/client/ClientTupleTest.java:
##########
@@ -94,16 +96,16 @@ public void testEmptySchemaThrows() {
     @Test
     public void testSetThrowsWhenColumnIsNotPresent() {
         var ex = assertThrows(IgniteException.class, () -> getBuilder().set("x", "y"));

Review Comment:
   BTW, should we assert a more specific type here? We do have a right subclass here



##########
modules/api/src/main/java/org/apache/ignite/lang/ColumnAlreadyExistsException.java:
##########
@@ -27,6 +30,6 @@ public class ColumnAlreadyExistsException extends IgniteException {
      * @param name Column name.
      */
     public ColumnAlreadyExistsException(String name) {

Review Comment:
   It's weird that exceptions like these are in "lang" module. Makes no sense. We should fix it in the future.



-- 
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