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

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

On 18/04/2020 17:29, Leonid Vasiliev wrote:
> 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>)`.

I added a note, that the field is optional:

    The field is optional, and may be not present in response, if the
    error does not have a custom type.

Talking of the patch - I still think IProto errors should reuse the
code used to encode/decode MP_EXT errors. If we go for that, this patch
is going to be 100% overrode by that refactoring, and does not make
sense as a separate commit.

There appeared a problem, that it is not that simple, since netbox
is not able to decode MP_EXT for now, as Leonid noticed. So there is
a workaround.

We discussed it verbally, below is the solution.

Now we have `IPROTO_ERROR_STACK` of a format:
```
IPROTO_ERROR_STACK <MP_ARRAY>: [
    <MP_MAP> {
        IPROTO_ERROR_CODE: <MP_UINT>,
        IPROTO_ERROR_MESSAGE: <MP_STR>,
    },
    ...
]
```
MP_ERROR format is going to be this:
```
    <MP_EXT>:
        MP_ERROR <MP_MAP>: {
            MP_ERROR_STACK <MP_ARRAY>: [
                <MP_MAP> {
                    MP_ERROR_TYPE: <MP_STR>,
                    MP_ERROR_FILE: <MP_STR>,
                    MP_ERROR_LINE: <MP_UINT>,
                    MP_ERROR_REASON: <MP_STR>,
                    MP_ERROR_ERRNO: <MP_UINT>,
                    MP_ERROR_FIELDS: <MP_MAP> {
                        <MP_STR>: ...,
                        <MP_STR>: ...,
                        ...
                    },
                },
                ...
            }
        }
```

I propose to make these formats the same, not counting `MP_EXT`.
For that we rename the old `IPROTO_ERROR` to `IPROTO_ERROR_24`
(similar to `IPROTO_CALL_16`), `IPROTO_ERROR_STACK` rename to
`IPROTO_ERROR`. Renames are ok, since the underlying numbers
are not going to change. So IProto error will look like this:
```
IPROTO_ERROR <MP_MAP>: {
    IPROTO_ERROR_STACK <MP_ARRAY>: [
        <MP_MAP> {
            IPROTO_ERROR_TYPE: <MP_STR>,
            IPROTO_ERROR_FILE: <MP_STR>,
            IPROTO_ERROR_LINE: <MP_UINT>,
            IPROTO_ERROR_REASON: <MP_STR>,
            IPROTO_ERROR_ERRNO: <MP_UINT>,
            IPROTO_ERROR_FIELDS: <MP_MAP> {
                <MP_STR>: ...,
                <MP_STR>: ...,
                ...
            },
        },
        ...
    }
}
```
+ `IPROTO_ERROR_24`, like it is done in the stacked diag now.
Essentially, it looks exactly the same as `MP_ERROR` - `MP_EXT`
header.

Then we do that all keys `IPROTO_ERROR_*` and `MP_ERROR_*` have
the same values. So any `IPROTO_ERROR_<name>` == `MP_ERROR_<name>`.
For example, `MP_ERROR_STACK = 0x00`, and `IPROTO_ERROR_STACK = 0x00`.
 
Now all packing/unpacking can be done by the same functions for
IProto and for normal returns. IProto will encode it as `MP_MAP` in
its header, normal serialization will be via `MP_EXT`.