From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp5.mail.ru (smtp5.mail.ru [94.100.179.24]) (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 9611F4696C3 for ; Thu, 16 Apr 2020 15:27:36 +0300 (MSK) References: <831ae724-0d73-916b-0fad-dd52ce967664@tarantool.org> <20200416011118.3jbhcdtqpqr7lwit@tkn_work_nb> <20200416093037.vgtdm6xrto7g6qxb@tkn_work_nb> From: lvasiliev Message-ID: Date: Thu, 16 Apr 2020 15:27:35 +0300 MIME-Version: 1.0 In-Reply-To: <20200416093037.vgtdm6xrto7g6qxb@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 Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy On 16.04.2020 12:30, Alexander Turenko wrote: > 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. > Now __serialize is used for transfer old style error too. >> >>>>>> >>>>>>      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. No, the error without traceback is not 'partial'. If global error_is_traceback_enabled is false - all errors haven't a traceback. If error creates with traceback=false, it hasn't a traceback. > > 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'. > You are hyperbolizing. It can be regarded as "Technical debt". If you insist, I can remove this patch.