Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics
@ 2020-04-06 14:17 Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 1/6] box: introduce stacked diagnostic area Nikita Pettik
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Nikita Pettik @ 2020-04-06 14:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

Branch: https://github.com/tarantool/tarantool/commits/np/gh-1148-stacked-diag
Issue:
https://github.com/tarantool/tarantool/issues/1148
https://github.com/tarantool/tarantool/issues/4829

Implementation of stacked diagnostics in Tarantool according to RFC:
https://github.com/tarantool/tarantool/commit/1acd32d98f628431429b427df19caa9d269bb9c8).
Error structure is extended with double linked list; Lua and C interfaces
are provided to interact with it; stacked diagnostics is supported in
IProto protocol and net.box modules.

Note that patch-set also contains fix of #4829 since some tests rely
on the fact that box.error(err_obj) promotes error to diagnotic area.

Changes in v2:

- renamed 'prev' and 'next' links in struct error to 'cause' and 'effect';
- moved all tests related to box.error module to a separate file (box/error.test.lua);
- removed recursion from error_unref();
- removed box_error_construct() and box_error_add() from public API;
- removed error_prev() getter which was used to access error.prev member in Lua
(now it is accessed directly via error._prev);
- moved diag_add() usages to a separate commit;
- several ref/unref usages corrections in the code;
- added mp_check_* auxiliary checks to iproto_decode_error_stack();
- other minor refactoring (removing unused headers and redundant testing
facilities, fixing comments etc);
- rebased to fresh master branch.

Changes in v3:

- refactored iproto_decode_error_stack(): removed mp_check_*() calls,
added checks on broken map keys values including overflowed values;
- added unit tests to cover iproto_decode_error_stack() function
(which can be called by replication applier);
- integration tests were simplified: instead of using complicated
schema involving persistent functions and functional indexes, simple
function which raises stack of errors is used;
- optimized error_set_prev() function: cycle detection is avoided in
most cases;
- other refactoring: code, tests and commit message clean-ups; added
a few comments and assertion checks.
- rebased to fresh master branch.

@ChangeLog 
* Always promote error created via box.error() to diagnostic area (gh-4829).
* Introduced stacked diagnostick area: now each Lua table representing error
object features .prev member and :set_prev() method. So that errors can be
organized into lists. IProto protocol is extended with new command keys to
support this feature as well (gh-1148).

Kirill Shcherbatov (1):
  iproto: refactor error encoding with mpstream

Nikita Pettik (5):
  box: introduce stacked diagnostic area
  box: use stacked diagnostic area for functional indexes
  box/error: clarify purpose of reference counting in struct error
  box: always promote error created via box.error() to diag
  iproto: support error stacked diagnostic area

 extra/exports                   |   1 +
 src/box/error.cc                |  21 +++
 src/box/error.h                 |  16 ++
 src/box/iproto_constants.h      |   6 +
 src/box/key_list.c              |  12 +-
 src/box/lua/call.c              |   6 +-
 src/box/lua/error.cc            |  13 +-
 src/box/lua/net_box.lua         |  30 ++-
 src/box/xrow.c                  | 150 ++++++++++++---
 src/lib/core/diag.c             |  47 +++++
 src/lib/core/diag.h             | 111 ++++++++++-
 src/lib/core/exception.cc       |   1 +
 src/lua/error.lua               |  32 ++++
 test/box-py/iproto.result       |   6 +-
 test/box-py/iproto.test.py      |   6 +-
 test/box/error.result           | 325 ++++++++++++++++++++++++++++++++
 test/box/error.test.lua         | 120 ++++++++++++
 test/box/iproto.result          | 142 ++++++++++++++
 test/box/iproto.test.lua        |  66 +++++++
 test/box/net.box.result         |  71 +++++++
 test/box/net.box.test.lua       |  31 +++
 test/engine/func_index.result   |  54 +++++-
 test/engine/func_index.test.lua |   8 +
 test/unit/xrow.cc               | 184 +++++++++++++++++-
 test/unit/xrow.result           |  27 ++-
 25 files changed, 1424 insertions(+), 62 deletions(-)
 create mode 100644 test/box/iproto.result
 create mode 100644 test/box/iproto.test.lua

-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Tarantool-patches] [PATCH v3 1/6] box: introduce stacked diagnostic area
  2020-04-06 14:17 [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Nikita Pettik
@ 2020-04-06 14:17 ` Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 2/6] box: use stacked diagnostic area for functional indexes Nikita Pettik
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Nikita Pettik @ 2020-04-06 14:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

In terms of implementation, now struct error objects can be organized
into double-linked lists. To achieve this pointers to the next and
previous elements (cause and effect correspondingly) have been added to
struct error. It is worth mentioning that already existing rlist and
stailq list implementations are not suitable: rlist is cycled list, as a
result it is impossible to start iteration over the list from random
list entry and finish it at the logical end of the list; stailq is
single-linked list leaving no possibility to remove elements from the
middle of the list.

As a part of C interface, box_error_add() has been introduced. In
contrast to box_error_set() it does not replace last raised error, but
instead it adds error to the list of diagnostic errors having already
been set. If error is to be deleted (its reference counter hits 0 value)
it is unlinked from the list it belongs to and destroyed. Meanwhile,
error destruction leads to decrement of reference counter of its
previous error and so on.

To organize errors into lists in Lua, table representing error object in
Lua now has .prev field (corresponding to 'previous' error) and method
:set_prev(e). The latter accepts error object (i.e. created via
box.error.new() or box.error.last()) and nil value. Both field .prev and
:set_prev() method are implemented as ffi functions. Also note that
cycles are not allowed while organizing errors into lists:
e1 -> e2 -> e3; e3:set_prev(e1) -- would lead to error.

Part of #1148
---
 extra/exports             |   1 +
 src/lib/core/diag.c       |  47 ++++++
 src/lib/core/diag.h       | 102 ++++++++++++-
 src/lib/core/exception.cc |   1 +
 src/lua/error.lua         |  32 ++++
 test/box/error.result     | 303 ++++++++++++++++++++++++++++++++++++++
 test/box/error.test.lua   | 112 ++++++++++++++
 7 files changed, 595 insertions(+), 3 deletions(-)

diff --git a/extra/exports b/extra/exports
index cbb5adcf4..9323996c1 100644
--- a/extra/exports
+++ b/extra/exports
@@ -234,6 +234,7 @@ box_error_message
 box_error_last
 box_error_clear
 box_error_set
+error_set_prev
 box_latch_new
 box_latch_delete
 box_latch_lock
diff --git a/src/lib/core/diag.c b/src/lib/core/diag.c
index 8352cbdf7..e143db18d 100644
--- a/src/lib/core/diag.c
+++ b/src/lib/core/diag.c
@@ -31,6 +31,51 @@
 #include "diag.h"
 #include "fiber.h"
 
+int
+error_set_prev(struct error *e, struct error *prev)
+{
+	/*
+	 * Make sure that adding error won't result in cycles.
+	 * Don't bother with sophisticated cycle-detection
+	 * algorithms, simple iteration is OK since as a rule
+	 * list contains a dozen errors at maximum.
+	 */
+	if (prev != NULL) {
+		if (e == prev)
+			return -1;
+		if (prev->effect != NULL || e->effect != NULL) {
+			/*
+			 * e and prev are already compared, so start
+			 * from prev->cause.
+			 */
+			struct error *tmp = prev->cause;
+			while (tmp != NULL) {
+				 if (tmp == e)
+					return -1;
+				tmp = tmp->cause;
+			}
+			/*
+			 * Unlink new 'effect' node from its old
+			 * list of 'cause' errors.
+			 */
+			error_unlink_effect(prev);
+		}
+		error_ref(prev);
+		prev->effect = e;
+	}
+	/*
+	 * At once error can feature only one reason.
+	 * So unlink previous 'cause' node.
+	 */
+	if (e->cause != NULL) {
+		e->cause->effect = NULL;
+		error_unref(e->cause);
+	}
+	/* Set new 'prev' node. */
+	e->cause = prev;
+	return 0;
+}
+
 void
 error_create(struct error *e,
 	     error_f destroy, error_f raise, error_f log,
@@ -50,6 +95,8 @@ error_create(struct error *e,
 		e->line = 0;
 	}
 	e->errmsg[0] = '\0';
+	e->cause = NULL;
+	e->effect = NULL;
 }
 
 struct diag *
diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h
index 7e1e1a174..9c238f5a2 100644
--- a/src/lib/core/diag.h
+++ b/src/lib/core/diag.h
@@ -84,6 +84,23 @@ struct error {
 	char file[DIAG_FILENAME_MAX];
 	/* Error description. */
 	char errmsg[DIAG_ERRMSG_MAX];
+	/**
+	 * Link to the cause and effect of given error. The cause
+	 * creates the effect:
+	 * e1 = box.error.new({code = 0, reason = 'e1'})
+	 * e2 = box.error.new({code = 0, reason = 'e2'})
+	 * e1:set_prev(e2) -- Now e2 is the cause of e1 and e1 is
+	 * the effect of e2.
+	 * Only cause keeps reference to avoid cyclic dependence.
+	 * RLIST implementation is not really suitable here
+	 * since it is organized as circular list. In such
+	 * a case it is impossible to start an iteration
+	 * from any node and finish at the logical end of the
+	 * list. Double-linked list is required to allow deletion
+	 * from the middle of the list.
+	 */
+	struct error *cause;
+	struct error *effect;
 };
 
 static inline void
@@ -96,11 +113,56 @@ static inline void
 error_unref(struct error *e)
 {
 	assert(e->refs > 0);
-	--e->refs;
-	if (e->refs == 0)
-		e->destroy(e);
+	struct error *to_delete = e;
+	while (--to_delete->refs == 0) {
+		/* Unlink error from lists completely.*/
+		struct error *cause = to_delete->cause;
+		assert(to_delete->effect == NULL);
+		if (to_delete->cause != NULL) {
+			to_delete->cause->effect = NULL;
+			to_delete->cause = NULL;
+		}
+		to_delete->destroy(to_delete);
+		if (cause == NULL)
+			return;
+		to_delete = cause;
+	}
+}
+
+/**
+ * Unlink error from its effect. For instance:
+ * e1 -> e2 -> e3 -> e4 (e1:set_prev(e2); e2:set_prev(e3) ...)
+ * unlink(e3): e1 -> e2 -> NULL; e3 -> e4 -> NULL
+ */
+static inline void
+error_unlink_effect(struct error *e)
+{
+	if (e->effect != NULL) {
+		assert(e->refs > 1);
+		error_unref(e);
+		e->effect->cause = NULL;
+	}
+	e->effect = NULL;
 }
 
+/**
+ * Set previous error: cut @a prev from its previous 'tail' of
+ * causes and link to the one @a e belongs to. Note that all
+ * previous errors starting from @a prev->cause are transferred
+ * with it as well (i.e. causes for given error are not erased).
+ * For instance:
+ * e1 -> e2 -> NULL; e3 -> e4 -> NULL;
+ * e2:set_effect(e3): e1 -> e2 -> e3 -> e4 -> NULL
+ *
+ * @a effect can be NULL. To be used as ffi method in
+ * lua/error.lua.
+ *
+ * @retval -1 in case adding @a effect results in list cycles;
+ *          0 otherwise.
+ */
+int
+error_set_prev(struct error *e, struct error *prev);
+
 NORETURN static inline void
 error_raise(struct error *e)
 {
@@ -178,6 +240,31 @@ diag_set_error(struct diag *diag, struct error *e)
 	assert(e != NULL);
 	error_ref(e);
 	diag_clear(diag);
+	error_unlink_effect(e);
+	diag->last = e;
+}
+
+/**
+ * Add a new error to the diagnostics area. It is added to the
+ * tail, so that list forms stack.
+ * @param diag Diagnostics area.
+ * @param e Error to be added.
+ */
+static inline void
+diag_add_error(struct diag *diag, struct error *e)
+{
+	assert(e != NULL);
+	/* diag_set_error() should be called before. */
+	assert(diag->last != NULL);
+	/*
+	 * e should be the bottom of its own stack.
+	 * Otherwise some errors may be lost.
+	 */
+	assert(e->effect == NULL);
+	assert(diag->last->effect == NULL);
+	error_ref(e);
+	e->cause = diag->last;
+	diag->last->effect = e;
 	diag->last = e;
 }
 
@@ -280,6 +367,15 @@ BuildSocketError(const char *file, unsigned line, const char *socketname,
 	errno = save_errno;                                             \
 } while (0)
 
+#define diag_add(class, ...) do {					\
+	int save_errno = errno;						\
+	say_debug("%s at %s:%i", #class, __FILE__, __LINE__);		\
+	struct error *e;						\
+	e = Build##class(__FILE__, __LINE__, ##__VA_ARGS__);		\
+	diag_add_error(diag_get(), e);					\
+	errno = save_errno;						\
+} while (0)
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/src/lib/core/exception.cc b/src/lib/core/exception.cc
index 0ab10c4bd..180cb0e97 100644
--- a/src/lib/core/exception.cc
+++ b/src/lib/core/exception.cc
@@ -114,6 +114,7 @@ Exception::~Exception()
 	if (this != &out_of_memory) {
 		assert(refs == 0);
 	}
+	TRASH((struct error *) this);
 }
 
 Exception::Exception(const struct type_info *type_arg, const char *file,
diff --git a/src/lua/error.lua b/src/lua/error.lua
index 7f249864a..bdc9c714d 100644
--- a/src/lua/error.lua
+++ b/src/lua/error.lua
@@ -24,12 +24,17 @@ struct error {
     char _file[DIAG_FILENAME_MAX];
     /* Error description. */
     char _errmsg[DIAG_ERRMSG_MAX];
+    struct error *_cause;
+    struct error *_effect;
 };
 
 char *
 exception_get_string(struct error *e, const struct method_info *method);
 int
 exception_get_int(struct error *e, const struct method_info *method);
+
+int
+error_set_prev(struct error *e, struct error *prev);
 ]]
 
 local REFLECTION_CACHE = {}
@@ -95,11 +100,37 @@ local function error_errno(err)
     return e
 end
 
+local function error_prev(err)
+    local e = err._cause;
+    if e ~= nil then
+        return e
+    else
+        return nil
+    end
+end
+
+local function error_set_prev(err, prev)
+    -- First argument must be error.
+    if not ffi.istype('struct error', err) then
+        error("Usage: error1:set_prev(error2)")
+    end
+    -- Second argument must be error or nil.
+    if not ffi.istype('struct error', prev) and prev ~= nil then
+        error("Usage: error1:set_prev(error2)")
+    end
+    local ok = ffi.C.error_set_prev(err, prev);
+    if ok ~= 0 then
+        error("Cycles are not allowed")
+    end
+
+end
+
 local error_fields = {
     ["type"]        = error_type;
     ["message"]     = error_message;
     ["trace"]       = error_trace;
     ["errno"]       = error_errno;
+    ["prev"]        = error_prev;
 }
 
 local function error_unpack(err)
@@ -143,6 +174,7 @@ local error_methods = {
     ["raise"] = error_raise;
     ["match"] = error_match; -- Tarantool 1.6 backward compatibility
     ["__serialize"] = error_serialize;
+    ["set_prev"] = error_set_prev;
 }
 
 local function error_index(err, key)
diff --git a/test/box/error.result b/test/box/error.result
index 0a7ec6553..3d07f6e64 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -505,3 +505,306 @@ box.error()
 space:drop()
  | ---
  | ...
+
+-- gh-1148: errors can be arranged into list (so called
+-- stacked diagnostics).
+--
+e1 = box.error.new({code = 111, reason = "cause"})
+ | ---
+ | ...
+assert(e1.prev == nil)
+ | ---
+ | - true
+ | ...
+e1:set_prev(e1)
+ | ---
+ | - error: 'builtin/error.lua: Cycles are not allowed'
+ | ...
+assert(e1.prev == nil)
+ | ---
+ | - true
+ | ...
+e2 = box.error.new({code = 111, reason = "cause of cause"})
+ | ---
+ | ...
+e1:set_prev(e2)
+ | ---
+ | ...
+assert(e1.prev == e2)
+ | ---
+ | - true
+ | ...
+e2:set_prev(e1)
+ | ---
+ | - error: 'builtin/error.lua: Cycles are not allowed'
+ | ...
+assert(e2.prev == nil)
+ | ---
+ | - true
+ | ...
+-- At this point stack is following: e1 -> e2
+-- Let's test following cases:
+-- 1. e3 -> e2, e1 -> NULL (e3:set_prev(e2))
+-- 2. e1 -> e3, e2 -> NULL (e1:set_prev(e3))
+-- 3. e3 -> e1 -> e2 (e3:set_prev(e1))
+-- 4. e1 -> e2 -> e3 (e2:set_prev(e3))
+--
+e3 = box.error.new({code = 111, reason = "another cause"})
+ | ---
+ | ...
+e3:set_prev(e2)
+ | ---
+ | ...
+assert(e3.prev == e2)
+ | ---
+ | - true
+ | ...
+assert(e2.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e1.prev == nil)
+ | ---
+ | - true
+ | ...
+
+-- Reset stack to e1 -> e2 and test case 2.
+--
+e1:set_prev(e2)
+ | ---
+ | ...
+assert(e2.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e3.prev == nil)
+ | ---
+ | - true
+ | ...
+e1:set_prev(e3)
+ | ---
+ | ...
+assert(e2.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e1.prev == e3)
+ | ---
+ | - true
+ | ...
+assert(e3.prev == nil)
+ | ---
+ | - true
+ | ...
+
+-- Reset stack to e1 -> e2 and test case 3.
+--
+e1:set_prev(e2)
+ | ---
+ | ...
+assert(e1.prev == e2)
+ | ---
+ | - true
+ | ...
+assert(e2.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e3.prev == nil)
+ | ---
+ | - true
+ | ...
+e3:set_prev(e1)
+ | ---
+ | ...
+assert(e1.prev == e2)
+ | ---
+ | - true
+ | ...
+assert(e2.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e3.prev == e1)
+ | ---
+ | - true
+ | ...
+
+-- Unlink errors and test case 4.
+--
+e1:set_prev(nil)
+ | ---
+ | ...
+e2:set_prev(nil)
+ | ---
+ | ...
+e3:set_prev(nil)
+ | ---
+ | ...
+e1:set_prev(e2)
+ | ---
+ | ...
+e2:set_prev(e3)
+ | ---
+ | ...
+assert(e1.prev == e2)
+ | ---
+ | - true
+ | ...
+assert(e2.prev == e3)
+ | ---
+ | - true
+ | ...
+assert(e3.prev == nil)
+ | ---
+ | - true
+ | ...
+
+-- Test circle detecting. At the moment stack is
+-- following: e1 -> e2 -> e3
+--
+e3:set_prev(e1)
+ | ---
+ | - error: 'builtin/error.lua: Cycles are not allowed'
+ | ...
+assert(e3.prev == nil)
+ | ---
+ | - true
+ | ...
+e3:set_prev(e2)
+ | ---
+ | - error: 'builtin/error.lua: Cycles are not allowed'
+ | ...
+assert(e3.prev == nil)
+ | ---
+ | - true
+ | ...
+
+-- Test splitting list into two ones.
+-- After that we will get two lists: e1->e2->e5 and e3->e4
+--
+e4 = box.error.new({code = 111, reason = "yet another cause"})
+ | ---
+ | ...
+e5 = box.error.new({code = 111, reason = "and another one"})
+ | ---
+ | ...
+e3:set_prev(e4)
+ | ---
+ | ...
+e2:set_prev(e5)
+ | ---
+ | ...
+assert(e1.prev == e2)
+ | ---
+ | - true
+ | ...
+assert(e2.prev == e5)
+ | ---
+ | - true
+ | ...
+assert(e3.prev == e4)
+ | ---
+ | - true
+ | ...
+assert(e5.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e4.prev == nil)
+ | ---
+ | - true
+ | ...
+
+-- Another splitting option: e1->e2 and e5->e3->e4
+-- But firstly restore to one single list e1->e2->e3->e4
+--
+e2:set_prev(e3)
+ | ---
+ | ...
+e5:set_prev(e3)
+ | ---
+ | ...
+assert(e1.prev == e2)
+ | ---
+ | - true
+ | ...
+assert(e2.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e5.prev == e3)
+ | ---
+ | - true
+ | ...
+assert(e3.prev == e4)
+ | ---
+ | - true
+ | ...
+assert(e4.prev == nil)
+ | ---
+ | - true
+ | ...
+
+-- In case error is destroyed, it unrefs reference counter
+-- of its previous error. In turn, box.error.clear() refs/unrefs
+-- only head and doesn't touch other errors.
+--
+e2:set_prev(nil)
+ | ---
+ | ...
+box.error.set(e1)
+ | ---
+ | ...
+assert(box.error.last() == e1)
+ | ---
+ | - true
+ | ...
+assert(box.error.last().prev == e2)
+ | ---
+ | - true
+ | ...
+box.error.clear()
+ | ---
+ | ...
+assert(box.error.last() == nil)
+ | ---
+ | - true
+ | ...
+assert(e1.prev == e2)
+ | ---
+ | - true
+ | ...
+assert(e2.code == 111)
+ | ---
+ | - true
+ | ...
+box.error.set(e1)
+ | ---
+ | ...
+box.error.clear()
+ | ---
+ | ...
+assert(e1.prev == e2)
+ | ---
+ | - true
+ | ...
+
+-- Set middle of an error stack into the diagnostics area.
+e1:set_prev(e2)
+ | ---
+ | ...
+e2:set_prev(e3)
+ | ---
+ | ...
+box.error.set(e2)
+ | ---
+ | ...
+assert(e1.prev == nil)
+ | ---
+ | - true
+ | ...
+assert(e2.prev == e3)
+ | ---
+ | - true
+ | ...
diff --git a/test/box/error.test.lua b/test/box/error.test.lua
index a0b7d3e78..ed7eb7565 100644
--- a/test/box/error.test.lua
+++ b/test/box/error.test.lua
@@ -109,3 +109,115 @@ box.error.clear()
 box.error()
 
 space:drop()
+
+-- gh-1148: errors can be arranged into list (so called
+-- stacked diagnostics).
+--
+e1 = box.error.new({code = 111, reason = "cause"})
+assert(e1.prev == nil)
+e1:set_prev(e1)
+assert(e1.prev == nil)
+e2 = box.error.new({code = 111, reason = "cause of cause"})
+e1:set_prev(e2)
+assert(e1.prev == e2)
+e2:set_prev(e1)
+assert(e2.prev == nil)
+-- At this point stack is following: e1 -> e2
+-- Let's test following cases:
+-- 1. e3 -> e2, e1 -> NULL (e3:set_prev(e2))
+-- 2. e1 -> e3, e2 -> NULL (e1:set_prev(e3))
+-- 3. e3 -> e1 -> e2 (e3:set_prev(e1))
+-- 4. e1 -> e2 -> e3 (e2:set_prev(e3))
+--
+e3 = box.error.new({code = 111, reason = "another cause"})
+e3:set_prev(e2)
+assert(e3.prev == e2)
+assert(e2.prev == nil)
+assert(e1.prev == nil)
+
+-- Reset stack to e1 -> e2 and test case 2.
+--
+e1:set_prev(e2)
+assert(e2.prev == nil)
+assert(e3.prev == nil)
+e1:set_prev(e3)
+assert(e2.prev == nil)
+assert(e1.prev == e3)
+assert(e3.prev == nil)
+
+-- Reset stack to e1 -> e2 and test case 3.
+--
+e1:set_prev(e2)
+assert(e1.prev == e2)
+assert(e2.prev == nil)
+assert(e3.prev == nil)
+e3:set_prev(e1)
+assert(e1.prev == e2)
+assert(e2.prev == nil)
+assert(e3.prev == e1)
+
+-- Unlink errors and test case 4.
+--
+e1:set_prev(nil)
+e2:set_prev(nil)
+e3:set_prev(nil)
+e1:set_prev(e2)
+e2:set_prev(e3)
+assert(e1.prev == e2)
+assert(e2.prev == e3)
+assert(e3.prev == nil)
+
+-- Test circle detecting. At the moment stack is
+-- following: e1 -> e2 -> e3
+--
+e3:set_prev(e1)
+assert(e3.prev == nil)
+e3:set_prev(e2)
+assert(e3.prev == nil)
+
+-- Test splitting list into two ones.
+-- After that we will get two lists: e1->e2->e5 and e3->e4
+--
+e4 = box.error.new({code = 111, reason = "yet another cause"})
+e5 = box.error.new({code = 111, reason = "and another one"})
+e3:set_prev(e4)
+e2:set_prev(e5)
+assert(e1.prev == e2)
+assert(e2.prev == e5)
+assert(e3.prev == e4)
+assert(e5.prev == nil)
+assert(e4.prev == nil)
+
+-- Another splitting option: e1->e2 and e5->e3->e4
+-- But firstly restore to one single list e1->e2->e3->e4
+--
+e2:set_prev(e3)
+e5:set_prev(e3)
+assert(e1.prev == e2)
+assert(e2.prev == nil)
+assert(e5.prev == e3)
+assert(e3.prev == e4)
+assert(e4.prev == nil)
+
+-- In case error is destroyed, it unrefs reference counter
+-- of its previous error. In turn, box.error.clear() refs/unrefs
+-- only head and doesn't touch other errors.
+--
+e2:set_prev(nil)
+box.error.set(e1)
+assert(box.error.last() == e1)
+assert(box.error.last().prev == e2)
+box.error.clear()
+assert(box.error.last() == nil)
+assert(e1.prev == e2)
+assert(e2.code == 111)
+box.error.set(e1)
+box.error.clear()
+assert(e1.prev == e2)
+
+-- Set middle of an error stack into the diagnostics area.
+e1:set_prev(e2)
+e2:set_prev(e3)
+box.error.set(e2)
+assert(e1.prev == nil)
+assert(e2.prev == e3)
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Tarantool-patches] [PATCH v3 2/6] box: use stacked diagnostic area for functional indexes
  2020-04-06 14:17 [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 1/6] box: introduce stacked diagnostic area Nikita Pettik
@ 2020-04-06 14:17 ` Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 3/6] box/error: clarify purpose of reference counting in struct error Nikita Pettik
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Nikita Pettik @ 2020-04-06 14:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

Since we've introduced stacked diagnostic in previous commit, let's use
it in the code implementing functional indexes.

Part of #1148
---
 src/box/key_list.c              | 12 ++++----
 src/box/lua/call.c              |  6 ++--
 test/engine/func_index.result   | 54 ++++++++++++++++++++++++++++-----
 test/engine/func_index.test.lua |  8 +++++
 4 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/src/box/key_list.c b/src/box/key_list.c
index 3d736b55f..7c1fa70e3 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,
+		diag_add(ClientError, ER_FUNC_INDEX_FUNC, index_def->name,
 			 space ? space_name(space) : "",
-			 diag_last_error(diag_get())->errmsg);
+			 "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,
+		diag_add(ClientError, ER_FUNC_INDEX_FUNC, index_def->name,
 			 space ? space_name(space) : "",
-			 diag_last_error(diag_get())->errmsg);
+			 "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 definition");
 		return -1;
 	}
 
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index f1bbde7f0..5d3579eff 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -687,9 +687,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/test/engine/func_index.result b/test/engine/func_index.result
index 84cb83022..a827c929f 100644
--- a/test/engine/func_index.result
+++ b/test/engine/func_index.result
@@ -5,6 +5,14 @@ test_run = require('test_run').new()
 engine = test_run:get_cfg('engine')
  | ---
  | ...
+test_run:cmd("push filter \"file: .*\" to \"file: <filename>\"")
+ | ---
+ | - true
+ | ...
+test_run:cmd("push filter \"line: .*\" to \"line: <line>\"")
+ | ---
+ | - true
+ | ...
 
 --
 -- gh-1260: Func index.
@@ -158,8 +166,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 definition'
  | ...
 idx:drop()
  | ---
@@ -197,8 +204,7 @@ 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 definition'
  | ...
 idx:drop()
  | ---
@@ -217,8 +223,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 definition'
  | ...
 idx:drop()
  | ---
@@ -264,8 +269,41 @@ 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'
+ | ...
+e = box.error.last()
+ | ---
+ | ...
+e:unpack()
+ | ---
+ | - code: 198
+ |   trace:
+ |   - file: <filename>
+ |     line: <line>
+ |   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)'
+ | ...
+e = e.prev
+ | ---
+ | ...
+e:unpack()
+ | ---
+ | - type: LuajitError
+ |   message: '[string "return function(tuple)                 local ..."]:1: attempt
+ |     to call global ''require'' (a nil value)'
+ |   trace:
+ |   - file: <filename>
+ |     line: <line>
+ | ...
+e = e.prev
+ | ---
+ | ...
+e == nil
+ | ---
+ | - true
  | ...
 idx:drop()
  | ---
diff --git a/test/engine/func_index.test.lua b/test/engine/func_index.test.lua
index f31162c97..5db588c1f 100644
--- a/test/engine/func_index.test.lua
+++ b/test/engine/func_index.test.lua
@@ -1,5 +1,7 @@
 test_run = require('test_run').new()
 engine = test_run:get_cfg('engine')
+test_run:cmd("push filter \"file: .*\" to \"file: <filename>\"")
+test_run:cmd("push filter \"line: .*\" to \"line: <line>\"")
 
 --
 -- gh-1260: Func index.
@@ -99,6 +101,12 @@ 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})
+e = box.error.last()
+e:unpack()
+e = e.prev
+e:unpack()
+e = e.prev
+e == nil
 idx:drop()
 
 -- Remove old persistent functions
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Tarantool-patches] [PATCH v3 3/6] box/error: clarify purpose of reference counting in struct error
  2020-04-06 14:17 [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 1/6] box: introduce stacked diagnostic area Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 2/6] box: use stacked diagnostic area for functional indexes Nikita Pettik
@ 2020-04-06 14:17 ` Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 4/6] iproto: refactor error encoding with mpstream Nikita Pettik
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Nikita Pettik @ 2020-04-06 14:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

---
 src/lib/core/diag.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/lib/core/diag.h b/src/lib/core/diag.h
index 9c238f5a2..7a5454d1c 100644
--- a/src/lib/core/diag.h
+++ b/src/lib/core/diag.h
@@ -71,6 +71,15 @@ struct error {
 	error_f raise;
 	error_f log;
 	const struct type_info *type;
+	/**
+	 * Reference counting is basically required since
+	 * instances of this structure are available in Lua
+	 * as well (as cdata with overloaded fields and methods
+	 * by the means of introspection). Thus, it may turn
+	 * out that Lua's GC attempts at releasing object
+	 * meanwhile it is still used in C internals or vice
+	 * versa. For details see luaT_pusherror().
+	 */
 	int refs;
 	/**
 	 * Errno at the moment of the error
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Tarantool-patches] [PATCH v3 4/6] iproto: refactor error encoding with mpstream
  2020-04-06 14:17 [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Nikita Pettik
                   ` (2 preceding siblings ...)
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 3/6] box/error: clarify purpose of reference counting in struct error Nikita Pettik
@ 2020-04-06 14:17 ` Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 5/6] box: always promote error created via box.error() to diag Nikita Pettik
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Nikita Pettik @ 2020-04-06 14:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

From: Kirill Shcherbatov <kshcherbatov@tarantool.org>

Refactor iproto_reply_error and iproto_write_error with a new
mpstream-based helper mpstream_iproto_encode_error that encodes
error object for iproto protocol on a given stream object.
Previously each routine implemented an own error encoding, but
with the increasing complexity of encode operation with following
patches we need a uniform way to do it.

The iproto_write_error routine starts using region location
to use region-based mpstream. It is not a problem itself, because
errors reporting is not really performance-critical path.

Needed for #1148
---
 src/box/xrow.c | 70 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 49 insertions(+), 21 deletions(-)

diff --git a/src/box/xrow.c b/src/box/xrow.c
index 5e3cb0709..28adbdc1f 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -42,6 +42,7 @@
 #include "vclock.h"
 #include "scramble.h"
 #include "iproto_constants.h"
+#include "mpstream.h"
 
 static_assert(IPROTO_DATA < 0x7f && IPROTO_METADATA < 0x7f &&
 	      IPROTO_SQL_INFO < 0x7f, "encoded IPROTO_BODY keys must fit into "\
@@ -381,10 +382,6 @@ static const struct iproto_body_bin iproto_body_bin = {
 	0x81, IPROTO_DATA, 0xdd, 0
 };
 
-static const struct iproto_body_bin iproto_error_bin = {
-	0x81, IPROTO_ERROR, 0xdb, 0
-};
-
 /** Return a 4-byte numeric error code, with status flags. */
 static inline uint32_t
 iproto_encode_error(uint32_t error)
@@ -478,46 +475,77 @@ iproto_reply_vote(struct obuf *out, const struct ballot *ballot,
 	return 0;
 }
 
+static void
+mpstream_error_handler(void *error_ctx)
+{
+	*(bool *)error_ctx = true;
+}
+
+static void
+mpstream_iproto_encode_error(struct mpstream *stream, const struct error *error)
+{
+	mpstream_encode_map(stream, 2);
+	mpstream_encode_uint(stream, IPROTO_ERROR);
+	mpstream_encode_str(stream, error->errmsg);
+}
+
 int
 iproto_reply_error(struct obuf *out, const struct error *e, uint64_t sync,
 		   uint32_t schema_version)
 {
-	uint32_t msg_len = strlen(e->errmsg);
-	uint32_t errcode = box_error_code(e);
-
-	struct iproto_body_bin body = iproto_error_bin;
 	char *header = (char *)obuf_alloc(out, IPROTO_HEADER_LEN);
 	if (header == NULL)
 		return -1;
 
+	bool is_error = false;
+	struct mpstream stream;
+	mpstream_init(&stream, out, obuf_reserve_cb, obuf_alloc_cb,
+		      mpstream_error_handler, &is_error);
+
+	uint32_t used = obuf_size(out);
+	mpstream_iproto_encode_error(&stream, e);
+	mpstream_flush(&stream);
+
+	uint32_t errcode = box_error_code(e);
 	iproto_header_encode(header, iproto_encode_error(errcode), sync,
-			     schema_version, sizeof(body) + msg_len);
-	body.v_data_len = mp_bswap_u32(msg_len);
+			     schema_version, obuf_size(out) - used);
+
 	/* Malformed packet appears to be a lesser evil than abort. */
-	return obuf_dup(out, &body, sizeof(body)) != sizeof(body) ||
-	       obuf_dup(out, e->errmsg, msg_len) != msg_len ? -1 : 0;
+	return is_error ? -1 : 0;
 }
 
 void
 iproto_write_error(int fd, const struct error *e, uint32_t schema_version,
 		   uint64_t sync)
 {
-	uint32_t msg_len = strlen(e->errmsg);
-	uint32_t errcode = box_error_code(e);
+	bool is_error = false;
+	struct mpstream stream;
+	struct region *region = &fiber()->gc;
+	mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
+		      mpstream_error_handler, &is_error);
+
+	size_t region_svp = region_used(region);
+	mpstream_iproto_encode_error(&stream, e);
+	mpstream_flush(&stream);
+	if (is_error)
+		goto cleanup;
+
+	size_t payload_size = region_used(region) - region_svp;
+	char *payload = region_join(region, payload_size);
+	if (payload == NULL)
+		goto cleanup;
 
+	uint32_t errcode = box_error_code(e);
 	char header[IPROTO_HEADER_LEN];
-	struct iproto_body_bin body = iproto_error_bin;
-
 	iproto_header_encode(header, iproto_encode_error(errcode), sync,
-			     schema_version, sizeof(body) + msg_len);
-
-	body.v_data_len = mp_bswap_u32(msg_len);
+			     schema_version, payload_size);
 
 	ssize_t unused;
 	unused = write(fd, header, sizeof(header));
-	unused = write(fd, &body, sizeof(body));
-	unused = write(fd, e->errmsg, msg_len);
+	unused = write(fd, payload, payload_size);
 	(void) unused;
+cleanup:
+	region_truncate(region, region_svp);
 }
 
 int
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Tarantool-patches] [PATCH v3 5/6] box: always promote error created via box.error() to diag
  2020-04-06 14:17 [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Nikita Pettik
                   ` (3 preceding siblings ...)
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 4/6] iproto: refactor error encoding with mpstream Nikita Pettik
@ 2020-04-06 14:17 ` Nikita Pettik
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 6/6] iproto: support error stacked diagnostic area Nikita Pettik
  2020-04-07 11:13 ` [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Kirill Yukhin
  6 siblings, 0 replies; 8+ messages in thread
From: Nikita Pettik @ 2020-04-06 14:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

This patch makes box.error() always promote error to the diagnostic
area despite of passed arguments.

Closes #4829

@TarantoolBot document
Title: always promote error created via box.error() to diag

box.error() is able to accept two types of argument: either pair of code
and reason (box.error{code = 555, reason = 'Arbitrary message'}) or error
object (box.error(err)). In the first case error is promoted to
diagnostic area, meanwhile in the latter - it is not:
```
e1 = box.error.new({code = 111, reason = "cause"})
box.error({code = 111, reason = "err"})
- error: err
box.error.last()
- err
box.error(e1)
- error: cause
box.error.last()
- err
```
From now box.error(e1) sets error to diagnostic area as well:
```
box.error(e1)
- error: cause
box.error.last()
- cause
```
---
 src/box/lua/error.cc    | 13 ++++++++++---
 test/box/error.result   | 22 ++++++++++++++++++++++
 test/box/error.test.lua |  8 ++++++++
 3 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
index 08c2d983d..b2625bf5f 100644
--- a/src/box/lua/error.cc
+++ b/src/box/lua/error.cc
@@ -114,9 +114,16 @@ luaT_error_call(lua_State *L)
 			return luaT_error(L);
 		return 0;
 	}
-	if (lua_gettop(L) == 2 && luaL_iserror(L, 2))
-		return lua_error(L);
-	struct error *e = luaT_error_create(L, 2);
+	struct error *e = NULL;
+	if (lua_gettop(L) == 2) {
+		e = luaL_iserror(L, 2);
+		if (e != NULL) {
+			/* Re-set error to diag area. */
+			diag_set_error(&fiber()->diag, e);
+			return lua_error(L);
+		}
+	}
+	e = luaT_error_create(L, 2);
 	if (e == NULL)
 		return luaL_error(L, "box.error(): bad arguments");
 	diag_set_error(&fiber()->diag, e);
diff --git a/test/box/error.result b/test/box/error.result
index 3d07f6e64..234c26371 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -808,3 +808,25 @@ assert(e2.prev == e3)
  | ---
  | - true
  | ...
+
+-- gh-4829: always promote error created via box.error() to
+-- diagnostic area.
+e1 = box.error.new({code = 111, reason = "cause"})
+ | ---
+ | ...
+box.error({code = 111, reason = "err"})
+ | ---
+ | - error: err
+ | ...
+box.error.last()
+ | ---
+ | - err
+ | ...
+box.error(e1)
+ | ---
+ | - error: cause
+ | ...
+assert(box.error.last() == e1)
+ | ---
+ | - true
+ | ...
diff --git a/test/box/error.test.lua b/test/box/error.test.lua
index ed7eb7565..41baed52d 100644
--- a/test/box/error.test.lua
+++ b/test/box/error.test.lua
@@ -221,3 +221,11 @@ e2:set_prev(e3)
 box.error.set(e2)
 assert(e1.prev == nil)
 assert(e2.prev == e3)
+
+-- gh-4829: always promote error created via box.error() to
+-- diagnostic area.
+e1 = box.error.new({code = 111, reason = "cause"})
+box.error({code = 111, reason = "err"})
+box.error.last()
+box.error(e1)
+assert(box.error.last() == e1)
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Tarantool-patches] [PATCH v3 6/6] iproto: support error stacked diagnostic area
  2020-04-06 14:17 [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Nikita Pettik
                   ` (4 preceding siblings ...)
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 5/6] box: always promote error created via box.error() to diag Nikita Pettik
@ 2020-04-06 14:17 ` Nikita Pettik
  2020-04-07 11:13 ` [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Kirill Yukhin
  6 siblings, 0 replies; 8+ messages in thread
From: Nikita Pettik @ 2020-04-06 14:17 UTC (permalink / raw)
  To: tarantool-patches; +Cc: v.shpilevoy

This patch introduces support of stacked errors in IProto protocol and
in net.box module.

Closes #1148

@TarantoolBot document
Title: Stacked error diagnostic area

Starting from now errors can be organized into lists. To achieve this
Lua table representing error object is extended with .prev field and
e:set_prev(err) method. .prev field returns previous error if any exist.
e:set_prev(err) method expects err to be error object or nil and sets
err as previous error of e. For instance:
```
e1 = box.error.new({code = 111, reason = "cause"})
e2 = box.error.new({code = 111, reason = "cause of cause"})

e1:set_prev(e2)
assert(e1.prev == e2) -- true
```
Cycles are not allowed for error lists:
```
e2:set_prev(e1)
- error: 'builtin/error.lua: Cycles are not allowed'
```
Nil is valid input to :set_prev() method:
```
e1:set_prev(nil)
assert(e1.prev == nil) -- true
```
Note that error can be 'previous' only to the one error at once:
```
e1:set_prev(e2)
e3:set_prev(e2)
assert(e1.prev == nil) -- true
assert(e3.prev == e2) -- true
```
Setting previous error does not erase its own previous members:
```
-- e1 -> e2 -> e3 -> e4
e1:set_prev(e2)
e2:set_prev(e3)
e3:set_prev(e4)
e2:set_prev(e5)
-- Now there are two lists: e1->e2->e5 and e3->e4
assert(e1.prev == e2) -- true
assert(e2.prev == e5) -- true
assert(e3.prev == e4) -- true
```
Alternatively:
```
e1:set_prev(e2)
e2:set_prev(e3)
e3:set_prev(e4)
e5:set_prev(e3)
-- Now there are two lists: e1->e2 and e5->e3->e4
assert(e1.prev == e2) -- true
assert(e2.prev == nil) -- true
assert(e5.prev == e3) -- true
assert(e3.prev == e4) -- true
``
Stacked diagnostics is also supported by IProto protocol. Now responses
containing errors always (even if there's only one error to be returned)
include new IProto key: IPROTO_ERROR_STACK (0x51). So, body corresponding to
error response now looks like:
```
MAP{IPROTO_ERROR : string, IPROTO_ERROR_STACK : ARRAY[MAP{ERROR_CODE : uint, ERROR_MESSAGE : string}, MAP{...}, ...]}
```
where IPROTO_ERROR is 0x31 key, IPROTO_ERROR_STACK is 0x52, ERROR_CODE
is 0x01 and ERROR_MESSAGE is 0x02.
Instances of older versions (without support of stacked errors in
protocol) simply ignore unknown keys and still rely only on IPROTO_ERROR
key.
---
 src/box/error.cc           |  21 +++++
 src/box/error.h            |  16 ++++
 src/box/iproto_constants.h |   6 ++
 src/box/lua/net_box.lua    |  30 +++++-
 src/box/xrow.c             |  80 ++++++++++++++--
 test/box-py/iproto.result  |   6 +-
 test/box-py/iproto.test.py |   6 +-
 test/box/iproto.result     | 142 ++++++++++++++++++++++++++++
 test/box/iproto.test.lua   |  66 +++++++++++++
 test/box/net.box.result    |  71 ++++++++++++++
 test/box/net.box.test.lua  |  31 +++++++
 test/unit/xrow.cc          | 184 ++++++++++++++++++++++++++++++++++++-
 test/unit/xrow.result      |  27 +++++-
 13 files changed, 668 insertions(+), 18 deletions(-)
 create mode 100644 test/box/iproto.result
 create mode 100644 test/box/iproto.test.lua

diff --git a/src/box/error.cc b/src/box/error.cc
index 8e77c2e9e..233b312a2 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -102,6 +102,27 @@ box_error_new(const char *file, unsigned line, uint32_t code,
 	return e;
 }
 
+int
+box_error_add(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) {
+		client_error->m_errcode = code;
+		va_list ap;
+		va_start(ap, fmt);
+		error_vformat_msg(e, fmt, ap);
+		va_end(ap);
+	}
+	struct diag *d = &fiber()->diag;
+	if (diag_is_empty(d))
+		diag_set_error(d, e);
+	else
+		diag_add_error(d, e);
+	return -1;
+}
+
 /* }}} */
 
 struct rmean *rmean_error = NULL;
diff --git a/src/box/error.h b/src/box/error.h
index ef3ccf104..ca5d5b2ae 100644
--- a/src/box/error.h
+++ b/src/box/error.h
@@ -137,6 +137,22 @@ box_error_set(const char *file, unsigned line, uint32_t code,
 
 /** \endcond public */
 
+/**
+ * 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 format (const char * ) - printf()-like format string
+ * \param ... - format arguments
+ * \returns -1 for convention use
+ *
+ * \sa enum box_error_code
+ */
+int
+box_error_add(const char *file, unsigned line, uint32_t code,
+	      const char *fmt, ...);
+
 /**
  * Construct error object without setting it in the diagnostics
  * area. On the memory allocation fail returns NULL.
diff --git a/src/box/iproto_constants.h b/src/box/iproto_constants.h
index f9d413a31..7ed829645 100644
--- a/src/box/iproto_constants.h
+++ b/src/box/iproto_constants.h
@@ -126,6 +126,7 @@ enum iproto_key {
 	/* Leave a gap between SQL keys and additional request keys */
 	IPROTO_REPLICA_ANON = 0x50,
 	IPROTO_ID_FILTER = 0x51,
+	IPROTO_ERROR_STACK = 0x52,
 	IPROTO_KEY_MAX
 };
 
@@ -150,6 +151,11 @@ enum iproto_ballot_key {
 	IPROTO_BALLOT_IS_LOADING = 0x04,
 };
 
+enum iproto_error_key {
+	IPROTO_ERROR_CODE = 0x01,
+	IPROTO_ERROR_MESSAGE = 0x02,
+};
+
 #define bit(c) (1ULL<<IPROTO_##c)
 
 #define IPROTO_HEAD_BMAP (bit(REQUEST_TYPE) | bit(SYNC) | bit(REPLICA_ID) |\
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 3f611c027..07fa54c38 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -44,6 +44,9 @@ local SQL_INFO_ROW_COUNT_KEY = 0
 local IPROTO_FIELD_NAME_KEY = 0
 local IPROTO_DATA_KEY      = 0x30
 local IPROTO_ERROR_KEY     = 0x31
+local IPROTO_ERROR_STACK   = 0x52
+local IPROTO_ERROR_CODE    = 0x01
+local IPROTO_ERROR_MESSAGE = 0x02
 local IPROTO_GREETING_SIZE = 128
 local IPROTO_CHUNK_KEY     = 128
 local IPROTO_OK_KEY        = 0
@@ -277,8 +280,22 @@ local function create_transport(host, port, user, password, callback,
     --
     function request_index:result()
         if self.errno then
-            return nil, box.error.new({code = self.errno,
-                                       reason = self.response})
+            if type(self.response) == 'table' then
+                -- Decode and fill in error stack.
+                local prev = nil
+                for i = #self.response, 1, -1 do
+                    local error = self.response[i]
+                    local code = error[IPROTO_ERROR_CODE]
+                    local msg = error[IPROTO_ERROR_MESSAGE]
+                    local new_err = box.error.new({code = code, reason = msg})
+                    new_err:set_prev(prev)
+                    prev = new_err
+                end
+                return nil, prev
+            else
+                return nil, box.error.new({code = self.errno,
+                                           reason = self.response})
+            end
         elseif not self.id then
             return self.response
         elseif not worker_fiber then
@@ -559,7 +576,14 @@ local function create_transport(host, port, user, password, callback,
             body, body_end_check = decode(body_rpos)
             assert(body_end == body_end_check, "invalid xrow length")
             request.errno = band(status, IPROTO_ERRNO_MASK)
-            request.response = body[IPROTO_ERROR_KEY]
+            -- IPROTO_ERROR_STACK comprises error encoded with
+            -- IPROTO_ERROR_KEY, so we may ignore content of that key.
+            if body[IPROTO_ERROR_STACK] ~= nil then
+                request.response = body[IPROTO_ERROR_STACK]
+                assert(type(request.response) == 'table')
+            else
+                request.response = body[IPROTO_ERROR_KEY]
+            end
             request.cond:broadcast()
             return
         end
diff --git a/src/box/xrow.c b/src/box/xrow.c
index 28adbdc1f..be026a43c 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -487,6 +487,19 @@ mpstream_iproto_encode_error(struct mpstream *stream, const struct error *error)
 	mpstream_encode_map(stream, 2);
 	mpstream_encode_uint(stream, IPROTO_ERROR);
 	mpstream_encode_str(stream, error->errmsg);
+
+	uint32_t err_cnt = 0;
+	for (const struct error *it = error; it != NULL; it = it->cause)
+		err_cnt++;
+	mpstream_encode_uint(stream, IPROTO_ERROR_STACK);
+	mpstream_encode_array(stream, err_cnt);
+	for (const struct error *it = error; it != NULL; it = it->cause) {
+		mpstream_encode_map(stream, 2);
+		mpstream_encode_uint(stream, IPROTO_ERROR_CODE);
+		mpstream_encode_uint(stream, box_error_code(it));
+		mpstream_encode_uint(stream, IPROTO_ERROR_MESSAGE);
+		mpstream_encode_str(stream, it->errmsg);
+	}
 }
 
 int
@@ -1070,6 +1083,51 @@ xrow_encode_auth(struct xrow_header *packet, const char *salt, size_t salt_len,
 	return 0;
 }
 
+static int
+iproto_decode_error_stack(const char **pos)
+{
+	const char *reason = NULL;
+	static_assert(TT_STATIC_BUF_LEN >= DIAG_ERRMSG_MAX, "static buffer is "\
+		      "expected to be larger than error message max length");
+	/*
+	 * Erase previously set diag errors. It is required since
+	 * box_error_add() does not replace previous errors.
+	 */
+	box_error_clear();
+	if (mp_typeof(**pos) != MP_ARRAY)
+		return -1;
+	uint32_t stack_sz = mp_decode_array(pos);
+	for (uint32_t i = 0; i < stack_sz; i++) {
+		uint64_t code = 0;
+		if (mp_typeof(**pos) != MP_MAP)
+			return -1;
+		uint32_t map_sz = mp_decode_map(pos);
+		for (uint32_t key_idx = 0; key_idx < map_sz; key_idx++) {
+			if (mp_typeof(**pos) != MP_UINT)
+				return -1;
+			uint64_t key = mp_decode_uint(pos);
+			if (key == IPROTO_ERROR_CODE) {
+				if (mp_typeof(**pos) != MP_UINT)
+					return -1;
+				code = mp_decode_uint(pos);
+				if (code > UINT32_MAX)
+					return -1;
+			} else if (key == IPROTO_ERROR_MESSAGE) {
+				if (mp_typeof(**pos) != MP_STR)
+					return -1;
+				uint32_t len;
+				const char *str = mp_decode_str(pos, &len);
+				reason = tt_cstr(str, len);
+			} else {
+				mp_next(pos);
+				continue;
+			}
+		}
+		box_error_add(__FILE__, __LINE__, code, reason);
+	}
+	return 0;
+}
+
 void
 xrow_decode_error(struct xrow_header *row)
 {
@@ -1096,15 +1154,25 @@ xrow_decode_error(struct xrow_header *row)
 			continue;
 		}
 		uint8_t key = mp_decode_uint(&pos);
-		if (key != IPROTO_ERROR || mp_typeof(*pos) != MP_STR) {
-			mp_next(&pos); /* value */
+		if (key == IPROTO_ERROR && mp_typeof(*pos) == MP_STR) {
+			/*
+			 * Obsolete way of sending error responses.
+			 * To be deprecated but still should be supported
+			 * to not break backward compatibility.
+			 */
+			uint32_t len;
+			const char *str = mp_decode_str(&pos, &len);
+			snprintf(error, sizeof(error), "%.*s", len, str);
+			box_error_set(__FILE__, __LINE__, code, error);
+		} else if (key == IPROTO_ERROR_STACK) {
+			if (iproto_decode_error_stack(&pos) != 0)
+				goto error;
+		} else {
+			mp_next(&pos);
 			continue;
 		}
-
-		uint32_t len;
-		const char *str = mp_decode_str(&pos, &len);
-		snprintf(error, sizeof(error), "%.*s", len, str);
 	}
+	return;
 
 error:
 	box_error_set(__FILE__, __LINE__, code, error);
diff --git a/test/box-py/iproto.result b/test/box-py/iproto.result
index 900e6e24f..04e2e220c 100644
--- a/test/box-py/iproto.result
+++ b/test/box-py/iproto.result
@@ -169,9 +169,9 @@ box.schema.user.revoke('guest', 'read,write,execute', 'universe')
 # Test bugs gh-272, gh-1654 if the packet was incorrect, respond with
 # an error code and do not close connection
 
-sync=0, {49: 'Invalid MsgPack - packet header'}
-sync=1234, {49: "Missing mandatory field 'space id' in request"}
-sync=5678, {49: "Read access to space '_user' is denied for user 'guest'"}
+sync=0, Invalid MsgPack - packet header
+sync=1234, Missing mandatory field 'space id' in request
+sync=5678, Read access to space '_user' is denied for user 'guest'
 space = box.schema.space.create('test_index_base', { id = 568 })
 ---
 ...
diff --git a/test/box-py/iproto.test.py b/test/box-py/iproto.test.py
index 77637d8ed..cdd1a71c5 100644
--- a/test/box-py/iproto.test.py
+++ b/test/box-py/iproto.test.py
@@ -355,15 +355,15 @@ s = c._socket
 header = { "hello": "world"}
 body = { "bug": 272 }
 resp = test_request(header, body)
-print 'sync=%d, %s' % (resp['header'][IPROTO_SYNC], resp['body'])
+print 'sync=%d, %s' % (resp['header'][IPROTO_SYNC], resp['body'].get(IPROTO_ERROR))
 header = { IPROTO_CODE : REQUEST_TYPE_SELECT }
 header[IPROTO_SYNC] = 1234
 resp = test_request(header, body)
-print 'sync=%d, %s' % (resp['header'][IPROTO_SYNC], resp['body'])
+print 'sync=%d, %s' % (resp['header'][IPROTO_SYNC], resp['body'].get(IPROTO_ERROR))
 header[IPROTO_SYNC] = 5678
 body = { IPROTO_SPACE_ID: 304, IPROTO_KEY: [], IPROTO_LIMIT: 1 }
 resp = test_request(header, body)
-print 'sync=%d, %s' % (resp['header'][IPROTO_SYNC], resp['body'])
+print 'sync=%d, %s' % (resp['header'][IPROTO_SYNC], resp['body'].get(IPROTO_ERROR))
 c.close()
 
 
diff --git a/test/box/iproto.result b/test/box/iproto.result
new file mode 100644
index 000000000..b6dc7ed4c
--- /dev/null
+++ b/test/box/iproto.result
@@ -0,0 +1,142 @@
+net_box = require('net.box')
+---
+...
+urilib = require('uri')
+---
+...
+msgpack = require('msgpack')
+---
+...
+test_run = require('test_run').new()
+---
+...
+IPROTO_REQUEST_TYPE   = 0x00
+---
+...
+IPROTO_SYNC           = 0x01
+---
+...
+IPROTO_CALL           = 0x0A
+---
+...
+IPROTO_FUNCTION_NAME  = 0x22
+---
+...
+IPROTO_TUPLE          = 0x21
+---
+...
+IPROTO_ERROR          = 0x31
+---
+...
+IPROTO_ERROR_STACK    = 0x52
+---
+...
+IPROTO_ERROR_CODE     = 0x01
+---
+...
+IPROTO_ERROR_MESSAGE  = 0x02
+---
+...
+-- gh-1148: test capabilities of stacked diagnostics bypassing net.box.
+--
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+stack_err = function()
+    local e1 = box.error.new({code = 111, reason = "e1"})
+    local e2 = box.error.new({code = 111, reason = "e2"})
+    local e3 = box.error.new({code = 111, reason = "e3"})
+    assert(e1 ~= nil)
+    e2:set_prev(e1)
+    assert(e2.prev == e1)
+    e3:set_prev(e2)
+    box.error(e3)
+end;
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+box.schema.user.grant('guest', 'read, write, execute', 'universe')
+---
+...
+next_request_id = 16
+---
+...
+header = { \
+    [IPROTO_REQUEST_TYPE] = IPROTO_CALL, \
+    [IPROTO_SYNC]         = next_request_id, \
+}
+---
+...
+body = { \
+    [IPROTO_FUNCTION_NAME] = 'stack_err', \
+    [IPROTO_TUPLE]    = box.tuple.new({nil}) \
+}
+---
+...
+uri = urilib.parse(box.cfg.listen)
+---
+...
+sock = net_box.establish_connection(uri.host, uri.service)
+---
+...
+response = iproto_request(sock, header, body)
+---
+...
+sock:close()
+---
+- true
+...
+-- Both keys (obsolete and stack ones) are present in response.
+--
+assert(response.body[IPROTO_ERROR_STACK] ~= nil)
+---
+- true
+...
+assert(response.body[IPROTO_ERROR] ~= nil)
+---
+- true
+...
+err = response.body[IPROTO_ERROR_STACK][1]
+---
+...
+assert(err[IPROTO_ERROR_MESSAGE] == response.body[IPROTO_ERROR])
+---
+- true
+...
+assert(err[IPROTO_ERROR_MESSAGE] == 'e3')
+---
+- true
+...
+assert(err[IPROTO_ERROR_CODE] == 111)
+---
+- true
+...
+err = response.body[IPROTO_ERROR_STACK][2]
+---
+...
+assert(err[IPROTO_ERROR_MESSAGE] == 'e2')
+---
+- true
+...
+assert(err[IPROTO_ERROR_CODE] == 111)
+---
+- true
+...
+err = response.body[IPROTO_ERROR_STACK][3]
+---
+...
+assert(err[IPROTO_ERROR_MESSAGE] == 'e1')
+---
+- true
+...
+assert(err[IPROTO_ERROR_CODE] == 111)
+---
+- true
+...
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+---
+...
diff --git a/test/box/iproto.test.lua b/test/box/iproto.test.lua
new file mode 100644
index 000000000..6402a22ba
--- /dev/null
+++ b/test/box/iproto.test.lua
@@ -0,0 +1,66 @@
+net_box = require('net.box')
+urilib = require('uri')
+msgpack = require('msgpack')
+test_run = require('test_run').new()
+
+IPROTO_REQUEST_TYPE   = 0x00
+
+IPROTO_SYNC           = 0x01
+IPROTO_CALL           = 0x0A
+IPROTO_FUNCTION_NAME  = 0x22
+IPROTO_TUPLE          = 0x21
+IPROTO_ERROR          = 0x31
+IPROTO_ERROR_STACK    = 0x52
+IPROTO_ERROR_CODE     = 0x01
+IPROTO_ERROR_MESSAGE  = 0x02
+
+-- gh-1148: test capabilities of stacked diagnostics bypassing net.box.
+--
+test_run:cmd("setopt delimiter ';'")
+stack_err = function()
+    local e1 = box.error.new({code = 111, reason = "e1"})
+    local e2 = box.error.new({code = 111, reason = "e2"})
+    local e3 = box.error.new({code = 111, reason = "e3"})
+    assert(e1 ~= nil)
+    e2:set_prev(e1)
+    assert(e2.prev == e1)
+    e3:set_prev(e2)
+    box.error(e3)
+end;
+test_run:cmd("setopt delimiter ''");
+box.schema.user.grant('guest', 'read, write, execute', 'universe')
+
+next_request_id = 16
+header = { \
+    [IPROTO_REQUEST_TYPE] = IPROTO_CALL, \
+    [IPROTO_SYNC]         = next_request_id, \
+}
+
+body = { \
+    [IPROTO_FUNCTION_NAME] = 'stack_err', \
+    [IPROTO_TUPLE]    = box.tuple.new({nil}) \
+}
+
+uri = urilib.parse(box.cfg.listen)
+sock = net_box.establish_connection(uri.host, uri.service)
+
+response = iproto_request(sock, header, body)
+sock:close()
+
+-- Both keys (obsolete and stack ones) are present in response.
+--
+assert(response.body[IPROTO_ERROR_STACK] ~= nil)
+assert(response.body[IPROTO_ERROR] ~= nil)
+
+err = response.body[IPROTO_ERROR_STACK][1]
+assert(err[IPROTO_ERROR_MESSAGE] == response.body[IPROTO_ERROR])
+assert(err[IPROTO_ERROR_MESSAGE] == 'e3')
+assert(err[IPROTO_ERROR_CODE] == 111)
+err = response.body[IPROTO_ERROR_STACK][2]
+assert(err[IPROTO_ERROR_MESSAGE] == 'e2')
+assert(err[IPROTO_ERROR_CODE] == 111)
+err = response.body[IPROTO_ERROR_STACK][3]
+assert(err[IPROTO_ERROR_MESSAGE] == 'e1')
+assert(err[IPROTO_ERROR_CODE] == 111)
+
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
diff --git a/test/box/net.box.result b/test/box/net.box.result
index e3dabf7d9..6475707ef 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3902,6 +3902,77 @@ sock:close()
 ---
 - true
 ...
+-- gh-1148: test stacked diagnostics.
+--
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+stack_err = function()
+    local e1 = box.error.new({code = 111, reason = "e1"})
+    local e2 = box.error.new({code = 111, reason = "e2"})
+    local e3 = box.error.new({code = 111, reason = "e3"})
+    assert(e1 ~= nil)
+    e2:set_prev(e1)
+    assert(e2.prev == e1)
+    e3:set_prev(e2)
+    box.error(e3)
+end;
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+---
+...
+c = net.connect(box.cfg.listen)
+---
+...
+f = function(...) return c:call(...) end
+---
+...
+r, e3 = pcall(f, 'stack_err')
+---
+...
+assert(r == false)
+---
+- true
+...
+e3
+---
+- e3
+...
+e2 = e3.prev
+---
+...
+assert(e2 ~= nil)
+---
+- true
+...
+e2
+---
+- e2
+...
+e1 = e2.prev
+---
+...
+assert(e1 ~= nil)
+---
+- true
+...
+e1
+---
+- e1
+...
+assert(e1.prev == nil)
+---
+- true
+...
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+---
+...
 test_run:wait_log('default', 'Got a corrupted row.*', nil, 10)
 ---
 - 'Got a corrupted row:'
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 8e65ff470..72a4207ee 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1574,6 +1574,37 @@ data = string.fromhex('3C'..string.rep(require('digest').sha1_hex('bcde'), 3))
 sock:write(data)
 sock:close()
 
+-- gh-1148: test stacked diagnostics.
+--
+test_run:cmd("setopt delimiter ';'")
+stack_err = function()
+    local e1 = box.error.new({code = 111, reason = "e1"})
+    local e2 = box.error.new({code = 111, reason = "e2"})
+    local e3 = box.error.new({code = 111, reason = "e3"})
+    assert(e1 ~= nil)
+    e2:set_prev(e1)
+    assert(e2.prev == e1)
+    e3:set_prev(e2)
+    box.error(e3)
+end;
+test_run:cmd("setopt delimiter ''");
+
+box.schema.user.grant('guest', 'read,write,execute', 'universe')
+c = net.connect(box.cfg.listen)
+f = function(...) return c:call(...) end
+r, e3 = pcall(f, 'stack_err')
+assert(r == false)
+e3
+e2 = e3.prev
+assert(e2 ~= nil)
+e2
+e1 = e2.prev
+assert(e1 ~= nil)
+e1
+assert(e1.prev == nil)
+
+box.schema.user.revoke('guest', 'read,write,execute', 'universe')
+
 test_run:wait_log('default', 'Got a corrupted row.*', nil, 10)
 test_run:wait_log('default', '00000000:.*', nil, 10)
 test_run:wait_log('default', '00000010:.*', nil, 10)
diff --git a/test/unit/xrow.cc b/test/unit/xrow.cc
index 68a334239..dd1a85dc2 100644
--- a/test/unit/xrow.cc
+++ b/test/unit/xrow.cc
@@ -32,6 +32,7 @@ extern "C" {
 #include "unit.h"
 } /* extern "C" */
 #include "trivia/util.h"
+#include "box/error.h"
 #include "box/xrow.h"
 #include "box/iproto_constants.h"
 #include "uuid/tt_uuid.h"
@@ -241,6 +242,186 @@ test_xrow_header_encode_decode()
 	check_plan();
 }
 
+static char *
+error_stack_entry_encode(char *pos, const char *err_str)
+{
+	pos = mp_encode_map(pos, 2);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+	pos = mp_encode_uint(pos, 159);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+	pos = mp_encode_str(pos, err_str, strlen(err_str));
+	return pos;
+}
+
+void
+test_xrow_error_stack_decode()
+{
+	plan(21);
+	char buffer[2048];
+	/*
+	 * To start with, let's test the simplest and obsolete
+	 * way of setting errors.
+	 */
+	char *pos = mp_encode_map(buffer, 1);
+	pos = mp_encode_uint(pos, IPROTO_ERROR);
+	pos = mp_encode_str(pos, "e1", strlen("e1"));
+
+	struct xrow_header header;
+	header.bodycnt = 666;
+	header.type = 159 | IPROTO_TYPE_ERROR;
+	header.body[0].iov_base = buffer;
+	header.body[0].iov_len = pos - buffer;
+
+	xrow_decode_error(&header);
+	struct error *last = diag_last_error(diag_get());
+	isnt(last, NULL, "xrow_decode succeed: diag has been set");
+	is(strcmp(last->errmsg, "e1"), 0,
+	   "xrow_decode succeed: error is parsed");
+
+	pos = mp_encode_map(buffer, 1);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+	pos = mp_encode_array(pos, 2);
+	pos = error_stack_entry_encode(pos, "e1");
+	pos = error_stack_entry_encode(pos, "e2");
+	header.body[0].iov_base = buffer;
+	header.body[0].iov_len = pos - buffer;
+
+	diag_clear(diag_get());
+	xrow_decode_error(&header);
+	last = diag_last_error(diag_get());
+	isnt(last, NULL, "xrow_decode succeed: diag has been set");
+	is(strcmp(last->errmsg, "e2"), 0, "xrow_decode error stack suceed: "
+	   "e2 at the top of stack");
+	last = last->cause;
+	isnt(last, NULL, "xrow_decode succeed: 'cause' is present in stack")
+	is(strcmp(last->errmsg, "e1"), 0, "xrow_decode succeed: "
+	   "stack has been parsed");
+	last = last->cause;
+	is(last, NULL, "xrow_decode succeed: only two errors in the stack")
+	/*
+	 * Let's try decode broken stack. Variations:
+	 * 1. Stack is not encoded as an array;
+	 * 2. Stack doesn't contain maps;
+	 * 3. Stack's map keys are not uints;
+	 * 4. Stack's map values have wrong types;
+	 * 5. Stack's map key is broken (doesn't fit into u8);
+	 * 6. Stack's map contains overflowed (> 2^32) error code;
+	 * In all these cases diag_last should contain empty err.
+	 */
+	/* Stack is encoded as map. */
+	pos = mp_encode_map(buffer, 1);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+	pos = mp_encode_map(pos, 1);
+	pos = error_stack_entry_encode(pos, "e1");
+	header.body[0].iov_base = buffer;
+	header.body[0].iov_len = pos - buffer;
+
+	diag_clear(diag_get());
+	xrow_decode_error(&header);
+	last = diag_last_error(diag_get());
+	isnt(last, NULL, "xrow_decode succeed: diag has been set");
+	is(strcmp(last->errmsg, ""), 0, "xrow_decode corrupted stack: "
+	   "stack contains map instead of array");
+
+	/* Stack doesn't containt map(s) - array instead. */
+	pos = mp_encode_map(buffer, 1);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+	pos = mp_encode_array(pos, 2);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+	header.body[0].iov_base = buffer;
+	header.body[0].iov_len = pos - buffer;
+
+	diag_clear(diag_get());
+	xrow_decode_error(&header);
+	last = diag_last_error(diag_get());
+	isnt(last, NULL, "xrow_decode succeed: diag has been set");
+	is(strcmp(last->errmsg, ""), 0, "xrow_decode corrupted stack: "
+	   "stack contains array values instead of maps");
+
+	/* Stack's map keys are not uints. */
+	pos = mp_encode_map(buffer, 1);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+	pos = mp_encode_array(pos, 1);
+	pos = mp_encode_map(pos, 2);
+	pos = mp_encode_str(pos, "string instead of uint",
+			    strlen("string instead of uint"));
+	pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+	pos = mp_encode_uint(pos, 159);
+	header.body[0].iov_base = buffer;
+	header.body[0].iov_len = pos - buffer;
+
+	diag_clear(diag_get());
+	xrow_decode_error(&header);
+	last = diag_last_error(diag_get());
+	isnt(last, NULL, "xrow_decode succeed: diag has been set");
+	is(strcmp(last->errmsg, ""), 0, "xrow_decode corrupted stack: "
+	   "stack's map keys are not uints");
+
+	/* Stack's map values have wrong types. */
+	pos = mp_encode_map(buffer, 1);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+	pos = mp_encode_array(pos, 1);
+	pos = mp_encode_map(pos, 2);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+	pos = mp_encode_uint(pos, 159);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+	pos = mp_encode_uint(pos, 666);
+	header.body[0].iov_base = buffer;
+	header.body[0].iov_len = pos - buffer;
+
+	diag_clear(diag_get());
+	xrow_decode_error(&header);
+	last = diag_last_error(diag_get());
+	isnt(last, NULL, "xrow_decode succeed: diag has been set");
+	is(strcmp(last->errmsg, ""), 0, "xrow_decode corrupted stack: "
+	   "stack's map wrong value type");
+
+	/* Bad key in the packet. */
+	pos = mp_encode_map(buffer, 1);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+	pos = mp_encode_array(pos, 1);
+	pos = mp_encode_map(pos, 2);
+	pos = mp_encode_uint(pos, 0xff00000000 | IPROTO_ERROR_CODE);
+	pos = mp_encode_uint(pos, 159);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+	pos = mp_encode_str(pos, "test msg", strlen("test msg"));
+	header.body[0].iov_base = buffer;
+	header.body[0].iov_len = pos - buffer;
+
+	diag_clear(diag_get());
+	xrow_decode_error(&header);
+	last = diag_last_error(diag_get());
+	isnt(last, NULL, "xrow_decode succeed: diag has been set");
+	is(box_error_code(last), 0, "xrow_decode last error code is default 0");
+	is(strcmp(last->errmsg, "test msg"), 0, "xrow_decode corrupted stack: "
+	   "stack's map wrong key");
+
+	/* Overflow error code. */
+	pos = mp_encode_map(buffer, 1);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_STACK);
+	pos = mp_encode_array(pos, 1);
+	pos = mp_encode_map(pos, 2);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_CODE);
+	pos = mp_encode_uint(pos, (uint64_t)1 << 40);
+	pos = mp_encode_uint(pos, IPROTO_ERROR_MESSAGE);
+	pos = mp_encode_str(pos, "test msg", strlen("test msg"));
+	header.body[0].iov_base = buffer;
+	header.body[0].iov_len = pos - buffer;
+
+	diag_clear(diag_get());
+	xrow_decode_error(&header);
+	last = diag_last_error(diag_get());
+	isnt(last, NULL, "xrow_decode succeed: diag has been set");
+	is(box_error_code(last), 159, "xrow_decode failed, took code from "
+	   "header");
+	is(strcmp(last->errmsg, ""), 0, "xrow_decode failed, message is not "
+	   "decoded");
+
+	check_plan();
+}
+
 void
 test_request_str()
 {
@@ -279,13 +460,14 @@ main(void)
 {
 	memory_init();
 	fiber_init(fiber_c_invoke);
-	plan(3);
+	plan(4);
 
 	random_init();
 
 	test_iproto_constants();
 	test_greeting();
 	test_xrow_header_encode_decode();
+	test_xrow_error_stack_decode();
 	test_request_str();
 
 	random_free();
diff --git a/test/unit/xrow.result b/test/unit/xrow.result
index 5ee92ad7b..c78109ae0 100644
--- a/test/unit/xrow.result
+++ b/test/unit/xrow.result
@@ -1,4 +1,4 @@
-1..3
+1..4
     1..40
     ok 1 - round trip
     ok 2 - roundtrip.version_id
@@ -53,6 +53,29 @@ ok 1 - subtests
     ok 9 - decoded sync
     ok 10 - decoded bodycnt
 ok 2 - subtests
+    1..21
+    ok 1 - xrow_decode succeed: diag has been set
+    ok 2 - xrow_decode succeed: error is parsed
+    ok 3 - xrow_decode succeed: diag has been set
+    ok 4 - xrow_decode error stack suceed: e2 at the top of stack
+    ok 5 - xrow_decode succeed: 'cause' is present in stack
+    ok 6 - xrow_decode succeed: stack has been parsed
+    ok 7 - xrow_decode succeed: only two errors in the stack
+    ok 8 - xrow_decode succeed: diag has been set
+    ok 9 - xrow_decode corrupted stack: stack contains map instead of array
+    ok 10 - xrow_decode succeed: diag has been set
+    ok 11 - xrow_decode corrupted stack: stack contains array values instead of maps
+    ok 12 - xrow_decode succeed: diag has been set
+    ok 13 - xrow_decode corrupted stack: stack's map keys are not uints
+    ok 14 - xrow_decode succeed: diag has been set
+    ok 15 - xrow_decode corrupted stack: stack's map wrong value type
+    ok 16 - xrow_decode succeed: diag has been set
+    ok 17 - xrow_decode last error code is default 0
+    ok 18 - xrow_decode corrupted stack: stack's map wrong key
+    ok 19 - xrow_decode succeed: diag has been set
+    ok 20 - xrow_decode failed, took code from header
+    ok 21 - xrow_decode failed, message is not decoded
+ok 3 - subtests
     1..1
     ok 1 - request_str
-ok 3 - subtests
+ok 4 - subtests
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics
  2020-04-06 14:17 [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Nikita Pettik
                   ` (5 preceding siblings ...)
  2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 6/6] iproto: support error stacked diagnostic area Nikita Pettik
@ 2020-04-07 11:13 ` Kirill Yukhin
  6 siblings, 0 replies; 8+ messages in thread
From: Kirill Yukhin @ 2020-04-07 11:13 UTC (permalink / raw)
  To: Nikita Pettik; +Cc: tarantool-patches, v.shpilevoy

Hello,

On 06 апр 17:17, Nikita Pettik wrote:
> Branch: https://github.com/tarantool/tarantool/commits/np/gh-1148-stacked-diag
> Issue:
> https://github.com/tarantool/tarantool/issues/1148
> https://github.com/tarantool/tarantool/issues/4829
> 
> Implementation of stacked diagnostics in Tarantool according to RFC:
> https://github.com/tarantool/tarantool/commit/1acd32d98f628431429b427df19caa9d269bb9c8).
> Error structure is extended with double linked list; Lua and C interfaces
> are provided to interact with it; stacked diagnostics is supported in
> IProto protocol and net.box modules.
> 
> Note that patch-set also contains fix of #4829 since some tests rely
> on the fact that box.error(err_obj) promotes error to diagnotic area.

I've checked your patchset into master.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-04-07 11:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-06 14:17 [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Nikita Pettik
2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 1/6] box: introduce stacked diagnostic area Nikita Pettik
2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 2/6] box: use stacked diagnostic area for functional indexes Nikita Pettik
2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 3/6] box/error: clarify purpose of reference counting in struct error Nikita Pettik
2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 4/6] iproto: refactor error encoding with mpstream Nikita Pettik
2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 5/6] box: always promote error created via box.error() to diag Nikita Pettik
2020-04-06 14:17 ` [Tarantool-patches] [PATCH v3 6/6] iproto: support error stacked diagnostic area Nikita Pettik
2020-04-07 11:13 ` [Tarantool-patches] [PATCH v3 0/6] Stacked diagnostics Kirill Yukhin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox