From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 9B1FD4696C4 for ; Thu, 16 Apr 2020 20:38:48 +0300 (MSK) From: Leonid Vasiliev Date: Thu, 16 Apr 2020 20:38:40 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH V4 1/6] error: add custom error type List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: v.shpilevoy@tarantool.org Cc: tarantool-patches@dev.tarantool.org Co-authored-by: Vladislav Shpilevoy Part of #4398 @TarantoolBot document Title: Custom error types for Lua errors Errors can be created in 2 ways: `box.error.new()` and `box.error()`. Both used to take either `code, reason, ` or `{code = code, reason = reason, ...}` arguments. Now in the first option instead of code a user can specify a string as its own error type. In the second option a user can specify both code and type. For example: ```Lua box.error('MyErrorType', 'Message') box.error({type = 'MyErrorType', code = 1024, reason = 'Message'}) ``` Or no-throw version: ```Lua box.error.new('MyErrorType', 'Message') box.error.new({type = 'MyErrorType', code = 1024, reason = 'Message'}) ``` When a custom type is specified, it is shown in `err.type` attribute. When it is not specified, `err.type` shows one of built-in errors such as 'ClientError', 'OurOfMemory', etc. Name length limit on the custom type is 63 bytes. All is longer is truncated. Original error type can be checked using `err.base_type` member, although normally it should not be used. For user-defined types base type is 'CustomError'. For example: ``` tarantool> e = box.error.new({type = 'MyErrorType', code = 1024, reason = 'Message'}) --- ... tarantool> e:unpack() --- - code: 1024 trace: - file: '[string "e = box.error.new({type = ''MyErrorType'', code..."]' line: 1 type: MyErrorType custom_type: MyErrorType message: Message base_type: CustomError ... ``` --- extra/exports | 1 + src/box/errcode.h | 1 + src/box/error.cc | 92 +++++++++++++++++++++++++++++++++---------- src/box/error.h | 36 +++++++++++++++-- src/box/lua/error.cc | 42 ++++++++++++++------ src/box/xrow.c | 2 +- src/lua/error.lua | 14 ++++++- test/app/fiber.result | 5 ++- test/box/error.result | 66 +++++++++++++++++++++++++++++-- test/box/error.test.lua | 15 +++++++ test/engine/func_index.result | 14 ++++--- 11 files changed, 240 insertions(+), 48 deletions(-) diff --git a/extra/exports b/extra/exports index a9add2c..9467398 100644 --- a/extra/exports +++ b/extra/exports @@ -235,6 +235,7 @@ box_index_min box_index_max box_index_count box_error_type +box_error_custom_type box_error_code box_error_message box_error_last diff --git a/src/box/errcode.h b/src/box/errcode.h index d1e4d02..e37b7bd 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -266,6 +266,7 @@ struct errcode_record { /*211 */_(ER_WRONG_QUERY_ID, "Prepared statement with id %u does not exist") \ /*212 */_(ER_SEQUENCE_NOT_STARTED, "Sequence '%s' is not started") \ /*213 */_(ER_NO_SUCH_SESSION_SETTING, "Session setting %s doesn't exist") \ + /*214 */_(ER_CUSTOM_ERROR, "%s") \ /* * !IMPORTANT! Please follow instructions at start of the file diff --git a/src/box/error.cc b/src/box/error.cc index 233b312..c8fadfb 100644 --- a/src/box/error.cc +++ b/src/box/error.cc @@ -86,35 +86,50 @@ box_error_set(const char *file, unsigned line, uint32_t code, return -1; } +static struct error * +box_error_new_va(const char *file, unsigned line, uint32_t code, + const char *custom_type, const char *fmt, va_list ap) +{ + if (custom_type == NULL) { + struct error *e = BuildClientError(file, line, ER_UNKNOWN); + ClientError *client_error = type_cast(ClientError, e); + if (client_error != NULL) { + client_error->m_errcode = code; + error_vformat_msg(e, fmt, ap); + } + return e; + } else { + struct error *e = BuildCustomError(file, line, custom_type); + CustomError *custom_error = type_cast(CustomError, e); + if (custom_error != NULL) + error_vformat_msg(e, fmt, ap); + + return e; + } +} + struct error * box_error_new(const char *file, unsigned line, uint32_t code, - const char *fmt, ...) + const char *custom_type, 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); - } + va_list ap; + va_start(ap, fmt); + struct error *e = box_error_new_va(file, line, code, custom_type, + fmt, ap); + va_end(ap); return e; } int box_error_add(const char *file, unsigned line, uint32_t code, - const char *fmt, ...) + const char *custom_type, const char *fmt, ...) { - struct error *e = BuildClientError(file, line, ER_UNKNOWN); - ClientError *client_error = type_cast(ClientError, e); - if (client_error) { - client_error->m_errcode = code; - va_list ap; - va_start(ap, fmt); - error_vformat_msg(e, fmt, ap); - va_end(ap); - } + va_list ap; + va_start(ap, fmt); + struct error *e = box_error_new_va(file, line, code, custom_type, + fmt, ap); + va_end(ap); + struct diag *d = &fiber()->diag; if (diag_is_empty(d)) diag_set_error(d, e); @@ -125,6 +140,16 @@ box_error_add(const char *file, unsigned line, uint32_t code, /* }}} */ +const char * +box_error_custom_type(const struct error *e) +{ + CustomError *custom_error = type_cast(CustomError, e); + if (custom_error) + return custom_error->custom_type(); + + return NULL; +} + struct rmean *rmean_error = NULL; const char *rmean_error_strings[RMEAN_ERROR_LAST] = { @@ -290,3 +315,30 @@ BuildAccessDeniedError(const char *file, unsigned int line, return e; } } + +static struct method_info customerror_methods[] = { + make_method(&type_CustomError, "custom_type", &CustomError::custom_type), + METHODS_SENTINEL +}; + +const struct type_info type_CustomError = + make_type("CustomError", &type_ClientError, customerror_methods); + +CustomError::CustomError(const char *file, unsigned int line, + const char *custom_type) + :ClientError(&type_CustomError, file, line, ER_CUSTOM_ERROR) +{ + error_format_msg(this, tnt_errcode_desc(m_errcode), custom_type); + strncpy(m_custom_type, custom_type, sizeof(m_custom_type) - 1); + m_custom_type[sizeof(m_custom_type) - 1] = '\0'; +} + +struct error * +BuildCustomError(const char *file, unsigned int line, const char *custom_type) +{ + try { + return new CustomError(file, line, custom_type); + } catch (OutOfMemory *e) { + return e; + } +} diff --git a/src/box/error.h b/src/box/error.h index ca5d5b2..311d543 100644 --- a/src/box/error.h +++ b/src/box/error.h @@ -53,6 +53,9 @@ struct error * BuildXlogGapError(const char *file, unsigned line, const struct vclock *from, const struct vclock *to); +struct error * +BuildCustomError(const char *file, unsigned int line, const char *custom_type); + /** \cond public */ struct error; @@ -138,11 +141,21 @@ box_error_set(const char *file, unsigned line, uint32_t code, /** \endcond public */ /** + * Return the error custom type, + * \param e Error object. + * \return Pointer to custom error type. On error return NULL. + */ +const char * +box_error_custom_type(const struct error *e); + +/** * Add error to the diagnostic area. In contrast to box_error_set() * it does not replace previous error being set, but rather link * them into list. * * \param code IPROTO error code (enum \link box_error_code \endlink) + * \param custom_type User-defined error type which will be + * displayed instead of ClientError. * \param format (const char * ) - printf()-like format string * \param ... - format arguments * \returns -1 for convention use @@ -151,19 +164,20 @@ box_error_set(const char *file, unsigned line, uint32_t code, */ int box_error_add(const char *file, unsigned line, uint32_t code, - const char *fmt, ...); + const char *custom_type, const char *fmt, ...); /** * Construct error object without setting it in the diagnostics - * area. On the memory allocation fail returns NULL. + * area. On the memory allocation fail returns OutOfMemory error. */ struct error * box_error_new(const char *file, unsigned line, uint32_t code, - const char *fmt, ...); + const char *custom_type, const char *fmt, ...); extern const struct type_info type_ClientError; extern const struct type_info type_XlogError; extern const struct type_info type_AccessDeniedError; +extern const struct type_info type_CustomError; #if defined(__cplusplus) } /* extern "C" */ @@ -290,6 +304,22 @@ struct XlogGapError: public XlogError virtual void raise() { throw this; } }; +class CustomError: public ClientError +{ +public: + CustomError(const char *file, unsigned int line, + const char *custom_type); + + const char* + custom_type() + { + return m_custom_type; + } +private: + /** Custom type name */ + char m_custom_type[64]; +}; + #endif /* defined(__cplusplus) */ #endif /* TARANTOOL_BOX_ERROR_H_INCLUDED */ diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc index b2625bf..68703d8 100644 --- a/src/box/lua/error.cc +++ b/src/box/lua/error.cc @@ -46,6 +46,13 @@ extern "C" { * 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. + * + * Instead of 'code' it is possible to specify a string name of + * the error object's type: + * + * box.error(type, reason, ...) + * box.error({type = string, reason = string, ...}) + * * In case one of arguments is missing its corresponding field * in struct error is filled with default value. */ @@ -53,13 +60,21 @@ static struct error * luaT_error_create(lua_State *L, int top_base) { uint32_t code = 0; + const char *custom_type = NULL; const char *reason = NULL; const char *file = ""; unsigned line = 0; lua_Debug info; int top = lua_gettop(L); - if (top >= top_base && lua_type(L, top_base) == LUA_TNUMBER) { - code = lua_tonumber(L, top_base); + int top_type = lua_type(L, top_base); + if (top >= top_base && (top_type == LUA_TNUMBER || + top_type == LUA_TSTRING)) { + if (top_type == LUA_TNUMBER) { + code = lua_tonumber(L, top_base); + } else { + code = ER_CUSTOM_ERROR; + custom_type = lua_tostring(L, top_base); + } reason = tnt_errcode_desc(code); if (top > top_base) { /* Call string.format(reason, ...) to format message */ @@ -78,14 +93,19 @@ luaT_error_create(lua_State *L, int top_base) /* Missing arguments to format string */ return NULL; } - } else if (top == top_base && lua_istable(L, top_base)) { + } else if (top == top_base && top_type == LUA_TTABLE) { lua_getfield(L, top_base, "code"); - code = lua_tonumber(L, -1); - lua_pop(L, 1); + if (!lua_isnil(L, -1)) + code = lua_tonumber(L, -1); + else + code = ER_CUSTOM_ERROR; lua_getfield(L, top_base, "reason"); reason = lua_tostring(L, -1); if (reason == NULL) reason = ""; + lua_getfield(L, top_base, "type"); + if (!lua_isnil(L, -1)) + custom_type = lua_tostring(L, -1); lua_pop(L, 1); } else { return NULL; @@ -102,7 +122,7 @@ raise: } line = info.currentline; } - return box_error_new(file, line, code, "%s", reason); + return box_error_new(file, line, code, custom_type, "%s", reason); } static int @@ -149,11 +169,11 @@ luaT_error_last(lua_State *L) static int luaT_error_new(lua_State *L) { - if (lua_gettop(L) == 0) - return luaL_error(L, "Usage: box.error.new(code, args)"); - struct error *e = luaT_error_create(L, 1); - if (e == NULL) - return luaL_error(L, "Usage: box.error.new(code, args)"); + struct error *e; + if (lua_gettop(L) == 0 || (e = luaT_error_create(L, 1)) == NULL) { + return luaL_error(L, "Usage: box.error.new(code, args) or "\ + "box.error.new(type, args)"); + } lua_settop(L, 0); luaT_pusherror(L, e); return 1; diff --git a/src/box/xrow.c b/src/box/xrow.c index a494d1f..5494b41 100644 --- a/src/box/xrow.c +++ b/src/box/xrow.c @@ -1123,7 +1123,7 @@ iproto_decode_error_stack(const char **pos) continue; } } - box_error_add(__FILE__, __LINE__, code, reason); + box_error_add(__FILE__, __LINE__, code, NULL, reason); } return 0; } diff --git a/src/lua/error.lua b/src/lua/error.lua index bdc9c71..a5f7435 100644 --- a/src/lua/error.lua +++ b/src/lua/error.lua @@ -35,6 +35,9 @@ exception_get_int(struct error *e, const struct method_info *method); int error_set_prev(struct error *e, struct error *prev); + +const char * +box_error_custom_type(const struct error *e); ]] local REFLECTION_CACHE = {} @@ -75,10 +78,18 @@ local function reflection_get(err, method) end end -local function error_type(err) +local function error_base_type(err) return ffi.string(err._type.name) end +local function error_type(err) + local res = ffi.C.box_error_custom_type(err) + if res ~= nil then + return ffi.string(res) + end + return error_base_type(err) +end + local function error_message(err) return ffi.string(err._errmsg) end @@ -131,6 +142,7 @@ local error_fields = { ["trace"] = error_trace; ["errno"] = error_errno; ["prev"] = error_prev; + ["base_type"] = error_base_type } local function error_unpack(err) diff --git a/test/app/fiber.result b/test/app/fiber.result index debfc67..9b82790 100644 --- a/test/app/fiber.result +++ b/test/app/fiber.result @@ -1038,8 +1038,9 @@ st; ... e:unpack(); --- -- type: ClientError - code: 1 +- code: 1 + base_type: ClientError + type: ClientError message: Illegal parameters, oh my trace: - file: '[string "function err() box.error(box.error.ILLEGAL_PA..."]' diff --git a/test/box/error.result b/test/box/error.result index df6d0eb..82db453 100644 --- a/test/box/error.result +++ b/test/box/error.result @@ -34,8 +34,9 @@ e | ... e:unpack() | --- - | - type: ClientError - | code: 1 + | - code: 1 + | base_type: ClientError + | type: ClientError | message: Illegal parameters, bla bla | trace: | - file: '[C]' @@ -105,7 +106,7 @@ e | ... box.error.new() | --- - | - error: 'Usage: box.error.new(code, args)' + | - error: 'Usage: box.error.new(code, args) or box.error.new(type, args)' | ... -- @@ -431,6 +432,7 @@ t; | 211: box.error.WRONG_QUERY_ID | 212: box.error.SEQUENCE_NOT_STARTED | 213: box.error.NO_SUCH_SESSION_SETTING + | 214: box.error.CUSTOM_ERROR | ... test_run:cmd("setopt delimiter ''"); @@ -489,7 +491,7 @@ assert(box.error.last() == nil) -- box.error.new(err) | --- - | - error: 'Usage: box.error.new(code, args)' + | - error: 'Usage: box.error.new(code, args) or box.error.new(type, args)' | ... -- box.error() is supposed to re-throw last diagnostic error. @@ -831,3 +833,59 @@ assert(box.error.last() == e1) | --- | - true | ... +-- +-- gh-4398: custom error type. +-- +-- Try no code. +e = box.error.new({type = 'TestType', reason = 'Test reason'}) + | --- + | ... +e:unpack() + | --- + | - code: 214 + | base_type: CustomError + | type: TestType + | custom_type: TestType + | message: Test reason + | trace: + | - file: '[string "e = box.error.new({type = ''TestType'', reason ..."]' + | line: 1 + | ... +-- Try code not the same as used by default. +e = box.error.new({type = 'TestType', reason = 'Test reason', code = 123}) + | --- + | ... +e:unpack() + | --- + | - code: 214 + | base_type: CustomError + | type: TestType + | custom_type: TestType + | message: Test reason + | trace: + | - file: '[string "e = box.error.new({type = ''TestType'', reason ..."]' + | line: 1 + | ... +-- Try to omit message. +e = box.error.new({type = 'TestType'}) + | --- + | ... +e:unpack() + | --- + | - code: 214 + | base_type: CustomError + | type: TestType + | custom_type: TestType + | message: + | trace: + | - file: '[string "e = box.error.new({type = ''TestType''}) "]' + | line: 1 + | ... +-- Try too long type name. +e = box.error.new({type = string.rep('a', 128)}) + | --- + | ... +#e.type + | --- + | - 63 + | ... diff --git a/test/box/error.test.lua b/test/box/error.test.lua index 41baed5..38f7406 100644 --- a/test/box/error.test.lua +++ b/test/box/error.test.lua @@ -229,3 +229,18 @@ box.error({code = 111, reason = "err"}) box.error.last() box.error(e1) assert(box.error.last() == e1) +-- +-- gh-4398: custom error type. +-- +-- Try no code. +e = box.error.new({type = 'TestType', reason = 'Test reason'}) +e:unpack() +-- Try code not the same as used by default. +e = box.error.new({type = 'TestType', reason = 'Test reason', code = 123}) +e:unpack() +-- Try to omit message. +e = box.error.new({type = 'TestType'}) +e:unpack() +-- Try too long type name. +e = box.error.new({type = string.rep('a', 128)}) +#e.type diff --git a/test/engine/func_index.result b/test/engine/func_index.result index a827c92..e70e591 100644 --- a/test/engine/func_index.result +++ b/test/engine/func_index.result @@ -277,21 +277,23 @@ e = box.error.last() e:unpack() | --- | - code: 198 - | trace: - | - file: - | line: + | base_type: ClientError | type: ClientError - | message: 'Failed to build a key for functional index ''idx'' of space ''withdata'': - | can''t evaluate function' | prev: '[string "return function(tuple) local ..."]:1: attempt to | call global ''require'' (a nil value)' + | message: 'Failed to build a key for functional index ''idx'' of space ''withdata'': + | can''t evaluate function' + | trace: + | - file: + | line: | ... e = e.prev | --- | ... e:unpack() | --- - | - type: LuajitError + | - base_type: LuajitError + | type: LuajitError | message: '[string "return function(tuple) local ..."]:1: attempt | to call global ''require'' (a nil value)' | trace: -- 2.7.4