[tarantool-patches] [PATCH v1 2/3] box: stacked diagnostics area in fiber
Kirill Shcherbatov
kshcherbatov at tarantool.org
Thu Aug 1 14:13:27 MSK 2019
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
More information about the Tarantool-patches
mailing list