From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp55.i.mail.ru (smtp55.i.mail.ru [217.69.128.35]) (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 8C2C64696C3 for ; Thu, 16 Apr 2020 12:30:43 +0300 (MSK) Date: Thu, 16 Apr 2020 12:30:37 +0300 From: Alexander Turenko Message-ID: <20200416093037.vgtdm6xrto7g6qxb@tkn_work_nb> References: <831ae724-0d73-916b-0fad-dd52ce967664@tarantool.org> <20200416011118.3jbhcdtqpqr7lwit@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: 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: lvasiliev Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy On Thu, Apr 16, 2020 at 11:58:03AM +0300, lvasiliev wrote: > 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. It is better to keep __serialize on track with newly added fields. I would not bother with possibility that someone may call __serialize manually or capture console output to compare against a sample. Extending a map should be okay from backward-compatibility point of view. > > > > > > > > > > >      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. You propose to introduce some kind of 'full' and 'partial' errors. It is hard to document, because there is no rationale for this. When something is introduced, it should be for the sake of something. A kind of 'the API is complex, but, I see, it is highly flexible' or 'here I should take care of this peculiar, but OTOH some cases may be processed much faster due to this'.