* [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
---
| 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(-)
--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 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