Re: [Tarantool-patches] [PATCH V4 2/6] error: send custom type in IProto

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

Thanks for the patch!

The commit message still contains plural verbs and nouns.
I changed it to:

====================
    Error custom type feature was added to the public Lua API in the
    previous commit. This one makes the new attribute being sent in
    IProto.
    
    Co-authored-by: Vladislav Shpilevoy<v.shpilevoy@tarantool.org>
    
    Needed for #4398
    
    @TarantoolBot document
    Title: New error object attribute in IProto
    
    Error objects in IProto already have 2 fields:
    `IPROTO_ERROR_CODE = 0x01` and `IPROTO_ERROR_MESSAGE = 0x02`.
    
    Now added:
    
    `IPROTO_ERROR_CUSTOM_TYPE = 0x03`.
    
    It's optional, has MP_STR type, and speaks for itself.
    Custom error type is error object attribute.
    This is what a user specifies in
    `box.error.new({type = <custom_type>})` or
    `box.error.new(<custom_type>)`.
====================

See 5 comments below. Some of them I fixed by force push, but
forgot to extract diff first, sorry.

On 16/04/2020 19:38, Leonid Vasiliev wrote:
> Error custom type features were added to the public
> Lua API in the previous commits. This one makes the new attribute
> being sent in IProto.
> 
> Co-authored-by: Vladislav Shpilevoy<v.shpilevoy@tarantool.org>
> 
> Needed for #4398
> 
> @TarantoolBot document
> Title: New error object attributes in IProto
> 
> Error objects in IProto already have 2 fields:
> `IPROTO_ERROR_CODE = 0x01` and `IPROTO_ERROR_MESSAGE = 0x02`.
> 
> Now added:
> 
> `IPROTO_ERROR_CUSTOM_TYPE = 0x03`.
> 
> It's optional, have MP_STR type, and speak for themselves.
> Custom error type is error object attribute.
> This is what a user specifies in
> `box.error.new({type = <custom_type>})` or
> `box.error.new(<custom_type>)`.
> ---
>  extra/exports              |  1 +
>  src/box/iproto_constants.h |  1 +
>  src/box/lua/net_box.lua    | 14 +++++++++++---
>  src/box/xrow.c             |  9 ++++++++-
>  test/box/error.result      | 40 ++++++++++++++++++++++++++++++++++++++++
>  test/box/error.test.lua    | 17 +++++++++++++++++
>  6 files changed, 78 insertions(+), 4 deletions(-)
> 
> diff --git a/extra/exports b/extra/exports
> index 9467398..b01b437 100644
> --- a/extra/exports
> +++ b/extra/exports
> @@ -242,6 +242,7 @@ box_error_last
>  box_error_clear
>  box_error_set
>  error_set_prev
> +error_set_traceback

1. Garbage left from remove traceback patch.

> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index 07fa54c..95f9cf8 100644
> --- a/src/box/lua/net_box.lua
> +++ b/src/box/lua/net_box.lua
> @@ -10,6 +10,11 @@ local urilib   = require('uri')
>  local internal = require('net.box.lib')
>  local trigger  = require('internal.trigger')
>  
> +ffi.cdef[[
> +void
> +error_set_traceback(struct error *e, const char *traceback);
> +]]

2. The same.

> diff --git a/test/box/error.result b/test/box/error.result
> index 82db453..d231f5d 100644
> --- a/test/box/error.result
> +++ b/test/box/error.result
> @@ -889,3 +889,43 @@ e = box.error.new({type = string.rep('a', 128)})
>   | ---
>   | - 63
>   | ...
> +
> +--
> +-- Check how custom error type are passed through

3. Still uses plural.

> +-- IProto.
> +--
> +netbox = require('net.box')
> + | ---
> + | ...
> +box.schema.user.grant('guest', 'super')
> + | ---
> + | ...
> +c = netbox.connect(box.cfg.listen)
> + | ---
> + | ...
> +_, e = pcall(c.call, c, 'box.error', {                                          \
> +    {code = 123, type = 'TestType', reason = 'Test reason'}   \

4. '\' symbols were aligned in my version of the patch.

> +})
> + | ---
> + | ...
> +e = e:unpack()
> + | ---
> + | ...
> +e.trace = nil
> + | ---
> + | ...
> +e
> + | ---
> + | - code: 214

5. The code was '123' in the call above, but here it is 214. The
code was silently ignored. This is what I am talking about in the
previous commit's discussion.

> + |   base_type: CustomError
> + |   type: TestType
> + |   custom_type: TestType
> + |   message: Test reason
> + | ...