[Tarantool-patches] [PATCH v2 05/10] box/error: don't set error created via box.error.new to diag
Nikita Pettik
korablev at tarantool.org
Thu Mar 26 20:59:15 MSK 2020
On 26 Mar 19:50, Konstantin Osipov wrote:
> * Nikita Pettik <korablev at tarantool.org> [20/03/25 09:32]:
> > To achieve this let's refactor luaT_error_create() to return error
> > object instead of setting it via box_error_set().
> > luaT_error_create() is used both to handle box.error() and
> > box.error.new() invocations, and box.error() is still expected to set
> > error to diagnostic area. So, luaT_error_call() which implements
> > box.error() processing at the end calls diag_set_error().
> > It is worth mentioning that net.box module relied on the fact that
> > box.error.new() set error to diagnostic area: otherwise request errors
> > don't get to diagnostic area on client side.
> >
>
> I know there was a discussion about construct vs create vs new.
It was not really discussion, I accidentally didn't notice that
box_error_construct() in fact allocated memory (meanwhile I thought
it didn't). I've already renamed it to box_error_new().
> Let me remind about approved name pairs:
>
> obj_create/obj_destroy - constructor/destructor, without
> allocation/deallocation.
>
> obj_new/obj_delete - constructor/destructor with
> allocation/deallocation
>
> init/free - library initialization/destruction
>
> _construct is not in the vocabulary. I'd avoid it.
Ok, Vlad already guided me (but it would be nice to see this
manual in our dev docs).
> I know this is bikeshed, pls feel free to ignore in scope of this
> patch. And sorry for jumping in late.
>
> > +struct error *
> > +box_error_construct(const char *file, unsigned line, uint32_t code,
> > + const char *fmt, ...)
> > +{
> > + struct error *e = BuildClientError(file, line, ER_UNKNOWN);
> > + ClientError *client_error = type_cast(ClientError, e);
> > + if (client_error != NULL) {
> > + client_error->m_errcode = code;
> > + va_list ap;
> > + va_start(ap, fmt);
> > + error_vformat_msg(e, fmt, ap);
> > + va_end(ap);
> > + }
> > + return e;
> > +}
>
> if box_error_new() is already taken by the public api, options are
> box_client_error_new() or box_build_client_error()?
No, box_error_new() name is still available. I've renamed
box_error_construct() to box_error_new() and pushed branch
containing this changes.
>
> --
> Konstantin Osipov, Moscow, Russia
More information about the Tarantool-patches
mailing list