Re: [Tarantool-patches] [PATCH v2 2/5] error: Add the custom error type

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi! Thanks for the answers!

>>>>> @@ -111,9 +148,16 @@ raise:
>>>>>            line = info.currentline;
>>>>>        }
>>>>>    -    struct error *err = box_error_new(file, line, code, "%s", reason);
>>>>> -    if (tb_parsed)
>>>>> -        err->traceback_mode = tb_mode;
>>>>> +    struct error *err = NULL;
>>>>> +    if (args.custom) {
>>>>> +        err = box_custom_error_new(file, line, args.custom,
>>>>> +                       "%s", args.reason);
>>>>
>>>> 18. Code argument is ignored here. Although I am not sure it shouldn't
>>>> be ignored. From what I remember, we decided that code should stay,
>>>> but I may be wrong.
>>> CustomError type has a predefinded code. Why it shouldn't be ignored?
>>
>> Because it is not a 'ClientError' object from user's point of view. The
>> fact that we implemented it as a descendant of 'ClientError' class should
>> not leak into the API. User can't assume anything about error hierarchies.
>> He does not even see them.
>>
>>  From user's point of view there are separate built-in 'ClientError'. And
>> separate 'CustomError'. Both accept error code now. But 'CustomError'
>> ignores it somewhy. Does not look right.
>>
> No, from user's point of view it have only type without any codes (In accordance with the task).

The task says, I cite: "Ability to skip error codes if we don't need them".
Not drop them. But skip when don't need. So essentially - make the code an
optional parameter.

> Besides, I think it's strange to have an AccessDeniedError with code "ER_SQL_PREPARE" (other branch of hierarchy).

How AccessDeniedError is related to that? It is a different error type. Not
ClientError, not CustomError. It does not have a code at all.

> And user doesn't have an access to "code" of error.

Well, they actually have. This is master branch:

    tarantool> e = box.error.new(1000, 'Message')
    ---
    ...

    tarantool> e.code
    ---
    - 1000
    ...

Talking of "user's point of view" regarding codes: ClientError has one
meaning for error code. CustomError can have another. When you will add
payload, users will add their codes for sure. Error code is something
very basic for any more or less general error type. And if some of them
will match our codes, it does not mean they have the same meaning.

Some error types don't have a code - they type is enough. Such as
AccessDeniedError.

box.error.* codes only valid for ClientError objects.

CustomError does not have any mapping of error codes. A user can define
it.