From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 22E0744184F for ; Wed, 25 Mar 2020 04:43:12 +0300 (MSK) From: Nikita Pettik Date: Wed, 25 Mar 2020 04:43:01 +0300 Message-Id: <1816245c768216733779be5c7420cc60725287ea.1585097339.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH v2 05/10] box/error: don't set error created via box.error.new to diag List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org 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. Needed for #1148 Closes #4778 @TarantoolBot document Title: Don't promote error created via box.error.new to diagnostic area Now box.error.new() only creates error object, but doesn't set it to Tarantool's diagnostic area: ``` box.error.clear() e = box.error.new({code = 111, reason = "cause"}) assert(box.error.last() == nil) --- - true ... ``` To set error in diagnostic area explicitly box.error.set() has been introduced. It accepts error object which is set as last system error (i.e. becomes available via box.error.last()). Finally, box.error.new() does not longer accept error object as an argument (this was undocumented feature). Note that patch does not affect box.error(), which still pushed error to diagnostic area. This fact is reflected in docs: ''' Emulate a request error, with text based on one of the pre-defined Tarantool errors... ''' --- src/box/error.cc | 16 ++++++++++++ src/box/error.h | 8 ++++++ src/box/lua/error.cc | 56 +++++++++++++++++++++++------------------ test/box/error.result | 32 ++++++++++++++++++++++- test/box/error.test.lua | 16 ++++++++++++ 5 files changed, 103 insertions(+), 25 deletions(-) diff --git a/src/box/error.cc b/src/box/error.cc index 7dfe1b3ee..824a4617c 100644 --- a/src/box/error.cc +++ b/src/box/error.cc @@ -86,6 +86,22 @@ box_error_set(const char *file, unsigned line, uint32_t code, return -1; } +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; +} + /* }}} */ struct rmean *rmean_error = NULL; diff --git a/src/box/error.h b/src/box/error.h index b8c7cf73d..ef9cf3e76 100644 --- a/src/box/error.h +++ b/src/box/error.h @@ -137,6 +137,14 @@ box_error_set(const char *file, unsigned line, uint32_t code, /** \endcond public */ +/** + * Construct error object without setting it in the diagnostics + * area. On the memory allocation fail returns NULL. + */ +struct error * +box_error_construct(const char *file, unsigned line, uint32_t code, + const char *fmt, ...); + extern const struct type_info type_ClientError; extern const struct type_info type_XlogError; extern const struct type_info type_AccessDeniedError; diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc index 640e33910..ff285d7eb 100644 --- a/src/box/lua/error.cc +++ b/src/box/lua/error.cc @@ -42,7 +42,14 @@ extern "C" { #include "lua/utils.h" #include "box/error.h" -static void +/** + * Parse Lua arguments (they can come as single table + * f({code : number, reason : string}) or as separate members + * f(code, reason)) and construct struct error with given values. + * In case one of arguments is missing its corresponding field + * in struct error is filled with default value. + */ +static struct error * luaT_error_create(lua_State *L, int top_base) { uint32_t code = 0; @@ -69,25 +76,19 @@ luaT_error_create(lua_State *L, int top_base) reason = lua_tostring(L, -1); } else if (strchr(reason, '%') != NULL) { /* Missing arguments to format string */ - luaL_error(L, "box.error(): bad arguments"); - } - } else if (top == top_base) { - if (lua_istable(L, top_base)) { - /* A special case that rethrows raw error (used by net.box) */ - lua_getfield(L, top_base, "code"); - code = lua_tonumber(L, -1); - lua_pop(L, 1); - lua_getfield(L, top_base, "reason"); - reason = lua_tostring(L, -1); - if (reason == NULL) - reason = ""; - lua_pop(L, 1); - } else if (luaL_iserror(L, top_base)) { - lua_error(L); - return; + return NULL; } + } else if (top == top_base && lua_istable(L, top_base)) { + lua_getfield(L, top_base, "code"); + code = lua_tonumber(L, -1); + lua_pop(L, 1); + lua_getfield(L, top_base, "reason"); + reason = lua_tostring(L, -1); + if (reason == NULL) + reason = ""; + lua_pop(L, 1); } else { - luaL_error(L, "box.error(): bad arguments"); + return NULL; } raise: @@ -101,8 +102,7 @@ raise: } line = info.currentline; } - say_debug("box.error() at %s:%i", file, line); - box_error_set(file, line, code, "%s", reason); + return box_error_construct(file, line, code, "%s", reason); } static int @@ -111,10 +111,15 @@ luaT_error_call(lua_State *L) if (lua_gettop(L) <= 1) { /* Re-throw saved exceptions if any. */ if (box_error_last()) - luaT_error(L); + return luaT_error(L); return 0; } - luaT_error_create(L, 2); + if (lua_gettop(L) == 2 && luaL_iserror(L, 2)) + return lua_error(L); + struct error *e = luaT_error_create(L, 2); + if (e == NULL) + return luaL_error(L, "box.error(): bad arguments"); + diag_set_error(&fiber()->diag, e); return luaT_error(L); } @@ -139,9 +144,12 @@ luaT_error_new(lua_State *L) { if (lua_gettop(L) == 0) return luaL_error(L, "Usage: box.error.new(code, args)"); - luaT_error_create(L, 1); + struct error *e = luaT_error_create(L, 1); + if (e == NULL) + return luaL_error(L, "box.error.new(): bad arguments"); lua_settop(L, 0); - return luaT_error_last(L); + luaT_pusherror(L, e); + return 1; } static int diff --git a/test/box/error.result b/test/box/error.result index 31b2ce52b..a19a7044f 100644 --- a/test/box/error.result +++ b/test/box/error.result @@ -445,7 +445,7 @@ err = box.error.new({code = 111, reason = "cause"}) | ... assert(box.error.last() ~= err) | --- - | - error: assertion failed! + | - true | ... box.error.set(err) | --- @@ -471,6 +471,36 @@ assert(box.error.last() == err) | --- | - true | ... +-- Check that box.error.new() does not set error to diag. +-- +box.error.clear() + | --- + | ... +err = box.error.new(1, "cause") + | --- + | ... +assert(box.error.last() == nil) + | --- + | - true + | ... + +-- box.error.new() does not accept error objects. +-- +box.error.new(err) + | --- + | - error: 'box.error.new(): bad arguments' + | ... + +-- box.error() is supposed to re-throw last diagnostic error. +-- Make sure it does not fail if there's no errors at all +-- (in diagnostics area). +-- +box.error.clear() + | --- + | ... +box.error() + | --- + | ... space:drop() | --- diff --git a/test/box/error.test.lua b/test/box/error.test.lua index 81591ea7e..a0b7d3e78 100644 --- a/test/box/error.test.lua +++ b/test/box/error.test.lua @@ -91,5 +91,21 @@ box.error.set(1) box.error.set(nil) box.error.set(box.error.last()) assert(box.error.last() == err) +-- Check that box.error.new() does not set error to diag. +-- +box.error.clear() +err = box.error.new(1, "cause") +assert(box.error.last() == nil) + +-- box.error.new() does not accept error objects. +-- +box.error.new(err) + +-- box.error() is supposed to re-throw last diagnostic error. +-- Make sure it does not fail if there's no errors at all +-- (in diagnostics area). +-- +box.error.clear() +box.error() space:drop() -- 2.17.1