From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id CD77E4696C5 for ; Wed, 19 Feb 2020 17:17:04 +0300 (MSK) From: Nikita Pettik Date: Wed, 19 Feb 2020 17:16:56 +0300 Message-Id: <98b175140b1bf2e2f9d3285ca281008b9d7b6c11.1582119629.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: Subject: [Tarantool-patches] [PATCH 7/7] iproto: support error stacked diagnostic area List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org This patch introduces support of stacked errors in IProto protocol and in net.box module. @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 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 0x51, 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. Closes #1148 --- src/box/error.cc | 17 +++++ src/box/error.h | 16 +++++ src/box/iproto_constants.h | 6 ++ src/box/lua/net_box.lua | 32 ++++++++- src/box/xrow.c | 78 +++++++++++++++++++-- test/box-py/iproto.result | 6 +- test/box-py/iproto.test.py | 6 +- test/box/iproto.result | 166 +++++++++++++++++++++++++++++++++++++++++++++ test/box/iproto.test.lua | 73 ++++++++++++++++++++ test/box/net.box.result | 65 ++++++++++++++++++ test/box/net.box.test.lua | 25 +++++++ 11 files changed, 475 insertions(+), 15 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 824a4617c..e3197c8e6 100644 --- a/src/box/error.cc +++ b/src/box/error.cc @@ -102,6 +102,23 @@ box_error_construct(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); + } + diag_add_error(&fiber()->diag, e); + return -1; +} + /* }}} */ struct rmean *rmean_error = NULL; diff --git a/src/box/error.h b/src/box/error.h index 42043ef80..626701f27 100644 --- a/src/box/error.h +++ b/src/box/error.h @@ -137,6 +137,22 @@ struct error * box_error_construct(const char *file, unsigned line, uint32_t code, const char *fmt, ...); +/** + * 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, ...); + /** * A backward-compatible API define. */ diff --git a/src/box/iproto_constants.h b/src/box/iproto_constants.h index b66c05c06..a77660018 100644 --- a/src/box/iproto_constants.h +++ b/src/box/iproto_constants.h @@ -125,6 +125,7 @@ enum iproto_key { IPROTO_STMT_ID = 0x43, /* Leave a gap between SQL keys and additional request keys */ IPROTO_REPLICA_ANON = 0x50, + IPROTO_ERROR_STACK = 0x51, IPROTO_KEY_MAX }; @@ -149,6 +150,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<errmsg); + + uint32_t err_cnt = 0; + for (const struct error *it = error; it != NULL; it = it->next) + 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->next) { + 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 @@ -1072,6 +1085,48 @@ 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) +{ + char *reason = tt_static_buf(); + 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(); + uint32_t stack_sz = mp_decode_array(pos); + for (uint32_t i = 0; i < stack_sz; i++) { + uint32_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; + uint8_t key = mp_decode_uint(pos); + if (key == IPROTO_ERROR_CODE) { + if (mp_typeof(**pos) != MP_UINT) + return -1; + code = mp_decode_uint(pos); + } 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); + snprintf(reason, TT_STATIC_BUF_LEN, "%.*s", + len, str); + } else { + mp_next(pos); + continue; + } + box_error_add(__FILE__, __LINE__, code, reason); + } + } + return 0; +} + void xrow_decode_error(struct xrow_header *row) { @@ -1098,15 +1153,26 @@ 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 && + mp_typeof(*pos) == MP_ARRAY) { + if (iproto_decode_error_stack(&pos) != 0) + continue; + } 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..28b8bf140 --- /dev/null +++ b/test/box/iproto.result @@ -0,0 +1,166 @@ +test_run = require('test_run').new() +--- +... +net_box = require('net.box') +--- +... +urilib = require('uri') +--- +... +msgpack = require('msgpack') +--- +... +IPROTO_REQUEST_TYPE = 0x00 +--- +... +IPROTO_INSERT = 0x02 +--- +... +IPROTO_SYNC = 0x01 +--- +... +IPROTO_SPACE_ID = 0x10 +--- +... +IPROTO_TUPLE = 0x21 +--- +... +IPROTO_ERROR = 0x31 +--- +... +IPROTO_ERROR_STACK = 0x51 +--- +... +IPROTO_ERROR_CODE = 0x01 +--- +... +IPROTO_ERROR_MESSAGE = 0x02 +--- +... +IPROTO_OK = 0x00 +--- +... +IPROTO_SCHEMA_VERSION = 0x05 +--- +... +IPROTO_STATUS_KEY = 0x00 +--- +... +-- gh-1148: test capabilities of stacked diagnostics bypassing net.box. +-- +test_run:cmd("setopt delimiter ';'") +--- +- true +... +lua_func = [[function(tuple) local json = require('json') return json.encode(tuple) end]] +test_run:cmd("setopt delimiter ''"); +--- +... +box.schema.func.create('f1', {body = lua_func, is_deterministic = true, is_sandboxed = true}) +--- +... +s = box.schema.space.create('s') +--- +... +pk = s:create_index('pk') +--- +... +idx = s:create_index('idx', {func = box.func.f1.id, parts = {{1, 'string'}}}) +--- +... +box.schema.user.grant('guest', 'read, write, execute', 'universe') +--- +... +next_request_id = 16 +--- +... +header = msgpack.encode({ \ + [IPROTO_REQUEST_TYPE] = IPROTO_INSERT, \ + [IPROTO_SYNC] = next_request_id, \ +}) +--- +... +body = msgpack.encode({ \ + [IPROTO_SPACE_ID] = s.id, \ + [IPROTO_TUPLE] = box.tuple.new({1}) \ +}) +--- +... +uri = urilib.parse(box.cfg.listen) +--- +... +sock = net_box.establish_connection(uri.host, uri.service) +--- +... +-- Send request. +-- +size = msgpack.encode(header:len() + body:len()) +--- +... +sock:write(size .. header .. body) +--- +- 14 +... +-- Read responce. +-- +size = msgpack.decode(sock:read(5)) +--- +... +header_body = sock:read(size) +--- +... +header, header_len = msgpack.decode(header_body) +--- +... +body = msgpack.decode(header_body:sub(header_len)) +--- +... +sock:close() +--- +- true +... +-- Both keys (obsolete and stack ones) are present in response. +-- +assert(body[IPROTO_ERROR_STACK] ~= nil) +--- +- true +... +assert(body[IPROTO_ERROR] ~= nil) +--- +- true +... +err = body[IPROTO_ERROR_STACK][1] +--- +... +assert(err[IPROTO_ERROR_MESSAGE] == body[IPROTO_ERROR]) +--- +- true +... +err = body[IPROTO_ERROR_STACK][2] +--- +... +assert(err[IPROTO_ERROR_CODE] ~= nil) +--- +- true +... +assert(type(err[IPROTO_ERROR_CODE]) == 'number') +--- +- true +... +assert(err[IPROTO_ERROR_MESSAGE] ~= nil) +--- +- true +... +assert(type(err[IPROTO_ERROR_MESSAGE]) == 'string') +--- +- true +... +box.schema.user.revoke('guest', 'read,write,execute', 'universe') +--- +... +s:drop() +--- +... +box.func.f1:drop() +--- +... diff --git a/test/box/iproto.test.lua b/test/box/iproto.test.lua new file mode 100644 index 000000000..0425d0a21 --- /dev/null +++ b/test/box/iproto.test.lua @@ -0,0 +1,73 @@ +test_run = require('test_run').new() +net_box = require('net.box') +urilib = require('uri') +msgpack = require('msgpack') + +IPROTO_REQUEST_TYPE = 0x00 +IPROTO_INSERT = 0x02 +IPROTO_SYNC = 0x01 +IPROTO_SPACE_ID = 0x10 +IPROTO_TUPLE = 0x21 +IPROTO_ERROR = 0x31 +IPROTO_ERROR_STACK = 0x51 +IPROTO_ERROR_CODE = 0x01 +IPROTO_ERROR_MESSAGE = 0x02 +IPROTO_OK = 0x00 +IPROTO_SCHEMA_VERSION = 0x05 +IPROTO_STATUS_KEY = 0x00 + +-- gh-1148: test capabilities of stacked diagnostics bypassing net.box. +-- +test_run:cmd("setopt delimiter ';'") +lua_func = [[function(tuple) local json = require('json') return json.encode(tuple) end]] +test_run:cmd("setopt delimiter ''"); + +box.schema.func.create('f1', {body = lua_func, is_deterministic = true, is_sandboxed = true}) +s = box.schema.space.create('s') +pk = s:create_index('pk') +idx = s:create_index('idx', {func = box.func.f1.id, parts = {{1, 'string'}}}) + +box.schema.user.grant('guest', 'read, write, execute', 'universe') + +next_request_id = 16 +header = msgpack.encode({ \ + [IPROTO_REQUEST_TYPE] = IPROTO_INSERT, \ + [IPROTO_SYNC] = next_request_id, \ +}) + +body = msgpack.encode({ \ + [IPROTO_SPACE_ID] = s.id, \ + [IPROTO_TUPLE] = box.tuple.new({1}) \ +}) + +uri = urilib.parse(box.cfg.listen) +sock = net_box.establish_connection(uri.host, uri.service) + +-- Send request. +-- +size = msgpack.encode(header:len() + body:len()) +sock:write(size .. header .. body) +-- Read responce. +-- +size = msgpack.decode(sock:read(5)) +header_body = sock:read(size) +header, header_len = msgpack.decode(header_body) +body = msgpack.decode(header_body:sub(header_len)) +sock:close() + +-- Both keys (obsolete and stack ones) are present in response. +-- +assert(body[IPROTO_ERROR_STACK] ~= nil) +assert(body[IPROTO_ERROR] ~= nil) + +err = body[IPROTO_ERROR_STACK][1] +assert(err[IPROTO_ERROR_MESSAGE] == body[IPROTO_ERROR]) +err = body[IPROTO_ERROR_STACK][2] +assert(err[IPROTO_ERROR_CODE] ~= nil) +assert(type(err[IPROTO_ERROR_CODE]) == 'number') +assert(err[IPROTO_ERROR_MESSAGE] ~= nil) +assert(type(err[IPROTO_ERROR_MESSAGE]) == 'string') + +box.schema.user.revoke('guest', 'read,write,execute', 'universe') +s:drop() +box.func.f1:drop() diff --git a/test/box/net.box.result b/test/box/net.box.result index e3dabf7d9..c5d5d3743 100644 --- a/test/box/net.box.result +++ b/test/box/net.box.result @@ -3902,6 +3902,71 @@ sock:close() --- - true ... +-- gh-1148: test stacked diagnostics. +-- +test_run:cmd("push filter \"file: .*\" to \"file: \"") +--- +- true +... +test_run:cmd("setopt delimiter ';'") +--- +- true +... +lua_code = [[function(tuple) local json = require('json') return json.encode(tuple) end]] +test_run:cmd("setopt delimiter ''"); +--- +... +box.schema.func.create('f1', {body = lua_code, is_deterministic = true, is_sandboxed = true}) +--- +... +s = box.schema.space.create('s') +--- +... +pk = s:create_index('pk') +--- +... +idx = s:create_index('idx', {func = box.func.f1.id, parts = {{1, 'string'}}}) +--- +... +box.schema.user.grant('guest', 'read,write,execute', 'universe') +--- +... +c = net.connect(box.cfg.listen) +--- +... +f = function(...) return c.space.s:insert(...) end +--- +... +_, e = pcall(f, {1}) +--- +... +assert(e ~= nil) +--- +- true +... +e:unpack().message +--- +- 'Failed to build a key for functional index ''idx'' of space ''s'': can''t evaluate + function' +... +e.prev:unpack().message +--- +- '[string "return function(tuple) local json = require(''..."]:1: attempt to call + global ''require'' (a nil value)' +... +box.schema.user.revoke('guest', 'read,write,execute', 'universe') +--- +... +s:drop() +--- +... +box.func.f1:drop() +--- +... +test_run:cmd("clear filter") +--- +- true +... 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..d459293df 100644 --- a/test/box/net.box.test.lua +++ b/test/box/net.box.test.lua @@ -1574,6 +1574,31 @@ 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("push filter \"file: .*\" to \"file: \"") +test_run:cmd("setopt delimiter ';'") +lua_code = [[function(tuple) local json = require('json') return json.encode(tuple) end]] +test_run:cmd("setopt delimiter ''"); +box.schema.func.create('f1', {body = lua_code, is_deterministic = true, is_sandboxed = true}) +s = box.schema.space.create('s') +pk = s:create_index('pk') +idx = s:create_index('idx', {func = box.func.f1.id, parts = {{1, 'string'}}}) + +box.schema.user.grant('guest', 'read,write,execute', 'universe') +c = net.connect(box.cfg.listen) +f = function(...) return c.space.s:insert(...) end +_, e = pcall(f, {1}) +assert(e ~= nil) + +e:unpack().message +e.prev:unpack().message + +box.schema.user.revoke('guest', 'read,write,execute', 'universe') +s:drop() +box.func.f1:drop() +test_run:cmd("clear filter") + 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) -- 2.15.1