[tarantool-patches] Re: [PATCH v3 1/6] box: rfc for stacked diagnostic area in Tarantool

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Mon Sep 9 22:44:17 MSK 2019


> 2.
>>> +```
>>> +
>>> +Error cdata object has `:unpack()`, `:raise()`, `:match(...)`,
>>> `:__serialize()` methods and uniform `.type`, `.message` and `.trace`
>>> fields. +
>>> +
>> What if we want to throw a table error - as vshard does.
> 
> https://github.com/tarantool/vshard/blob/master/vshard/error.lua
> As fores I've understood, vshard has an own error representation same as box_error
> adapter that constructs vshard-compatible table error object.

VShard repeats error:unpack() format. If you change result of unpack(), then
I will update VShard (if that will be necessary).

> The backward compatibility wouldn't be broken with introducing a couple new fields in core error object.

Yes, if you only add new fields to error:unpack(), then nothing
will break.

> However, to have a profit with new stacked diagnostics functionality the box_error adapter should be
> updated correspondingly. This likely requires extending vshard error functionality. 
> I'd better create a related ticket for vshard module, if you are agree with me.
> 
> 
> 3.
>> I don't see any valid purpose for svp* methods. Please explain the cases when 
>> we need to rollback a diag area.

Me neither. Svp won't solve any problems IMO. It will
just complicate the code. Talking of cases below - if you
call a function, you need to know how it works. If you called
a function not setting a diag, and you expected a diag, that
you either didn't test your code properly, or you was
inattentive, or both. Svp won't solve neither of that problems.
You still may forget to reset svp, to set svp, to test the code.

> Kostya had a strong opinion about an ability to reset a last-errors set.

Honestly, I didn't understand his point. And I think, if
we ever will understand, we can easily add svps. But if we add
them now and they appear to be useless, then we won't be able
to drop them, because of compatibility.

> 5.
>>> +-- Return a reason error object for given error
>>> +-- (when exists, nil otherwise)
>>> +box.error.prev(error) == error.prev
>>> +```
>> You let out of scope standard lua "error()" call. Please take it into 
>> consideration.
>  
> I can't say nothing about error() call. It is not box error, it can't be extended.
> It should be deprecated when #4398 would be closed, right?

error() is a built-in Lua function, it is a part of the language. You can't
deprecate it. But I also don't understand what is wrong with it. Seems like
it will work just like always.




More information about the Tarantool-patches mailing list