From: Nikita Pettik <korablev@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH] box: rfc for stacked diagnostic area Date: Thu, 6 Feb 2020 20:24:14 +0300 [thread overview] Message-ID: <20200206172414.GA27490@tarantool.org> (raw) In-Reply-To: <d1dcffd0-efb2-122c-043e-f6a5430bbea9@tarantool.org> On 05 Feb 23:07, Vladislav Shpilevoy wrote: > Thanks for the fix! > > On 05/02/2020 07:16, Nikita Pettik wrote: > > On 04 Feb 21:48, Vladislav Shpilevoy wrote: > >> Thanks for the RFC. > >> > >> It still does not conform with the template, but ok. I see that > >> that ship has sailed already, some other RFCs also violate the > >> template. > >> > >> See 2 comments below. > >> > >>>> 10. Are we not going to allow to link two existing errors? I imagine > >>>> it could be simpler and more flexible for a user, than filling > >>>> one big map in error.new(). > >>> > >>> Okay, I'm not against it: > >>> > >>> Another way to resolve this issue is to erase diagnostic area before > >>> @@ -173,13 +158,23 @@ box.error.prev(error) == error.prev > >>> ``` > >>> > >>> Furthermore, let's extend signature of `box.error.new()` with new (optional) > >>> -argument - the 'reason' parent error object: > >>> +argument - 'prev' - previous error object: > >>> > >>> ``` > >>> e1 = box.error.new({code = 111, reason = "just cause"}) > >>> e2 = box.error.new({code = 222, reason = "just cause x2", prev = e1}) > >>> ``` > >>> > >>> +User may want to link already existing errors. To achieve this let's add > >>> +`set_prev` method to error object or/and `link` to `box.error` so that one can > >>> +join two errors: > >>> +``` > >>> +e1 = box.error.new({code = 111, reason = "just cause"}) > >>> +e2 = box.error.new({code = 222, reason = "just cause x2"}) > >>> +... > >>> +e2.set_prev(e1) -- e2.prev == e1 > >>> +box.error.link(e1, e2) -- e2.prev == e1 > >>> +``` > >> > >> 1. I don't think we need to change box.error global API. It would be > >> enough to add new methods to error object. box.error.link() and > >> box.error.prev() look redundant. What is a case, when they should > >> be used instead of error object methods? > >> > >> box.error.new() and last() exist because there is no other way to > >> create an error, or to get a last one. > > > > I've added both since was not sure which one is better. > > Since you'd prefer avoid changing global interface (which is > > reasonable argument) let's leave only e:set_prev(): > > > > diff --git a/doc/rfc/1148-stacked-diagnostics.md b/doc/rfc/1148-stacked-diagnostics.md > > index d57e040ba..ed121770d 100644 > > --- a/doc/rfc/1148-stacked-diagnostics.md > > +++ b/doc/rfc/1148-stacked-diagnostics.md > > @@ -178,14 +178,12 @@ e2 = box.error.new({code = 222, reason = "just cause x2", prev = e1}) > > ``` > > > > User may want to link already existing errors. To achieve this let's add > > -`set_prev` method to error object or/and `link` to `box.error` so that one can > > -join two errors: > > +`set_prev` method to error object so that one can join two errors: > > ``` > > e1 = box.error.new({code = 111, reason = "just cause"}) > > e2 = box.error.new({code = 222, reason = "just cause x2"}) > > ... > > e2.set_prev(e1) -- e2.prev == e1 > > -box.error.link(e1, e2) -- e2.prev == e1 > > ``` > > ### Binary protocol > > What about box.error.prev()? I don't think we need this one as > well. How will it work anyway? You just call box.error.prev() > multiple times and iterate over the error list? Or it can only > return the second error in the stack. Both ways looks not really > useful. Probably, error:prev() is enough. Okay, I don't mind this change: diff --git a/doc/rfc/1148-stacked-diagnostics.md b/doc/rfc/1148-stacked-diagnostics.md index ed121770d..0e365361b 100644 --- a/doc/rfc/1148-stacked-diagnostics.md +++ b/doc/rfc/1148-stacked-diagnostics.md @@ -159,14 +159,14 @@ Tarantool returns a last-set (diag::last) error as `cdata` object from central diagnostic area to Lua in case of error. User should be unable to modify it (since it is considered to be a bad practice - in fact object doesn't belong to user). On the other hand, user needs an ability to inspect a collected -diagnostic information. Hence, let's extend the `box.error` API with a function +diagnostic information. Hence, let's extend the error object API with a method which provides the way to get the previous error (reason): `:prev()` (and correspondingly `.prev` field). ``` --- Return a reason error object for given error +-- Return a reason error object for given error object 'e' -- (when exists, nil otherwise). -box.error.prev(error) == error.prev +e:prev(error) == error.prev ```
next prev parent reply other threads:[~2020-02-06 17:24 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-01-14 20:16 Nikita Pettik 2020-01-22 13:54 ` Nikita Pettik 2020-01-22 23:07 ` Vladislav Shpilevoy 2020-01-23 22:56 ` Vladislav Shpilevoy 2020-01-27 11:23 ` Nikita Pettik 2020-01-28 0:23 ` Vladislav Shpilevoy 2020-01-29 14:36 ` Nikita Pettik 2020-02-04 20:48 ` Vladislav Shpilevoy 2020-02-05 6:16 ` Nikita Pettik 2020-02-05 22:07 ` Vladislav Shpilevoy 2020-02-06 17:24 ` Nikita Pettik [this message] 2020-02-06 20:48 ` Vladislav Shpilevoy
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=20200206172414.GA27490@tarantool.org \ --to=korablev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] box: rfc for stacked diagnostic area' \ /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