[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