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

[Vladislav Shpilevoy]

Thanks for the fixes!

>> 6. Make_type() perfectly fits in one line.
>>
>>> +
>>> +CustomError::CustomError(const char *file, unsigned int line,
>>> +             const char *custom_type)
>>> +    :ClientError(&type_CustomError, file, line, ER_CUSTOM_ERROR)
>>> +{
>>> +    error_format_msg(this, tnt_errcode_desc(m_errcode),
>>> +             custom_type ?: "");
>>
>> 7. custom_type is never NULL.
> Why? It's a constructor argument.

And still it is never NULL. I couldn't find any place, where
you passed NULL here, and anyway that would be incorrect. Type
can't be just NULL. How to present it in Lua then? nil? Doubt
it.

>>> + */
>>> +const char *
>>> +box_custom_error_type(const box_error_t *e);
>>> +
>>> +/**
>>>    * Add error to the diagnostic area. In contrast to box_error_set()
>>>    * it does not replace previous error being set, but rather link
>>>    * them into list.
>>> @@ -155,15 +166,24 @@ box_error_add(const char *file, unsigned line, uint32_t code,
>>>     /**
>>>    * Construct error object without setting it in the diagnostics
>>> - * area. On the memory allocation fail returns NULL.
>>> + * area. On the memory allocation fail returns  OutOfMemory error.
>>
>> 10. Unneccessary diff.
> The comment is wrong. How I can fix it?

Yeah, I just realized that after you said box_error_new() never
returns NULL. Then probably it is ok to change it.

>>
>>>    */
>>>   struct error *
>>>   box_error_new(const char *file, unsigned line, uint32_t code,
>>>             const char *fmt, ...);
>>>   +/**
>>> + * Construct error object without setting it in the diagnostics
>>> + * area. On the memory allocation fail returns OutOfMemory error.
>>
>> 11. This is a copy-paste box box_error_new() comment. Better write
>> here a couple of words how is it different. Or better merge box_error_new()
>> and box_custom_error_new(). Because in Lua I can specify both code
>> and type. Don't see why can't I do this in C.
> I thought we can’t change public API.

This is not public API. If something is not compiled into module.h,
it is not public (in C. For Lua rules are different).

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