From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp14.mail.ru (smtp14.mail.ru [94.100.181.95]) (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 162F44696C3 for ; Fri, 17 Apr 2020 03:51:36 +0300 (MSK) References: <4d28379a84646a41f9a02677a176fd1ee037a0b8.1587058424.git.lvasiliev@tarantool.org> From: Vladislav Shpilevoy Message-ID: <6ceace5c-1d73-3e2f-1724-36002de1c4ba@tarantool.org> Date: Fri, 17 Apr 2020 02:51:33 +0200 MIME-Version: 1.0 In-Reply-To: <4d28379a84646a41f9a02677a176fd1ee037a0b8.1587058424.git.lvasiliev@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH V4 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 Cc: tarantool-patches@dev.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 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()`. ==================== 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 > > 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 = })` or > `box.error.new()`. > --- > 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 > + | ...