From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 34DB24696C3 for ; Sat, 18 Apr 2020 23:39:51 +0300 (MSK) References: <4b3ee41137cefd5d01a34763d33be897ec5f8c09.1587223627.git.lvasiliev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <3b5898e4-c2bc-2704-ce63-e492799e4231@tarantool.org> Date: Sat, 18 Apr 2020 22:39:49 +0200 MIME-Version: 1.0 In-Reply-To: <4b3ee41137cefd5d01a34763d33be897ec5f8c09.1587223627.git.lvasiliev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH V5 2/6] error: send custom type in IProto List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Vasiliev , alexander.turenko@tarantool.org Cc: tarantool-patches@dev.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 > > 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 = })` or > `box.error.new()`. 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 : [ { IPROTO_ERROR_CODE: , IPROTO_ERROR_MESSAGE: , }, ... ] ``` MP_ERROR format is going to be this: ``` : MP_ERROR : { MP_ERROR_STACK : [ { MP_ERROR_TYPE: , MP_ERROR_FILE: , MP_ERROR_LINE: , MP_ERROR_REASON: , MP_ERROR_ERRNO: , MP_ERROR_FIELDS: { : ..., : ..., ... }, }, ... } } ``` 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 : { IPROTO_ERROR_STACK : [ { IPROTO_ERROR_TYPE: , IPROTO_ERROR_FILE: , IPROTO_ERROR_LINE: , IPROTO_ERROR_REASON: , IPROTO_ERROR_ERRNO: , IPROTO_ERROR_FIELDS: { : ..., : ..., ... }, }, ... } } ``` + `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_` == `MP_ERROR_`. 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`.