From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp31.i.mail.ru (smtp31.i.mail.ru [94.100.177.91]) (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 7F6134696C3 for ; Sat, 28 Mar 2020 02:11:47 +0300 (MSK) References: From: lvasiliev Message-ID: Date: Sat, 28 Mar 2020 02:11:46 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 0/6] Extending error functionality List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: alexander.turenko@tarantool.org Cc: tarantool-patches@dev.tarantool.org Summary of discussion with Alexander Turenko, Vlad, Mons, Sergos and Kirill Yukhin: 1) Session parameters Move negotiations to UPDATE (as for _vsession_settings). As further optimization session settings can be moved to auth message for to remove an additional RTT 2) Backtrace Leave adding to the error only Lua backtrace (which is added to the error at luaT_pusherror()). We must have a per server option for enable/disable adding backtrace. In addition a bactrace enable/disable adding can be forced by the creation function parametr. The backtrace should be combined with the message if tostring is used.(@knazarov ) Notes: To add a C bt is possible when error is created (for example) but unwind the Lua stack in diag.c is difficult (reason: the lua functions like lua_getstack, lua_getinfo are unavailable where) 3) About returning(return box.error.new(...)) an error through IPROTO Now the error is serialized to string for marshaling through connection, which makes it difficult to add various error parameters. In the first iteration format was changed to table. But after discussion was decided to add a new msgpack type for the errors transmitting(by analogy with MP_DECIMAL) An intermediate type mp_error will be introduced for this purposes (reason: it is impossible to create an object of the ClientError class (or some other classes) at the time of decoding) Due to the need to use different serialization methods, depending on the session settings, the serializator context (containing fickle options) was added. 4) Payload For storage the payload at struct error will be added ptr to data with data size. For using a payload from Lua set/get_payload (worked with table as argument/return type) will be used. It is possible to use msgpack encode when set and decode on get, but not is rationally. To reduce overhead is offered to save a link to the table and use it as response output. For these purposes is proposed to use inheritance from the struct error in the C style with adding additional attributes like ptr to the Lua table. In the case of a marshaling through net the payload will be encoded/decoded with msgpack using. On 24.03.2020 15:45, Leonid Vasiliev wrote: > https://github.com/tarantool/tarantool/issues/4398 > https://github.com/tarantool/tarantool/tree/lvasiliev/gh-4398-expose-error-module-4 > > According to https://github.com/tarantool/tarantool/issues/4398 > (and after some discussion) we would like box.error to have: > * Ability to create new error types > * Transparent marshalling through net.box > * Lua backtrace > * payload (not implemented in patch) > > > Leonid Vasiliev (6): > error: Add a Lua backtrace to error > error: Add the custom error type > iproto: Add negotiation phase > error: Add extended error transfer format > error: Add test for extended error > error: Transmit an error through IPROTO_OK as object > > src/box/errcode.h | 1 + > src/box/error.cc | 56 +++++++++ > src/box/error.h | 32 ++++++ > src/box/execute.c | 1 + > src/box/iproto.cc | 40 ++++++- > src/box/iproto_constants.h | 25 +++++ > src/box/lua/call.c | 29 +++-- > src/box/lua/error.cc | 126 +++++++++++++++++---- > src/box/lua/execute.c | 2 +- > src/box/lua/net_box.c | 34 ++++++ > src/box/lua/net_box.lua | 103 +++++++++++++++-- > src/box/lua/tuple.c | 12 +- > src/box/session.cc | 16 +++ > src/box/session.h | 20 ++++ > src/box/sql/func.c | 4 +- > src/box/xrow.c | 160 ++++++++++++++++++++++++++ > src/box/xrow.h | 48 +++++++- > src/lib/core/diag.c | 20 ++++ > src/lib/core/diag.h | 5 + > src/lib/core/exception.cc | 1 + > src/lib/core/mp_extension_types.h | 4 +- > src/lib/core/port.h | 4 +- > src/lua/error.c | 45 +++++++- > src/lua/error.h | 6 +- > src/lua/error.lua | 65 ++++++++--- > src/lua/msgpack.c | 26 +++-- > src/lua/msgpack.h | 8 +- > src/lua/utils.c | 28 ++++- > src/lua/utils.h | 27 ++++- > test/app/fiber.result | 35 ++++-- > test/app/fiber.test.lua | 11 +- > test/box-tap/extended_error.test.lua | 212 +++++++++++++++++++++++++++++++++++ > test/box/misc.result | 51 +++++++-- > test/box/misc.test.lua | 19 +++- > 34 files changed, 1161 insertions(+), 115 deletions(-) > create mode 100755 test/box-tap/extended_error.test.lua >