From: Alexander Turenko <alexander.turenko@tarantool.org> To: lvasiliev <lvasiliev@tarantool.org> Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 1/5] error: Add a Lua backtrace to error Date: Thu, 16 Apr 2020 15:45:48 +0300 [thread overview] Message-ID: <20200416124548.66rhbezr7ve6vx7o@tkn_work_nb> (raw) In-Reply-To: <ab329357-c6df-0057-407e-2964bde8c638@tarantool.org> On Thu, Apr 16, 2020 at 03:27:35PM +0300, lvasiliev wrote: > > > 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 <builtin/box/console.lua:713>" > > > > > > > ... <snipped> > > > > > > > > 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. As we discussed, a serializer should not see at __serialize of a type it knows natively. Let's keep __serialize for REPL only. > > > > > > > > > > > > > > > > > tarantool> err.prev:unpack() > > > > > > > --- > > > > > > > - type: LuajitError > > > > > > > message: '[string "return function(tuple)..."]:2: attempt to call global ''require'' > > > > > > > (a nil value)' > > > > > > > ... <snipped> > > > > > > > > > > > > > > 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. So we anyway need to docuemnt a rule when an error will contain the traceback and the way you implement it does not add much complexity. I can buy this point. > > > > 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. I didn't review the patchset, just looked at the question, so I cannot insist here. Personally I have questions around traceback (see [1]), so I would prefer safe way: concentrate on marshaling, postpone traceback. However I don't feel myself in power to decide. [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/016043.html
next prev parent reply other threads:[~2020-04-16 12:45 UTC|newest] Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-10 8:10 [Tarantool-patches] [PATCH v2 0/5] Extending error functionality Leonid Vasiliev 2020-04-10 8:10 ` [Tarantool-patches] [PATCH v2 1/5] error: Add a Lua backtrace to error Leonid Vasiliev 2020-04-14 1:11 ` Vladislav Shpilevoy 2020-04-15 9:25 ` lvasiliev 2020-04-16 0:00 ` Vladislav Shpilevoy 2020-04-16 1:11 ` Alexander Turenko 2020-04-16 8:58 ` lvasiliev 2020-04-16 9:30 ` Alexander Turenko 2020-04-16 12:27 ` lvasiliev 2020-04-16 12:45 ` Alexander Turenko [this message] 2020-04-16 17:48 ` lvasiliev 2020-04-16 8:40 ` lvasiliev 2020-04-16 9:04 ` lvasiliev 2020-04-10 8:10 ` [Tarantool-patches] [PATCH v2 2/5] error: Add the custom error type Leonid Vasiliev 2020-04-14 1:11 ` Vladislav Shpilevoy 2020-04-15 9:25 ` lvasiliev 2020-04-16 0:02 ` Vladislav Shpilevoy 2020-04-16 9:18 ` lvasiliev 2020-04-16 21:03 ` Vladislav Shpilevoy 2020-04-10 8:10 ` [Tarantool-patches] [PATCH v2 3/5] error: Increase the number of fields transmitted through IPROTO Leonid Vasiliev 2020-04-14 1:12 ` Vladislav Shpilevoy 2020-04-15 9:25 ` lvasiliev 2020-04-16 0:02 ` Vladislav Shpilevoy 2020-04-10 8:10 ` [Tarantool-patches] [PATCH v2 4/5] iproto: Add session settings for IPROTO Leonid Vasiliev 2020-04-14 1:12 ` Vladislav Shpilevoy 2020-04-15 9:26 ` lvasiliev 2020-04-16 0:06 ` Vladislav Shpilevoy 2020-04-16 9:36 ` lvasiliev 2020-04-16 21:04 ` Vladislav Shpilevoy 2020-04-10 8:10 ` [Tarantool-patches] [PATCH v2 5/5] iproto: Update error MsgPack encoding Leonid Vasiliev 2020-04-14 1:12 ` Vladislav Shpilevoy 2020-04-15 9:25 ` lvasiliev 2020-04-16 0:11 ` Vladislav Shpilevoy 2020-04-16 10:04 ` lvasiliev 2020-04-16 21:06 ` Vladislav Shpilevoy 2020-04-14 1:10 ` [Tarantool-patches] [PATCH v2 0/5] Extending error functionality Vladislav Shpilevoy 2020-04-15 9:48 ` lvasiliev
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200416124548.66rhbezr7ve6vx7o@tkn_work_nb \ --to=alexander.turenko@tarantool.org \ --cc=lvasiliev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 1/5] error: Add a Lua backtrace to error' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox