[Tarantool-patches] [PATCH V6 08/10] error: make iproto errors reuse mp_error module

Leonid Vasiliev lvasiliev at tarantool.org
Mon Apr 20 01:25:10 MSK 2020


From: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>

After error objects marshaling was implemented in #4398, there
were essentially 2 versions of the marshaling - when an error is
sent inside response body, and when it is thrown and is encoded
in iproto fields IPROTO_ERROR and IPROTO_ERROR_STACK. That is not
really useful to have 2 implementation of the same feature. This
commit drops the old iproto error encoding (its IPROTO_ERROR_STACK
part), and makes it reuse the common error encoder.

Note, the encoder skips MP_EXT header. This is because

* The header is not needed - error is encoded as a value of
  IPROTO_ERROR_STACK key, so it is known this is an error. MP_EXT
  is needed only when type is unknown on decoding side in advance;

* Old clients may not expect MP_EXT in iproto fields. That is the
  case of netbox connector, at least.

Follow up #4398

@TarantoolBot document
Title: Stacked diagnostics binary protocol
Stacked diagnostics is described in details in
https://github.com/tarantool/doc/issues/1224. This commit
changes nothing except binary protocol. The old protocol should
not be documented anywhere.

`IPROTO_ERROR_STACK` is still 0x52, but format of its value is
different now. It looks exactly like `MP_ERROR` object, without
`MP_EXT` header.

```
IPROTO_ERROR_STACK: <MP_MAP> {
    MP_ERROR_STACK: <MP_ARRAY> [
        <MP_MAP> {
            ... <all the other fields of MP_ERROR> ...
        },
        ...
    ]
}
```

It is easy to see, that key `IPROTO_ERROR_STACK` is called
'stack', and `MP_ERROR_STACK` is also 'stack'. So it may be good
to rename the former key in the documentation. For example, the
old `IPROTO_ERROR` can be renamed to `IPROTO_ERROR_24` and
`IPROTO_ERROR_STACK` can be renamed to just `IPROTO_ERROR`.
---
 extra/exports                        |   1 +
 src/box/iproto_constants.h           |   5 -
 src/box/lua/net_box.lua              |  86 +++++++++++-----
 src/box/mp_error.cc                  |  31 +++++-
 src/box/mp_error.h                   |  18 +++-
 src/box/xrow.c                       |  63 +-----------
 test/box-tap/extended_error.test.lua |  48 ++++++---
 test/box/iproto.result               |  37 -------
 test/box/iproto.test.lua             |  11 ---
 test/unit/xrow.cc                    | 183 +----------------------------------
 test/unit/xrow.result                |  27 +-----
 11 files changed, 151 insertions(+), 359 deletions(-)

diff --git a/extra/exports b/extra/exports
index 3317ddb..d6b02e7 100644
--- a/extra/exports
+++ b/extra/exports
@@ -242,6 +242,7 @@ box_error_last
 box_error_clear
 box_error_set
 error_set_prev
+error_unpack_unsafe
 error_unref
 box_latch_new
 box_latch_delete
diff --git a/src/box/iproto_constants.h b/src/box/iproto_constants.h
index 7ed8296..b57d565 100644
--- a/src/box/iproto_constants.h
+++ b/src/box/iproto_constants.h
@@ -151,11 +151,6 @@ 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 07fa54c..1e55eaa 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -16,6 +16,7 @@ local fiber_clock       = fiber.clock
 local fiber_self        = fiber.self
 local decode            = msgpack.decode_unchecked
 local decode_map_header = msgpack.decode_map_header
+local buffer_reg        = buffer.reg1
 
 local table_new           = require('table.new')
 local check_iterator_type = box.internal.check_iterator_type
@@ -45,8 +46,6 @@ 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
@@ -57,6 +56,14 @@ local E_NO_CONNECTION        = box.error.NO_CONNECTION
 local E_TIMEOUT              = box.error.TIMEOUT
 local E_PROC_LUA             = box.error.PROC_LUA
 
+ffi.cdef[[
+struct error *
+error_unpack_unsafe(const char **data);
+
+void
+error_unref(struct error *e);
+]]
+
 -- utility tables
 local is_final_state         = {closed = 1, error = 1}
 
@@ -143,6 +150,30 @@ local method_decoder = {
     push    = decode_push,
 }
 
+local function decode_error_stack(raw_data)
+    local ptr = buffer_reg.acucp
+    ptr[0] = raw_data
+    local err = ffi.C.error_unpack_unsafe(ptr)
+    if err ~= nil then
+        err._refs = err._refs + 1
+        err = ffi.gc(err, ffi.C.error_unref)
+        -- From FFI it is returned as 'struct error *', which is
+        -- not considered equal to 'const struct error &', and is
+        -- is not accepted by functions like box.error(). Need to
+        -- cast explicitly.
+        err = ffi.cast('const struct error &', err)
+    else
+        -- Error unpacker installs fail reason into diag.
+        box.error()
+    end
+    return err, ptr[0]
+end
+
+local response_decoder = {
+    [IPROTO_ERROR_KEY] = decode,
+    [IPROTO_ERROR_STACK] = decode_error_stack,
+}
+
 local function next_id(id) return band(id + 1, 0x7FFFFFFF) end
 
 --
@@ -280,22 +311,14 @@ local function create_transport(host, port, user, password, callback,
     --
     function request_index:result()
         if self.errno then
-            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})
+            if type(self.response) ~= 'cdata' then
+                -- Error could be set by the connection state
+                -- machine, and it is usually a string explaining
+                -- a reason.
+                self.response = box.error.new({code = self.errno,
+                                               reason = self.response})
             end
+            return nil, self.response
         elseif not self.id then
             return self.response
         elseif not worker_fiber then
@@ -567,22 +590,40 @@ local function create_transport(host, port, user, password, callback,
             return
         end
         local status = hdr[IPROTO_STATUS_KEY]
-        local body, body_end_check
+        local body
+        local body_len = body_end - body_rpos
 
         if status > IPROTO_CHUNK_KEY then
             -- Handle errors
             requests[id] = nil
             request.id = nil
-            body, body_end_check = decode(body_rpos)
-            assert(body_end == body_end_check, "invalid xrow length")
+            local map_len, key, skip
+            map_len, body_rpos = decode_map_header(body_rpos, body_len)
+            -- Reserve for 2 keys and 2 array indexes. Because no
+            -- any guarantees how Lua will decide to save the
+            -- sparse table.
+            body = table_new(2, 2)
+            for _ = 1, map_len do
+                key, body_rpos = decode(body_rpos)
+                local rdec = response_decoder[key]
+                if rdec then
+                    body[key], body_rpos = rdec(body_rpos)
+                else
+                    skip, body_rpos = decode(body_rpos)
+                end
+            end
+            assert(body_end == body_rpos, "invalid xrow length")
             request.errno = band(status, IPROTO_ERRNO_MASK)
             -- 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')
+                assert(type(request.response) == 'cdata')
             else
-                request.response = body[IPROTO_ERROR_KEY]
+                request.response = box.error.new({
+                    code = request.errno,
+                    reason = body[IPROTO_ERROR_KEY]
+                })
             end
             request.cond:broadcast()
             return
@@ -591,7 +632,6 @@ local function create_transport(host, port, user, password, callback,
         local buffer = request.buffer
         if buffer ~= nil then
             -- Copy xrow.body to user-provided buffer
-            local body_len = body_end - body_rpos
             if request.skip_header then
                 -- Skip {[IPROTO_DATA_KEY] = ...} wrapper.
                 local map_len, key
diff --git a/src/box/mp_error.cc b/src/box/mp_error.cc
index e7d2808..0491a7a 100644
--- a/src/box/mp_error.cc
+++ b/src/box/mp_error.cc
@@ -453,6 +453,31 @@ error:
 }
 
 void
+error_to_mpstream_noext(const struct error *error, struct mpstream *stream)
+{
+	uint32_t err_cnt = 0;
+	uint32_t data_size = mp_sizeof_map(1);
+	data_size += mp_sizeof_uint(MP_ERROR_STACK);
+	for (const struct error *it = error; it != NULL; it = it->cause) {
+		err_cnt++;
+		data_size += mp_sizeof_error(it);
+	}
+
+	data_size += mp_sizeof_array(err_cnt);
+	char *ptr = mpstream_reserve(stream, data_size);
+	char *data = ptr;
+	data = mp_encode_map(data, 1);
+	data = mp_encode_uint(data, MP_ERROR_STACK);
+	data = mp_encode_array(data, err_cnt);
+	for (const struct error *it = error; it != NULL; it = it->cause) {
+		mp_encode_error_one(it, &data);
+	}
+
+	assert(data == ptr + data_size);
+	mpstream_advance(stream, data_size);
+}
+
+void
 error_to_mpstream(const struct error *error, struct mpstream *stream)
 {
 	uint32_t err_cnt = 0;
@@ -480,9 +505,8 @@ error_to_mpstream(const struct error *error, struct mpstream *stream)
 }
 
 struct error *
-error_unpack(const char **data, uint32_t len)
+error_unpack_unsafe(const char **data)
 {
-	const char *end = *data + len;
 	struct error *err = NULL;
 
 	if (mp_typeof(**data) != MP_MAP) {
@@ -526,8 +550,5 @@ error_unpack(const char **data, uint32_t len)
 			mp_next(data);
 		}
 	}
-
-	(void)end;
-	assert(*data == end);
 	return err;
 }
diff --git a/src/box/mp_error.h b/src/box/mp_error.h
index 5c746d9..f040e0f 100644
--- a/src/box/mp_error.h
+++ b/src/box/mp_error.h
@@ -35,6 +35,7 @@ extern "C" {
 #endif /* defined(__cplusplus) */
 
 #include <stdint.h>
+#include <assert.h>
 
 struct mpstream;
 
@@ -46,14 +47,27 @@ struct mpstream;
 void
 error_to_mpstream(const struct error *error, struct mpstream *stream);
 
+void
+error_to_mpstream_noext(const struct error *error, struct mpstream *stream);
+
+struct error *
+error_unpack_unsafe(const char **data);
+
 /**
  * @brief Unpack MP_ERROR to error.
  * @param data pointer to MP_ERROR.
  * @param len data size.
  * @return struct error * or NULL if failed.
  */
-struct error *
-error_unpack(const char **data, uint32_t len);
+static inline struct error *
+error_unpack(const char **data, uint32_t len)
+{
+	const char *end = *data + len;
+	struct error *e = error_unpack_unsafe(data);
+	(void)end;
+	assert(*data == end);
+	return e;
+}
 
 #if defined(__cplusplus)
 } /* extern "C" */
diff --git a/src/box/xrow.c b/src/box/xrow.c
index 97a604a..06c6afb 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -39,6 +39,7 @@
 #include "version.h"
 #include "tt_static.h"
 #include "error.h"
+#include "mp_error.h"
 #include "vclock.h"
 #include "scramble.h"
 #include "iproto_constants.h"
@@ -497,19 +498,8 @@ 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);
-	}
+	error_to_mpstream_noext(error, stream);
 }
 
 int
@@ -1093,51 +1083,6 @@ 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, NULL, reason);
-	}
-	return 0;
-}
-
 void
 xrow_decode_error(struct xrow_header *row)
 {
@@ -1175,8 +1120,10 @@ xrow_decode_error(struct xrow_header *row)
 			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)
+			struct error *e = error_unpack_unsafe(&pos);
+			if (e == NULL)
 				goto error;
+			diag_set_error(diag_get(), e);
 		} else {
 			mp_next(&pos);
 			continue;
diff --git a/test/box-tap/extended_error.test.lua b/test/box-tap/extended_error.test.lua
index 1681419..884387b 100755
--- a/test/box-tap/extended_error.test.lua
+++ b/test/box-tap/extended_error.test.lua
@@ -30,12 +30,16 @@ box.schema.func.drop('forbidden_function')
 box.session.su(user)
 
 local test = tap.test('Error marshaling')
-test:plan(6)
+test:plan(12)
 
 function error_new(...)
     return box.error.new(...)
 end
 
+function error_throw(...)
+    box.error(error_new(...))
+end
+
 function error_new_stacked(args1, args2)
     local e1 = box.error.new(args1)
     local e2 = box.error.new(args2)
@@ -43,10 +47,18 @@ function error_new_stacked(args1, args2)
     return e1
 end
 
+function error_throw_stacked(...)
+    box.error(error_new_stacked(...))
+end
+
 function error_access_denied()
     return access_denied_error
 end
 
+function error_throw_access_denied()
+    box.error(access_denied_error)
+end
+
 local function check_error(err, check_list)
     assert(type(check_list) == 'table')
     if type(err.trace) ~= 'table' or err.trace[1] == nil or
@@ -64,7 +76,8 @@ end
 box.schema.user.grant('guest', 'super')
 local c = netbox.connect(box.cfg.listen)
 c:eval('box.session.settings.error_marshaling_enabled = true')
-local err = c:call('error_new', {{code = 1000, reason = 'Reason'}})
+local args = {{code = 1000, reason = 'Reason'}}
+local err = c:call('error_new', args)
 local checks = {
     code = 1000,
     message = 'Reason',
@@ -72,10 +85,11 @@ local checks = {
     type = 'ClientError',
 }
 test:ok(check_error(err, checks), "ClientError marshaling")
+tmp, err = pcall(c.call, c, 'error_throw', args)
+test:ok(check_error(err, checks), "ClientError marshaling in iproto fields")
 
-err = c:call('error_new', {{
-    code = 1001, reason = 'Reason2', type = 'MyError'
-}})
+args = {{code = 1001, reason = 'Reason2', type = 'MyError'}}
+err = c:call('error_new', args)
 checks = {
     code = 1001,
     message = 'Reason2',
@@ -83,6 +97,8 @@ checks = {
     type = 'MyError',
 }
 test:ok(check_error(err, checks), "CustomError marshaling")
+tmp, err = pcall(c.call, c, 'error_throw', args)
+test:ok(check_error(err, checks), "CustomError marshaling in iproto fields")
 
 err = c:call('error_access_denied')
 checks = {
@@ -95,28 +111,38 @@ checks = {
     access_type = 'Execute',
 }
 test:ok(check_error(err, checks), "AccessDeniedError marshaling")
+tmp, err = pcall(c.call, c, 'error_throw_access_denied')
+test:ok(check_error(err, checks), "AccessDeniedError marshaling in iproto fields")
 
-err = c:call('error_new_stacked', {
+args = {
     {code = 1003, reason = 'Reason3', type = 'MyError2'},
     {code = 1004, reason = 'Reason4'}
-})
+}
+err = c:call('error_new_stacked', args)
 local err1 = err
 local err2 = err.prev
 test:isnt(err2, nil, 'Stack is received')
-checks = {
+local checks1 = {
     code = 1003,
     message = 'Reason3',
     base_type = 'CustomError',
     type = 'MyError2'
 }
-test:ok(check_error(err1, checks), "First error in the stack")
-checks = {
+test:ok(check_error(err1, checks1), "First error in the stack")
+local checks2 = {
     code = 1004,
     message = 'Reason4',
     base_type = 'ClientError',
     type = 'ClientError'
 }
-test:ok(check_error(err2, checks), "Second error in the stack")
+test:ok(check_error(err2, checks2), "Second error in the stack")
+
+tmp, err = pcall(c.call, c, 'error_throw_stacked', args)
+err1 = err
+err2 = err.prev
+test:isnt(err2, nil, 'Stack is received via iproto fields')
+test:ok(check_error(err1, checks1), "First error in the stack in iproto fields")
+test:ok(check_error(err2, checks2), "Second error in the stack in iproto fields")
 
 c:close()
 box.schema.user.revoke('guest', 'super')
diff --git a/test/box/iproto.result b/test/box/iproto.result
index b6dc7ed..e4fe684 100644
--- a/test/box/iproto.result
+++ b/test/box/iproto.result
@@ -100,43 +100,6 @@ 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
index 6402a22..ec715e9 100644
--- a/test/box/iproto.test.lua
+++ b/test/box/iproto.test.lua
@@ -52,15 +52,4 @@ sock:close()
 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/unit/xrow.cc b/test/unit/xrow.cc
index dd1a85d..9fd1547 100644
--- a/test/unit/xrow.cc
+++ b/test/unit/xrow.cc
@@ -242,186 +242,6 @@ 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()
 {
@@ -460,14 +280,13 @@ main(void)
 {
 	memory_init();
 	fiber_init(fiber_c_invoke);
-	plan(4);
+	plan(3);
 
 	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 c78109a..5ee92ad 100644
--- a/test/unit/xrow.result
+++ b/test/unit/xrow.result
@@ -1,4 +1,4 @@
-1..4
+1..3
     1..40
     ok 1 - round trip
     ok 2 - roundtrip.version_id
@@ -53,29 +53,6 @@ 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 4 - subtests
+ok 3 - subtests
-- 
2.7.4



More information about the Tarantool-patches mailing list