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 A02BF207BE for ; Thu, 1 Aug 2019 07:13:34 -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 wGyrIxNVNjK1 for ; Thu, 1 Aug 2019 07:13:34 -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 2D5DD2500C for ; Thu, 1 Aug 2019 07:13:34 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v1 2/3] box: stacked diagnostics area in fiber Date: Thu, 1 Aug 2019 14:13:27 +0300 Message-Id: <0788e7745432f2ff59be4578db0daf0c632900ed.1564657285.git.kshcherbatov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit 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: tarantool-patches@freelists.org, v.shpilevoy@tarantool.org Cc: alexander.turenko@tarantool.org, kostja@tarantool.org, Kirill Shcherbatov This patch introduces stacked errors. A new API diag_add allows to extend an existent error information with a new one error. The previous state becomes a "reason" of the last-set error. Each error object takes a reference to it's reason error object. The :unpack() method patched correspondingly to display the whole errors trace. Part of #1148 --- src/lib/core/diag.h | 45 +++++++++++++++++++++-- src/lib/core/exception.h | 2 +- src/box/key_list.c | 16 ++++---- src/box/lua/call.c | 6 +-- src/box/vy_scheduler.c | 6 +-- src/lib/core/diag.c | 1 + src/lua/utils.c | 2 +- src/box/applier.cc | 2 +- src/box/error.cc | 2 +- src/box/relay.cc | 4 +- src/lib/core/exception.cc | 2 +- src/lua/error.lua | 25 +++++++++++-- test/app/fiber.result | 12 +++--- test/box/access.result | 2 +- test/box/access.test.lua | 2 +- test/box/misc.result | 12 +++--- test/engine/func_index.result | 65 +++++++++++++++++++++++++++++---- test/engine/func_index.test.lua | 4 ++ 18 files changed, 159 insertions(+), 51 deletions(-) diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h index fd3831e66..a70a0727d 100644 --- a/src/lib/core/diag.h +++ b/src/lib/core/diag.h @@ -78,6 +78,8 @@ struct error { char file[DIAG_FILENAME_MAX]; /* Error description. */ char errmsg[DIAG_ERRMSG_MAX]; + /* A pointer to the reason error. */ + struct error *reason; }; static inline void @@ -90,9 +92,13 @@ static inline void error_unref(struct error *e) { assert(e->refs > 0); - --e->refs; - if (e->refs == 0) + while (--e->refs == 0) { + struct error *reason = e->reason; e->destroy(e); + e = reason; + if (e == NULL) + break; + } } NORETURN static inline void @@ -162,7 +168,22 @@ diag_clear(struct diag *diag) } /** - * Add a new error to the diagnostics area + * Set a new error to the diagnostics area, replacing existent. + * \param diag diagnostics area + * \param e error to add + */ +static inline void +diag_set_error(struct diag *diag, struct error *e) +{ + assert(e != NULL); + error_ref(e); + diag_clear(diag); + diag->last = e; +} + +/** + * Add a new error to the diagnostics area: the previous error + * becomes a reason of a current. * \param diag diagnostics area * \param e error to add */ @@ -171,7 +192,12 @@ diag_add_error(struct diag *diag, struct error *e) { assert(e != NULL); error_ref(e); - diag_clear(diag); + /* + * Nominally e takes a reason's reference while diag + * releases it's reference because it holds e now + * instead. I.e. reason->refs kept unchanged. + */ + e->reason = diag->last; diag->last = e; } @@ -264,6 +290,17 @@ BuildSocketError(const char *file, unsigned line, const char *socketname, const char *format, ...); #define diag_set(class, ...) do { \ + /* Preserve the original errno. */ \ + int save_errno = errno; \ + say_debug("%s at %s:%i", #class, __FILE__, __LINE__); \ + struct error *e; \ + e = Build##class(__FILE__, __LINE__, ##__VA_ARGS__); \ + diag_set_error(diag_get(), e); \ + /* Restore the errno which might have been reset. */ \ + errno = save_errno; \ +} while (0) + +#define diag_add(class, ...) do { \ /* Preserve the original errno. */ \ int save_errno = errno; \ say_debug("%s at %s:%i", #class, __FILE__, __LINE__); \ diff --git a/src/lib/core/exception.h b/src/lib/core/exception.h index a29281427..e1a45b3fb 100644 --- a/src/lib/core/exception.h +++ b/src/lib/core/exception.h @@ -182,7 +182,7 @@ exception_init(); #define tnt_error(class, ...) ({ \ say_debug("%s at %s:%i", #class, __FILE__, __LINE__); \ class *e = new class(__FILE__, __LINE__, ##__VA_ARGS__); \ - diag_add_error(diag_get(), e); \ + diag_set_error(diag_get(), e); \ e; \ }) diff --git a/src/box/key_list.c b/src/box/key_list.c index e130d1c8c..5375e0823 100644 --- a/src/box/key_list.c +++ b/src/box/key_list.c @@ -63,9 +63,9 @@ key_list_iterator_create(struct key_list_iterator *it, struct tuple *tuple, if (rc != 0) { /* Can't evaluate function. */ struct space *space = space_by_id(index_def->space_id); - diag_set(ClientError, ER_FUNC_INDEX_FUNC, index_def->name, - space ? space_name(space) : "", - diag_last_error(diag_get())->errmsg); + diag_add(ClientError, ER_FUNC_INDEX_FUNC, index_def->name, + space != NULL ? space_name(space) : "", + "can't evaluate function"); return -1; } uint32_t key_data_sz; @@ -74,9 +74,9 @@ key_list_iterator_create(struct key_list_iterator *it, struct tuple *tuple, if (key_data == NULL) { struct space *space = space_by_id(index_def->space_id); /* Can't get a result returned by function . */ - diag_set(ClientError, ER_FUNC_INDEX_FUNC, index_def->name, - space ? space_name(space) : "", - diag_last_error(diag_get())->errmsg); + diag_add(ClientError, ER_FUNC_INDEX_FUNC, index_def->name, + space != NULL ? space_name(space) : "", + "can't get a value returned by function"); return -1; } @@ -170,9 +170,9 @@ key_list_iterator_next(struct key_list_iterator *it, const char **value) * The key doesn't follow functional index key * definition. */ - diag_set(ClientError, ER_FUNC_INDEX_FORMAT, it->index_def->name, + diag_add(ClientError, ER_FUNC_INDEX_FORMAT, it->index_def->name, space ? space_name(space) : "", - diag_last_error(diag_get())->errmsg); + "key does not follow functional index key definition"); return -1; } diff --git a/src/box/lua/call.c b/src/box/lua/call.c index 0ac2eb7a6..cebd8a680 100644 --- a/src/box/lua/call.c +++ b/src/box/lua/call.c @@ -680,9 +680,9 @@ func_persistent_lua_load(struct func_lua *func) if (func->base.def->is_sandboxed) { if (prepare_lua_sandbox(tarantool_L, default_sandbox_exports, nelem(default_sandbox_exports)) != 0) { - diag_set(ClientError, ER_LOAD_FUNCTION, - func->base.def->name, - diag_last_error(diag_get())->errmsg); + diag_add(ClientError, ER_LOAD_FUNCTION, + func->base.def->name, + "can't prepare a Lua sandbox"); goto end; } } else { diff --git a/src/box/vy_scheduler.c b/src/box/vy_scheduler.c index f3bded209..8b1a26ef4 100644 --- a/src/box/vy_scheduler.c +++ b/src/box/vy_scheduler.c @@ -606,7 +606,7 @@ vy_scheduler_dump(struct vy_scheduler *scheduler) if (scheduler->is_throttled) { /* Dump error occurred. */ struct error *e = diag_last_error(&scheduler->diag); - diag_add_error(diag_get(), e); + diag_set_error(diag_get(), e); return -1; } fiber_cond_wait(&scheduler->dump_cond); @@ -680,7 +680,7 @@ vy_scheduler_begin_checkpoint(struct vy_scheduler *scheduler) */ if (scheduler->is_throttled) { struct error *e = diag_last_error(&scheduler->diag); - diag_add_error(diag_get(), e); + diag_set_error(diag_get(), e); say_error("cannot checkpoint vinyl, " "scheduler is throttled with: %s", e->errmsg); return -1; @@ -716,7 +716,7 @@ vy_scheduler_wait_checkpoint(struct vy_scheduler *scheduler) if (scheduler->is_throttled) { /* A dump error occurred, abort checkpoint. */ struct error *e = diag_last_error(&scheduler->diag); - diag_add_error(diag_get(), e); + diag_set_error(diag_get(), e); say_error("vinyl checkpoint failed: %s", e->errmsg); return -1; } diff --git a/src/lib/core/diag.c b/src/lib/core/diag.c index 248277e74..c0aeb52b9 100644 --- a/src/lib/core/diag.c +++ b/src/lib/core/diag.c @@ -52,6 +52,7 @@ error_create(struct error *e, e->line = 0; } e->errmsg[0] = '\0'; + e->reason = NULL; } struct diag * diff --git a/src/lua/utils.c b/src/lua/utils.c index 0a4bcf517..5a2514f3a 100644 --- a/src/lua/utils.c +++ b/src/lua/utils.c @@ -994,7 +994,7 @@ luaT_toerror(lua_State *L) struct error *e = luaL_iserror(L, -1); if (e != NULL) { /* Re-throw original error */ - diag_add_error(&fiber()->diag, e); + diag_set_error(&fiber()->diag, e); } else { /* Convert Lua error to a Tarantool exception. */ diag_set(LuajitError, luaT_tolstring(L, -1, NULL)); diff --git a/src/box/applier.cc b/src/box/applier.cc index cf03ea160..dbc94c1a7 100644 --- a/src/box/applier.cc +++ b/src/box/applier.cc @@ -756,7 +756,7 @@ applier_on_rollback(struct trigger *trigger, void *event) struct applier *applier = (struct applier *)trigger->data; /* Setup a shared error. */ if (!diag_is_empty(&replicaset.applier.diag)) { - diag_add_error(&applier->diag, + diag_set_error(&applier->diag, diag_last_error(&replicaset.applier.diag)); } /* Stop the applier fiber. */ diff --git a/src/box/error.cc b/src/box/error.cc index 47dce3305..7dfe1b3ee 100644 --- a/src/box/error.cc +++ b/src/box/error.cc @@ -82,7 +82,7 @@ box_error_set(const char *file, unsigned line, uint32_t code, error_vformat_msg(e, fmt, ap); va_end(ap); } - diag_add_error(&fiber()->diag, e); + diag_set_error(&fiber()->diag, e); return -1; } diff --git a/src/box/relay.cc b/src/box/relay.cc index e9f5bdca9..e44b88ea0 100644 --- a/src/box/relay.cc +++ b/src/box/relay.cc @@ -447,7 +447,7 @@ relay_set_error(struct relay *relay, struct error *e) { /* Don't override existing error. */ if (diag_is_empty(&relay->diag)) - diag_add_error(&relay->diag, e); + diag_set_error(&relay->diag, e); } static void @@ -621,7 +621,7 @@ relay_subscribe_f(va_list ap) * Don't clear the error for status reporting. */ assert(!diag_is_empty(&relay->diag)); - diag_add_error(diag_get(), diag_last_error(&relay->diag)); + diag_set_error(diag_get(), diag_last_error(&relay->diag)); diag_log(); say_crit("exiting the relay loop"); diff --git a/src/lib/core/exception.cc b/src/lib/core/exception.cc index a6999af43..55ee8cd40 100644 --- a/src/lib/core/exception.cc +++ b/src/lib/core/exception.cc @@ -99,7 +99,7 @@ Exception::operator new(size_t size) void *buf = malloc(size); if (buf != NULL) return buf; - diag_add_error(diag_get(), &out_of_memory); + diag_set_error(diag_get(), &out_of_memory); throw &out_of_memory; } diff --git a/src/lua/error.lua b/src/lua/error.lua index 28fc0377d..2bb8ccfe0 100644 --- a/src/lua/error.lua +++ b/src/lua/error.lua @@ -23,6 +23,8 @@ struct error { char _file[DIAG_FILENAME_MAX]; /* Error description. */ char _errmsg[DIAG_ERRMSG_MAX]; + /* A pointer to the reason error. */ + struct error *_reason; }; char * @@ -92,10 +94,7 @@ local error_fields = { ["trace"] = error_trace; } -local function error_unpack(err) - if not ffi.istype('struct error', err) then - error("Usage: error:unpack()") - end +local function error_unpack_one(err) local result = {} for key, getter in pairs(error_fields) do result[key] = getter(err) @@ -109,6 +108,24 @@ local function error_unpack(err) return result end +local function error_unpack(err) + if not ffi.istype('struct error', err) then + error("Usage: error:unpack()") + end + -- Unwind errors + local errors = {} + while err ~= nil do + table.insert(errors, err) + err = err._reason + end + -- Prepare a result in a reverse order. + local result = {} + for i = #errors, 1, -1 do + table.insert(result, error_unpack_one(errors[i])) + end + return result +end + local function error_raise(err) if not ffi.istype('struct error', err) then error("Usage: error:raise()") diff --git a/test/app/fiber.result b/test/app/fiber.result index 94e690f6c..d9424499a 100644 --- a/test/app/fiber.result +++ b/test/app/fiber.result @@ -1038,12 +1038,12 @@ st; ... e:unpack(); --- -- type: ClientError - code: 1 - message: Illegal parameters, oh my - trace: - - file: '[string "function err() box.error(box.error.ILLEGAL_PA..."]' - line: 1 +- - type: ClientError + code: 1 + message: Illegal parameters, oh my + trace: + - file: '[string "function err() box.error(box.error.ILLEGAL_PA..."]' + line: 1 ... flag = false; --- diff --git a/test/box/access.result b/test/box/access.result index ba72b5f74..12a0047f8 100644 --- a/test/box/access.result +++ b/test/box/access.result @@ -1340,7 +1340,7 @@ box.session.su("test_user") st, e = pcall(s.select, s, {}) --- ... -e = e:unpack() +e = e:unpack()[1] --- ... e.type, e.access_type, e.object_type, e.message diff --git a/test/box/access.test.lua b/test/box/access.test.lua index 219cdb04a..24789a785 100644 --- a/test/box/access.test.lua +++ b/test/box/access.test.lua @@ -502,7 +502,7 @@ obj_name:match("function") box.schema.user.revoke("test_user", "usage", "universe") box.session.su("test_user") st, e = pcall(s.select, s, {}) -e = e:unpack() +e = e:unpack()[1] e.type, e.access_type, e.object_type, e.message obj_type, obj_name, op_type euid, auid diff --git a/test/box/misc.result b/test/box/misc.result index 7a15dabf0..25dda30a7 100644 --- a/test/box/misc.result +++ b/test/box/misc.result @@ -125,12 +125,12 @@ e ... e:unpack() --- -- type: ClientError - code: 1 - message: Illegal parameters, bla bla - trace: - - file: '[C]' - line: 4294967295 +- - type: ClientError + code: 1 + message: Illegal parameters, bla bla + trace: + - file: '[C]' + line: 4294967295 ... e.type --- diff --git a/test/engine/func_index.result b/test/engine/func_index.result index 877b76d5e..ae8873c9b 100644 --- a/test/engine/func_index.result +++ b/test/engine/func_index.result @@ -167,8 +167,7 @@ idx = s:create_index('idx', {func = box.func.invalidreturn1.id, parts = {{1, 'un s:insert({1}) | --- | - error: 'Key format doesn''t match one defined in functional index ''idx'' of space - | ''withdata'': Supplied key type of part 0 does not match index part type: expected - | unsigned' + | ''withdata'': key does not follow functional index key definition' | ... idx:drop() | --- @@ -189,6 +188,16 @@ s:insert({1}) | - error: 'Key format doesn''t match one defined in functional index ''idx'' of space | ''withdata'': to many values were returned' | ... +box.error.last():unpack() + | --- + | - - type: ClientError + | code: 199 + | message: 'Key format doesn''t match one defined in functional index ''idx'' of + | space ''withdata'': to many values were returned' + | trace: + | - file: /home/kir/WORK/tarantool/src/box/key_list.c + | line: 95 + | ... idx:drop() | --- | ... @@ -206,8 +215,24 @@ idx = s:create_index('idx', {func = box.func.invalidreturn3.id, parts = {{1, 'un s:insert({1}) | --- | - error: 'Key format doesn''t match one defined in functional index ''idx'' of space - | ''withdata'': Supplied key type of part 0 does not match index part type: expected - | unsigned' + | ''withdata'': key does not follow functional index key definition' + | ... +box.error.last():unpack() + | --- + | - - type: ClientError + | code: 18 + | message: 'Supplied key type of part 0 does not match index part type: expected + | unsigned' + | trace: + | - file: /home/kir/WORK/tarantool/src/box/key_def.h + | line: 534 + | - type: ClientError + | code: 199 + | message: 'Key format doesn''t match one defined in functional index ''idx'' of + | space ''withdata'': key does not follow functional index key definition' + | trace: + | - file: /home/kir/WORK/tarantool/src/box/key_list.c + | line: 175 | ... idx:drop() | --- @@ -226,8 +251,7 @@ idx = s:create_index('idx', {func = box.func.invalidreturn4.id, parts = {{1, 'un s:insert({1}) | --- | - error: 'Key format doesn''t match one defined in functional index ''idx'' of space - | ''withdata'': Supplied key type of part 0 does not match index part type: expected - | unsigned' + | ''withdata'': key does not follow functional index key definition' | ... idx:drop() | --- @@ -248,6 +272,16 @@ s:insert({1}) | - error: 'Key format doesn''t match one defined in functional index ''idx'' of space | ''withdata'': a multikey function mustn''t return a scalar' | ... +box.error.last():unpack() + | --- + | - - type: ClientError + | code: 199 + | message: 'Key format doesn''t match one defined in functional index ''idx'' of + | space ''withdata'': a multikey function mustn''t return a scalar' + | trace: + | - file: /home/kir/WORK/tarantool/src/box/key_list.c + | line: 109 + | ... idx:drop() | --- | ... @@ -273,8 +307,23 @@ idx = s:create_index('idx', {func = box.func.runtimeerror.id, parts = {{1, 'stri s:insert({1}) | --- | - error: 'Failed to build a key for functional index ''idx'' of space ''withdata'': - | [string "return function(tuple) local ..."]:1: attempt to call - | global ''require'' (a nil value)' + | can''t evaluate function' + | ... +box.error.last():unpack() + | --- + | - - type: LuajitError + | message: '[string "return function(tuple) local ..."]:1: attempt + | to call global ''require'' (a nil value)' + | trace: + | - file: /home/kir/WORK/tarantool/src/lua/utils.c + | line: 1000 + | - type: ClientError + | code: 198 + | message: 'Failed to build a key for functional index ''idx'' of space ''withdata'': + | can''t evaluate function' + | trace: + | - file: /home/kir/WORK/tarantool/src/box/key_list.c + | line: 68 | ... idx:drop() | --- diff --git a/test/engine/func_index.test.lua b/test/engine/func_index.test.lua index 372ec800d..587d3c4b1 100644 --- a/test/engine/func_index.test.lua +++ b/test/engine/func_index.test.lua @@ -69,6 +69,7 @@ lua_code = [[function(tuple) return {"hello", "world"}, {1, 2} end]] box.schema.func.create('invalidreturn2', {body = lua_code, is_deterministic = true, is_sandboxed = true, opts = {is_multikey = true}}) idx = s:create_index('idx', {func = box.func.invalidreturn2.id, parts = {{1, 'unsigned'}, {2, 'unsigned'}}}) s:insert({1}) +box.error.last():unpack() idx:drop() -- Invalid functional index extractor routine return: the second returned key invalid. @@ -76,6 +77,7 @@ lua_code = [[function(tuple) return {{"hello", "world"}, {1, 2}} end]] box.schema.func.create('invalidreturn3', {body = lua_code, is_deterministic = true, is_sandboxed = true, opts = {is_multikey = true}}) idx = s:create_index('idx', {func = box.func.invalidreturn3.id, parts = {{1, 'unsigned'}, {2, 'unsigned'}}}) s:insert({1}) +box.error.last():unpack() idx:drop() -- Invalid functional index extractor routine return: multikey return in case of regular index. @@ -90,6 +92,7 @@ lua_code = [[function(tuple) return "hello" end]] box.schema.func.create('invalidreturn5', {body = lua_code, is_deterministic = true, is_sandboxed = true, opts = {is_multikey = true}}) idx = s:create_index('idx', {func = box.func.invalidreturn5.id, parts = {{1, 'unsigned'}, {2, 'unsigned'}}}) s:insert({1}) +box.error.last():unpack() idx:drop() -- Invalid function: runtime extractor error @@ -102,6 +105,7 @@ test_run:cmd("setopt delimiter ''"); box.schema.func.create('runtimeerror', {body = lua_code, is_deterministic = true, is_sandboxed = true}) idx = s:create_index('idx', {func = box.func.runtimeerror.id, parts = {{1, 'string'}}}) s:insert({1}) +box.error.last():unpack() idx:drop() -- Remove old persistent functions -- 2.22.0