From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id B051B24729 for ; Mon, 9 Sep 2019 15:40:39 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id hbVx1IoQ2f6U for ; Mon, 9 Sep 2019 15:40:39 -0400 (EDT) Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 6D2E6204B7 for ; Mon, 9 Sep 2019 15:40:39 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3 1/6] box: rfc for stacked diagnostic area in Tarantool References: <5eaffc5d92bd910865f3da937f8b2831f968c4b0.1567435674.git.kshcherbatov@tarantool.org> <7b5287fa-c430-ca96-3b5b-0b879f28fba9@tarantool.org> From: Vladislav Shpilevoy Message-ID: <82acb857-925d-fd47-2cae-3c8592c7eb26@tarantool.org> Date: Mon, 9 Sep 2019 21:44:17 +0200 MIME-Version: 1.0 In-Reply-To: <7b5287fa-c430-ca96-3b5b-0b879f28fba9@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: Kirill Shcherbatov , tarantool-patches@freelists.org, georgy@tarantool.org Cc: alexander.turenko@tarantool.org > 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.