From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (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 36F534696C3 for ; Fri, 17 Apr 2020 00:04:00 +0300 (MSK) References: <2bb678a8aed7e1533ca2c626048080c4c1c5520b.1586505741.git.lvasiliev@tarantool.org> <5688aaca-1326-aed0-9dbc-a13e616539fe@tarantool.org> <40152977-6032-60e4-0396-f773d3dd395b@tarantool.org> <2b4b8329-437a-429e-577b-8e1bb49d14c1@tarantool.org> <428aeafc-58b5-899d-b62e-bf08853d3366@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Thu, 16 Apr 2020 23:03:58 +0200 MIME-Version: 1.0 In-Reply-To: <428aeafc-58b5-899d-b62e-bf08853d3366@tarantool.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [Tarantool-patches] [PATCH v2 2/5] error: Add the custom error type List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: lvasiliev Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the answers! >>>>> @@ -111,9 +148,16 @@ raise: >>>>>            line = info.currentline; >>>>>        } >>>>>    -    struct error *err = box_error_new(file, line, code, "%s", reason); >>>>> -    if (tb_parsed) >>>>> -        err->traceback_mode = tb_mode; >>>>> +    struct error *err = NULL; >>>>> +    if (args.custom) { >>>>> +        err = box_custom_error_new(file, line, args.custom, >>>>> +                       "%s", args.reason); >>>> >>>> 18. Code argument is ignored here. Although I am not sure it shouldn't >>>> be ignored. From what I remember, we decided that code should stay, >>>> but I may be wrong. >>> CustomError type has a predefinded code. Why it shouldn't be ignored? >> >> Because it is not a 'ClientError' object from user's point of view. The >> fact that we implemented it as a descendant of 'ClientError' class should >> not leak into the API. User can't assume anything about error hierarchies. >> He does not even see them. >> >>  From user's point of view there are separate built-in 'ClientError'. And >> separate 'CustomError'. Both accept error code now. But 'CustomError' >> ignores it somewhy. Does not look right. >> > No, from user's point of view it have only type without any codes (In accordance with the task). The task says, I cite: "Ability to skip error codes if we don't need them". Not drop them. But skip when don't need. So essentially - make the code an optional parameter. > Besides, I think it's strange to have an AccessDeniedError with code "ER_SQL_PREPARE" (other branch of hierarchy). How AccessDeniedError is related to that? It is a different error type. Not ClientError, not CustomError. It does not have a code at all. > And user doesn't have an access to "code" of error. Well, they actually have. This is master branch: tarantool> e = box.error.new(1000, 'Message') --- ... tarantool> e.code --- - 1000 ... Talking of "user's point of view" regarding codes: ClientError has one meaning for error code. CustomError can have another. When you will add payload, users will add their codes for sure. Error code is something very basic for any more or less general error type. And if some of them will match our codes, it does not mean they have the same meaning. Some error types don't have a code - they type is enough. Such as AccessDeniedError. box.error.* codes only valid for ClientError objects. CustomError does not have any mapping of error codes. A user can define it.