From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (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 60A604696C3 for ; Thu, 16 Apr 2020 11:58:04 +0300 (MSK) References: <831ae724-0d73-916b-0fad-dd52ce967664@tarantool.org> <20200416011118.3jbhcdtqpqr7lwit@tkn_work_nb> From: lvasiliev Message-ID: Date: Thu, 16 Apr 2020 11:58:03 +0300 MIME-Version: 1.0 In-Reply-To: <20200416011118.3jbhcdtqpqr7lwit@tkn_work_nb> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [PATCH v2 1/5] error: Add a Lua backtrace to error List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko , Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the feedback. On 16.04.2020 4:11, Alexander Turenko wrote: >>>> 2) What to do with stacked errors? Currently only the first >>>> error in the stack gets a traceback, because luaT_pusherror() is >>>> called only on the top error in the stack. Consider this test: >>>> >>>>      box.cfg{} >>>>      lua_code = [[function(tuple) >>>>                      local json = require('json') >>>>                      return json.encode(tuple) >>>>                   end]] >>>>      box.schema.func.create('runtimeerror', {body = lua_code, is_deterministic = true, is_sandboxed = true}) >>>>      s = box.schema.space.create('withdata') >>>>      pk = s:create_index('pk') >>>>      idx = s:create_index('idx', {func = box.func.runtimeerror.id, parts = {{1, 'string'}}}) >>>> >>>>      function test_func() return pcall(s.insert, s, {1}) end >>>>      box.error.cfg{traceback_enable = true} >>>>      ok, err = test_func() >>>> >>>> >>>>      tarantool> err:unpack() >>>>      --- >>>>      - traceback: "stack traceback:\n\t[C]: at 0x010014d1b0\n\t[C]: in function 'test_func'\n\t[string >>>>          \"ok, err = test_func()\"]:1: in main chunk\n\t[C]: in function 'pcall'\n\tbuiltin/box/console.lua:382: >>>>          in function 'eval'\n\tbuiltin/box/console.lua:676: in function 'repl'\n\tbuiltin/box/console.lua:725: >>>>          in function " >>>>      ... > > BTW, can we call :split('\n') for .traceback field in at least > __serialize? The cited output is hard to read. Alternative: place two > newlines in row somewhere to force yaml serializer to use multiline > string format. Traceback is absent in __serialize, because it will change the error view for old client. If the client matches result with some pattern it, will be broken. >>>> >>>>      tarantool> err.prev:unpack() >>>>      --- >>>>      - type: LuajitError >>>>        message: '[string "return function(tuple)..."]:2: attempt to call global ''require'' >>>>          (a nil value)' >>>>      ... >>>> >>>> The second error does not have a traceback at all. >>> (I added Turenko to To) >>> I have two variants: >>> - Leave as is and to document such behavior >>> - Add the same traceback to all errors in the stack >>> Alexander what do you think? > > The first approach look inconsistent. A user may want to get a cause of > a topmost error and pass it somewhere. The function, where the error > will be processed (say, serialized), don't know whether a traceback > should be grabbed from some other error object (and how to find it). > Not quite, you either have a traceback or not. Don't try to get it from another error. >> >> There is a third option - leave traceback out of this patchset for >> a next release. Because they are clearly underdesigned. But on the >> other hand it is not something critical. After all, we just can say, >> that a traceback can be present, or can be not, and its content is >> just a string, which can't be assumed to have any special format. >> >> That would allow us to add/remove them and change their format anytime. > > Are there other problems with traceback? > > BTW, how traceback is linked with .trace? > > Generally I'm for decomposing problems. Leonid, you may just keep the > feature at top patches of your series. If it'll look ready, it will > land. Otherwise, only first part will land. > > I think that it is okay to implement marshalling over net.box first. > > WBR, Alexander Turenko. > As I said here 3 separate parts (Custom type, traceback and marshalling) but I don’t want to shuffle the patchset (patches have some dependencies from previous). I suggest just making a decision whether we will add a traceback or not. If not, I will simply remove this patch.