From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
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 dev.tarantool.org (Postfix) with ESMTPS id AF0194696C3
 for <tarantool-patches@dev.tarantool.org>;
 Thu,  2 Apr 2020 03:37:58 +0300 (MSK)
References: <cb1010c1497f1d134e576b5bbbd6c1d0520ff7f4.1585756172.git.korablev@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <2d0316df-dda7-6438-d8cb-c7fd6290d830@tarantool.org>
Date: Thu, 2 Apr 2020 02:37:56 +0200
MIME-Version: 1.0
In-Reply-To: <cb1010c1497f1d134e576b5bbbd6c1d0520ff7f4.1585756172.git.korablev@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH] box: always promote error created
	via box.error() to diag
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Nikita Pettik <korablev@tarantool.org>, tarantool-patches@dev.tarantool.org

Thanks for the patch!

On 01/04/2020 17:52, Nikita Pettik wrote:
> Note that it is vital that box.error() now promotes error to diag, since
> otherwise user is unable to set to diag custom error, which in turn is

Why unable? I thought we have box.error.set(). You can always do

    box.error.set(error_object)
    box.error()

This is equivalent of

    box.error(error_object)

which you are doing in this patch.

> linked with other errors:
> 
> e1 = box.error.new({code = 111, reason = "cause"})
> e2 = box.error.new({code = 111, reason = "err"})
> e2:set_prev(e1)
> -- There's no means to set e2 to diagnostic area.

tarantool> e1 = box.error.new({code = 111})
---
...

tarantool> e2 = box.error.new({code = 111})
---
...

tarantool> e2:set_prev(e1)
---
...

tarantool> box.error.set(e2)
---
...

tarantool> box.error.last() == e2
---
- true
...

> This patch makes box.error() always promote error to the diagnostic
> area despite of passed arguments.

But I am not against this patch. It is a nice sugar, when you need to
both set + throw. It is LGTM except the commit message due to comments
above.

> Closes #4829
> 
> @TarantoolBot document
> Title: always promote error created via box.error() to diag
> 
> box.error() is able to accept two types of argument: either pair of code
> + reason (box.error{code = 555, reason = 'Arbitrary message'}) or error
> object (box.error(err)). In the first case error is promoted to
> diagnostic area, meanwhile in the latter - it is not:
> ```
> e1 = box.error.new({code = 111, reason = "cause"})
> box.error({code = 111, reason = "err"})
> - error: err
> box.error.last()
> - err
> box.error(e1)
> - error: cause
> box.error.last()
> - err
> ```
> From now box.error(e1) sets error to diagnostic area as well:
> ```
> box.error(e1)
> - error: cause
> box.error.last()
> - cause
> ```
> ---