* [Tarantool-patches] [PATCH V5 1/6] error: add custom error type
2020-04-18 15:29 [Tarantool-patches] [PATCH V5 0/6] Extending error functionality Leonid Vasiliev
@ 2020-04-18 15:29 ` Leonid Vasiliev
2020-04-18 18:52 ` Vladislav Shpilevoy
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 2/6] error: send custom type in IProto Leonid Vasiliev
` (5 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Leonid Vasiliev @ 2020-04-18 15:29 UTC (permalink / raw)
To: v.shpilevoy, alexander.turenko; +Cc: tarantool-patches
Co-authored-by: Vladislav Shpilevoy<v.shpilevoy@tarantool.org>
Part of #4398
@TarantoolBot document
Title: Custom error types for Lua errors
Errors can be created in 2 ways: `box.error.new()` and `box.error()`.
Both used to take either `code, reason, <reason string args>` or
`{code = code, reason = reason, ...}` arguments.
Now in the first option instead of code a user can specify a
string as its own error type. In the second option a user can
specify both code and type. For example:
```Lua
box.error('MyErrorType', 'Message')
box.error({type = 'MyErrorType', code = 1024, reason = 'Message'})
```
Or no-throw version:
```Lua
box.error.new('MyErrorType', 'Message')
box.error.new({type = 'MyErrorType', code = 1024, reason = 'Message'})
```
When a custom type is specified, it is shown in `err.type`
attribute. When it is not specified, `err.type` shows one of
built-in errors such as 'ClientError', 'OurOfMemory', etc.
Name length limit on the custom type is 63 bytes. All what is
longer is truncated.
Original error type can be checked using `err.base_type` member,
although normally it should not be used. For user-defined types
base type is 'CustomError'.
For example:
```
tarantool> e = box.error.new({type = 'MyErrorType', code = 1024, reason = 'Message'})
---
...
tarantool> e:unpack()
---
- code: 1024
trace:
- file: '[string "e = box.error.new({type = ''MyErrorType'', code..."]'
line: 1
type: MyErrorType
custom_type: MyErrorType
message: Message
base_type: CustomError
...
```
---
| 1 +
src/box/error.cc | 101 +++++++++++++++++++++++++++++++++---------
src/box/error.h | 39 ++++++++++++++--
src/box/lua/error.cc | 42 +++++++++++++-----
src/box/xrow.c | 2 +-
src/lua/error.lua | 14 +++++-
test/app/fiber.result | 5 ++-
test/box/error.result | 65 +++++++++++++++++++++++++--
test/box/error.test.lua | 15 +++++++
test/engine/func_index.result | 14 +++---
10 files changed, 249 insertions(+), 49 deletions(-)
--git a/extra/exports b/extra/exports
index a9add2c..9467398 100644
--- a/extra/exports
+++ b/extra/exports
@@ -235,6 +235,7 @@ box_index_min
box_index_max
box_index_count
box_error_type
+box_error_custom_type
box_error_code
box_error_message
box_error_last
diff --git a/src/box/error.cc b/src/box/error.cc
index 233b312..277727d 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -86,35 +86,52 @@ box_error_set(const char *file, unsigned line, uint32_t code,
return -1;
}
+static struct error *
+box_error_new_va(const char *file, unsigned line, uint32_t code,
+ const char *custom_type, const char *fmt, va_list ap)
+{
+ if (custom_type == NULL) {
+ struct error *e = BuildClientError(file, line, ER_UNKNOWN);
+ ClientError *client_error = type_cast(ClientError, e);
+ if (client_error != NULL) {
+ client_error->m_errcode = code;
+ error_vformat_msg(e, fmt, ap);
+ }
+ return e;
+ } else {
+ struct error *e = BuildCustomError(file, line, custom_type,
+ code);
+ CustomError *custom_error = type_cast(CustomError, e);
+ if (custom_error != NULL) {
+ error_vformat_msg(e, fmt, ap);
+ }
+
+ return e;
+ }
+}
+
struct error *
box_error_new(const char *file, unsigned line, uint32_t code,
- const char *fmt, ...)
+ const char *custom_type, const char *fmt, ...)
{
- struct error *e = BuildClientError(file, line, ER_UNKNOWN);
- ClientError *client_error = type_cast(ClientError, e);
- if (client_error != NULL) {
- client_error->m_errcode = code;
- va_list ap;
- va_start(ap, fmt);
- error_vformat_msg(e, fmt, ap);
- va_end(ap);
- }
+ va_list ap;
+ va_start(ap, fmt);
+ struct error *e = box_error_new_va(file, line, code, custom_type,
+ fmt, ap);
+ va_end(ap);
return e;
}
int
box_error_add(const char *file, unsigned line, uint32_t code,
- const char *fmt, ...)
+ const char *custom_type, 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);
- }
+ va_list ap;
+ va_start(ap, fmt);
+ struct error *e = box_error_new_va(file, line, code, custom_type,
+ fmt, ap);
+ va_end(ap);
+
struct diag *d = &fiber()->diag;
if (diag_is_empty(d))
diag_set_error(d, e);
@@ -125,6 +142,16 @@ box_error_add(const char *file, unsigned line, uint32_t code,
/* }}} */
+const char *
+box_error_custom_type(const struct error *e)
+{
+ CustomError *custom_error = type_cast(CustomError, e);
+ if (custom_error)
+ return custom_error->custom_type();
+
+ return NULL;
+}
+
struct rmean *rmean_error = NULL;
const char *rmean_error_strings[RMEAN_ERROR_LAST] = {
@@ -290,3 +317,37 @@ BuildAccessDeniedError(const char *file, unsigned int line,
return e;
}
}
+
+static struct method_info customerror_methods[] = {
+ make_method(&type_CustomError, "custom_type", &CustomError::custom_type),
+ METHODS_SENTINEL
+};
+
+const struct type_info type_CustomError =
+ make_type("CustomError", &type_ClientError, customerror_methods);
+
+CustomError::CustomError(const char *file, unsigned int line,
+ const char *custom_type, uint32_t errcode)
+ :ClientError(&type_CustomError, file, line, errcode)
+{
+ strncpy(m_custom_type, custom_type, sizeof(m_custom_type) - 1);
+ m_custom_type[sizeof(m_custom_type) - 1] = '\0';
+}
+
+void
+CustomError::log() const
+{
+ say_file_line(S_ERROR, file, line, errmsg, "%s",
+ "Custom type %s", m_custom_type);
+}
+
+struct error *
+BuildCustomError(const char *file, unsigned int line, const char *custom_type,
+ uint32_t errcode)
+{
+ try {
+ return new CustomError(file, line, custom_type, errcode);
+ } catch (OutOfMemory *e) {
+ return e;
+ }
+}
diff --git a/src/box/error.h b/src/box/error.h
index ca5d5b2..10f282f 100644
--- a/src/box/error.h
+++ b/src/box/error.h
@@ -53,6 +53,10 @@ struct error *
BuildXlogGapError(const char *file, unsigned line,
const struct vclock *from, const struct vclock *to);
+struct error *
+BuildCustomError(const char *file, unsigned int line, const char *custom_type,
+ uint32_t errcode);
+
/** \cond public */
struct error;
@@ -138,11 +142,21 @@ box_error_set(const char *file, unsigned line, uint32_t code,
/** \endcond public */
/**
+ * Return the error custom type,
+ * \param e Error object.
+ * \return Pointer to custom error type. On error return NULL.
+ */
+const char *
+box_error_custom_type(const struct error *e);
+
+/**
* 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 custom_type User-defined error type which will be
+ * displayed instead of ClientError.
* \param format (const char * ) - printf()-like format string
* \param ... - format arguments
* \returns -1 for convention use
@@ -151,19 +165,20 @@ box_error_set(const char *file, unsigned line, uint32_t code,
*/
int
box_error_add(const char *file, unsigned line, uint32_t code,
- const char *fmt, ...);
+ const char *custom_type, const char *fmt, ...);
/**
* Construct error object without setting it in the diagnostics
- * area. On the memory allocation fail returns NULL.
+ * area. On the memory allocation fail returns OutOfMemory error.
*/
struct error *
box_error_new(const char *file, unsigned line, uint32_t code,
- const char *fmt, ...);
+ const char *custom_type, const char *fmt, ...);
extern const struct type_info type_ClientError;
extern const struct type_info type_XlogError;
extern const struct type_info type_AccessDeniedError;
+extern const struct type_info type_CustomError;
#if defined(__cplusplus)
} /* extern "C" */
@@ -290,6 +305,24 @@ struct XlogGapError: public XlogError
virtual void raise() { throw this; }
};
+class CustomError: public ClientError
+{
+public:
+ CustomError(const char *file, unsigned int line,
+ const char *custom_type, uint32_t errcode);
+
+ virtual void log() const;
+
+ const char*
+ custom_type()
+ {
+ return m_custom_type;
+ }
+private:
+ /** Custom type name */
+ char m_custom_type[64];
+};
+
#endif /* defined(__cplusplus) */
#endif /* TARANTOOL_BOX_ERROR_H_INCLUDED */
diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
index b2625bf..e0a3e73 100644
--- a/src/box/lua/error.cc
+++ b/src/box/lua/error.cc
@@ -46,6 +46,13 @@ extern "C" {
* Parse Lua arguments (they can come as single table
* f({code : number, reason : string}) or as separate members
* f(code, reason)) and construct struct error with given values.
+ *
+ * Instead of 'code' it is possible to specify a string name of
+ * the error object's type:
+ *
+ * box.error(type, reason, ...)
+ * box.error({type = string, reason = string, ...})
+ *
* In case one of arguments is missing its corresponding field
* in struct error is filled with default value.
*/
@@ -53,14 +60,22 @@ static struct error *
luaT_error_create(lua_State *L, int top_base)
{
uint32_t code = 0;
+ const char *custom_type = NULL;
const char *reason = NULL;
const char *file = "";
unsigned line = 0;
lua_Debug info;
int top = lua_gettop(L);
- if (top >= top_base && lua_type(L, top_base) == LUA_TNUMBER) {
- code = lua_tonumber(L, top_base);
- reason = tnt_errcode_desc(code);
+ int top_type = lua_type(L, top_base);
+ if (top >= top_base && (top_type == LUA_TNUMBER ||
+ top_type == LUA_TSTRING)) {
+ if (top_type == LUA_TNUMBER) {
+ code = lua_tonumber(L, top_base);
+ reason = tnt_errcode_desc(code);
+ } else {
+ custom_type = lua_tostring(L, top_base);
+ reason = "%s";
+ }
if (top > top_base) {
/* Call string.format(reason, ...) to format message */
lua_getglobal(L, "string");
@@ -78,14 +93,17 @@ luaT_error_create(lua_State *L, int top_base)
/* Missing arguments to format string */
return NULL;
}
- } else if (top == top_base && lua_istable(L, top_base)) {
+ } else if (top == top_base && top_type == LUA_TTABLE) {
lua_getfield(L, top_base, "code");
- code = lua_tonumber(L, -1);
- lua_pop(L, 1);
+ if (!lua_isnil(L, -1))
+ code = lua_tonumber(L, -1);
lua_getfield(L, top_base, "reason");
reason = lua_tostring(L, -1);
if (reason == NULL)
reason = "";
+ lua_getfield(L, top_base, "type");
+ if (!lua_isnil(L, -1))
+ custom_type = lua_tostring(L, -1);
lua_pop(L, 1);
} else {
return NULL;
@@ -102,7 +120,7 @@ raise:
}
line = info.currentline;
}
- return box_error_new(file, line, code, "%s", reason);
+ return box_error_new(file, line, code, custom_type, "%s", reason);
}
static int
@@ -149,11 +167,11 @@ luaT_error_last(lua_State *L)
static int
luaT_error_new(lua_State *L)
{
- if (lua_gettop(L) == 0)
- return luaL_error(L, "Usage: box.error.new(code, args)");
- struct error *e = luaT_error_create(L, 1);
- if (e == NULL)
- return luaL_error(L, "Usage: box.error.new(code, args)");
+ struct error *e;
+ if (lua_gettop(L) == 0 || (e = luaT_error_create(L, 1)) == NULL) {
+ return luaL_error(L, "Usage: box.error.new(code, args) or "\
+ "box.error.new(type, args)");
+ }
lua_settop(L, 0);
luaT_pusherror(L, e);
return 1;
diff --git a/src/box/xrow.c b/src/box/xrow.c
index a494d1f..5494b41 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -1123,7 +1123,7 @@ iproto_decode_error_stack(const char **pos)
continue;
}
}
- box_error_add(__FILE__, __LINE__, code, reason);
+ box_error_add(__FILE__, __LINE__, code, NULL, reason);
}
return 0;
}
diff --git a/src/lua/error.lua b/src/lua/error.lua
index bdc9c71..a5f7435 100644
--- a/src/lua/error.lua
+++ b/src/lua/error.lua
@@ -35,6 +35,9 @@ exception_get_int(struct error *e, const struct method_info *method);
int
error_set_prev(struct error *e, struct error *prev);
+
+const char *
+box_error_custom_type(const struct error *e);
]]
local REFLECTION_CACHE = {}
@@ -75,10 +78,18 @@ local function reflection_get(err, method)
end
end
-local function error_type(err)
+local function error_base_type(err)
return ffi.string(err._type.name)
end
+local function error_type(err)
+ local res = ffi.C.box_error_custom_type(err)
+ if res ~= nil then
+ return ffi.string(res)
+ end
+ return error_base_type(err)
+end
+
local function error_message(err)
return ffi.string(err._errmsg)
end
@@ -131,6 +142,7 @@ local error_fields = {
["trace"] = error_trace;
["errno"] = error_errno;
["prev"] = error_prev;
+ ["base_type"] = error_base_type
}
local function error_unpack(err)
diff --git a/test/app/fiber.result b/test/app/fiber.result
index debfc67..9b82790 100644
--- a/test/app/fiber.result
+++ b/test/app/fiber.result
@@ -1038,8 +1038,9 @@ st;
...
e:unpack();
---
-- type: ClientError
- code: 1
+- code: 1
+ base_type: ClientError
+ type: ClientError
message: Illegal parameters, oh my
trace:
- file: '[string "function err() box.error(box.error.ILLEGAL_PA..."]'
diff --git a/test/box/error.result b/test/box/error.result
index df6d0eb..0192017 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -34,8 +34,9 @@ e
| ...
e:unpack()
| ---
- | - type: ClientError
- | code: 1
+ | - code: 1
+ | base_type: ClientError
+ | type: ClientError
| message: Illegal parameters, bla bla
| trace:
| - file: '[C]'
@@ -105,7 +106,7 @@ e
| ...
box.error.new()
| ---
- | - error: 'Usage: box.error.new(code, args)'
+ | - error: 'Usage: box.error.new(code, args) or box.error.new(type, args)'
| ...
--
@@ -489,7 +490,7 @@ assert(box.error.last() == nil)
--
box.error.new(err)
| ---
- | - error: 'Usage: box.error.new(code, args)'
+ | - error: 'Usage: box.error.new(code, args) or box.error.new(type, args)'
| ...
-- box.error() is supposed to re-throw last diagnostic error.
@@ -831,3 +832,59 @@ assert(box.error.last() == e1)
| ---
| - true
| ...
+--
+-- gh-4398: custom error type.
+--
+-- Try no code.
+e = box.error.new({type = 'TestType', reason = 'Test reason'})
+ | ---
+ | ...
+e:unpack()
+ | ---
+ | - code: 0
+ | base_type: CustomError
+ | type: TestType
+ | custom_type: TestType
+ | message: Test reason
+ | trace:
+ | - file: '[string "e = box.error.new({type = ''TestType'', reason ..."]'
+ | line: 1
+ | ...
+-- Try code not the same as used by default.
+e = box.error.new({type = 'TestType', reason = 'Test reason', code = 123})
+ | ---
+ | ...
+e:unpack()
+ | ---
+ | - code: 123
+ | base_type: CustomError
+ | type: TestType
+ | custom_type: TestType
+ | message: Test reason
+ | trace:
+ | - file: '[string "e = box.error.new({type = ''TestType'', reason ..."]'
+ | line: 1
+ | ...
+-- Try to omit message.
+e = box.error.new({type = 'TestType'})
+ | ---
+ | ...
+e:unpack()
+ | ---
+ | - code: 0
+ | base_type: CustomError
+ | type: TestType
+ | custom_type: TestType
+ | message:
+ | trace:
+ | - file: '[string "e = box.error.new({type = ''TestType''}) "]'
+ | line: 1
+ | ...
+-- Try too long type name.
+e = box.error.new({type = string.rep('a', 128)})
+ | ---
+ | ...
+#e.type
+ | ---
+ | - 63
+ | ...
diff --git a/test/box/error.test.lua b/test/box/error.test.lua
index 41baed5..38f7406 100644
--- a/test/box/error.test.lua
+++ b/test/box/error.test.lua
@@ -229,3 +229,18 @@ box.error({code = 111, reason = "err"})
box.error.last()
box.error(e1)
assert(box.error.last() == e1)
+--
+-- gh-4398: custom error type.
+--
+-- Try no code.
+e = box.error.new({type = 'TestType', reason = 'Test reason'})
+e:unpack()
+-- Try code not the same as used by default.
+e = box.error.new({type = 'TestType', reason = 'Test reason', code = 123})
+e:unpack()
+-- Try to omit message.
+e = box.error.new({type = 'TestType'})
+e:unpack()
+-- Try too long type name.
+e = box.error.new({type = string.rep('a', 128)})
+#e.type
diff --git a/test/engine/func_index.result b/test/engine/func_index.result
index a827c92..e70e591 100644
--- a/test/engine/func_index.result
+++ b/test/engine/func_index.result
@@ -277,21 +277,23 @@ e = box.error.last()
e:unpack()
| ---
| - code: 198
- | trace:
- | - file: <filename>
- | line: <line>
+ | base_type: ClientError
| 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)'
+ | message: 'Failed to build a key for functional index ''idx'' of space ''withdata'':
+ | can''t evaluate function'
+ | trace:
+ | - file: <filename>
+ | line: <line>
| ...
e = e.prev
| ---
| ...
e:unpack()
| ---
- | - type: LuajitError
+ | - base_type: LuajitError
+ | type: LuajitError
| message: '[string "return function(tuple) local ..."]:1: attempt
| to call global ''require'' (a nil value)'
| trace:
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [Tarantool-patches] [PATCH V5 2/6] error: send custom type in IProto
2020-04-18 15:29 [Tarantool-patches] [PATCH V5 0/6] Extending error functionality Leonid Vasiliev
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 1/6] error: add custom error type Leonid Vasiliev
@ 2020-04-18 15:29 ` Leonid Vasiliev
2020-04-18 20:39 ` Vladislav Shpilevoy
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 3/6] session: add offset to SQL session settings array Leonid Vasiliev
` (4 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Leonid Vasiliev @ 2020-04-18 15:29 UTC (permalink / raw)
To: v.shpilevoy, alexander.turenko; +Cc: tarantool-patches
Error custom type feature was added to the public Lua API in the
previous commit. This one makes the new attribute being sent in
IProto.
Co-authored-by: Vladislav Shpilevoy<v.shpilevoy@tarantool.org>
Needed for #4398
@TarantoolBot document
Title: New error object attribute in IProto
Error objects in IProto already have 2 fields:
`IPROTO_ERROR_CODE = 0x01` and `IPROTO_ERROR_MESSAGE = 0x02`.
Now added:
`IPROTO_ERROR_CUSTOM_TYPE = 0x03`.
It's optional, has MP_STR type, and speaks for itself.
Custom error type is error object attribute.
This is what a user specifies in
`box.error.new({type = <custom_type>})` or
`box.error.new(<custom_type>)`.
---
src/box/iproto_constants.h | 1 +
src/box/lua/net_box.lua | 9 ++++++---
src/box/xrow.c | 9 ++++++++-
test/box/error.result | 39 +++++++++++++++++++++++++++++++++++++++
test/box/error.test.lua | 16 ++++++++++++++++
5 files changed, 70 insertions(+), 4 deletions(-)
diff --git a/src/box/iproto_constants.h b/src/box/iproto_constants.h
index 7ed8296..1fca97b 100644
--- a/src/box/iproto_constants.h
+++ b/src/box/iproto_constants.h
@@ -154,6 +154,7 @@ enum iproto_ballot_key {
enum iproto_error_key {
IPROTO_ERROR_CODE = 0x01,
IPROTO_ERROR_MESSAGE = 0x02,
+ IPROTO_ERROR_CUSTOM_TYPE = 0x03,
};
#define bit(c) (1ULL<<IPROTO_##c)
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 07fa54c..13703e8 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -47,6 +47,7 @@ local IPROTO_ERROR_KEY = 0x31
local IPROTO_ERROR_STACK = 0x52
local IPROTO_ERROR_CODE = 0x01
local IPROTO_ERROR_MESSAGE = 0x02
+local IPROTO_ERROR_CUSTOM_TYPE = 0x03
local IPROTO_GREETING_SIZE = 128
local IPROTO_CHUNK_KEY = 128
local IPROTO_OK_KEY = 0
@@ -285,9 +286,11 @@ local function create_transport(host, port, user, password, callback,
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})
+ local new_err = box.error.new({
+ type = error[IPROTO_ERROR_CUSTOM_TYPE],
+ code = error[IPROTO_ERROR_CODE],
+ reason = error[IPROTO_ERROR_MESSAGE],
+ })
new_err:set_prev(prev)
prev = new_err
end
diff --git a/src/box/xrow.c b/src/box/xrow.c
index 5494b41..a717da6 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -494,11 +494,18 @@ mpstream_iproto_encode_error(struct mpstream *stream, const struct error *error)
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);
+ uint32_t map_size = 2;
+ const char *custom_type = box_error_custom_type(it);
+ map_size += (custom_type != NULL);
+ mpstream_encode_map(stream, map_size);
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);
+ if (custom_type != NULL) {
+ mpstream_encode_uint(stream, IPROTO_ERROR_CUSTOM_TYPE);
+ mpstream_encode_str(stream, custom_type);
+ }
}
}
diff --git a/test/box/error.result b/test/box/error.result
index 0192017..9f04c76 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -888,3 +888,42 @@ e = box.error.new({type = string.rep('a', 128)})
| ---
| - 63
| ...
+
+--
+-- Check how custom error type is passed through IProto.
+--
+netbox = require('net.box')
+ | ---
+ | ...
+box.schema.user.grant('guest', 'super')
+ | ---
+ | ...
+c = netbox.connect(box.cfg.listen)
+ | ---
+ | ...
+_, e = pcall(c.call, c, 'box.error', { \
+ {code = 123, type = 'TestType', reason = 'Test reason'} \
+})
+ | ---
+ | ...
+e = e:unpack()
+ | ---
+ | ...
+e.trace = nil
+ | ---
+ | ...
+e
+ | ---
+ | - code: 123
+ | base_type: CustomError
+ | type: TestType
+ | custom_type: TestType
+ | message: Test reason
+ | ...
+
+c:close()
+ | ---
+ | ...
+box.schema.user.revoke('guest', 'super')
+ | ---
+ | ...
diff --git a/test/box/error.test.lua b/test/box/error.test.lua
index 38f7406..716f2bd 100644
--- a/test/box/error.test.lua
+++ b/test/box/error.test.lua
@@ -244,3 +244,19 @@ e:unpack()
-- Try too long type name.
e = box.error.new({type = string.rep('a', 128)})
#e.type
+
+--
+-- Check how custom error type is passed through IProto.
+--
+netbox = require('net.box')
+box.schema.user.grant('guest', 'super')
+c = netbox.connect(box.cfg.listen)
+_, e = pcall(c.call, c, 'box.error', { \
+ {code = 123, type = 'TestType', reason = 'Test reason'} \
+})
+e = e:unpack()
+e.trace = nil
+e
+
+c:close()
+box.schema.user.revoke('guest', 'super')
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH V5 2/6] error: send custom type in IProto
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 2/6] error: send custom type in IProto Leonid Vasiliev
@ 2020-04-18 20:39 ` Vladislav Shpilevoy
0 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-18 20:39 UTC (permalink / raw)
To: Leonid Vasiliev, alexander.turenko; +Cc: tarantool-patches
On 18/04/2020 17:29, Leonid Vasiliev wrote:
> Error custom type feature was added to the public Lua API in the
> previous commit. This one makes the new attribute being sent in
> IProto.
>
> Co-authored-by: Vladislav Shpilevoy<v.shpilevoy@tarantool.org>
>
> Needed for #4398
>
> @TarantoolBot document
> Title: New error object attribute in IProto
>
> Error objects in IProto already have 2 fields:
> `IPROTO_ERROR_CODE = 0x01` and `IPROTO_ERROR_MESSAGE = 0x02`.
>
> Now added:
>
> `IPROTO_ERROR_CUSTOM_TYPE = 0x03`.
>
> It's optional, has MP_STR type, and speaks for itself.
> Custom error type is error object attribute.
> This is what a user specifies in
> `box.error.new({type = <custom_type>})` or
> `box.error.new(<custom_type>)`.
I added a note, that the field is optional:
The field is optional, and may be not present in response, if the
error does not have a custom type.
Talking of the patch - I still think IProto errors should reuse the
code used to encode/decode MP_EXT errors. If we go for that, this patch
is going to be 100% overrode by that refactoring, and does not make
sense as a separate commit.
There appeared a problem, that it is not that simple, since netbox
is not able to decode MP_EXT for now, as Leonid noticed. So there is
a workaround.
We discussed it verbally, below is the solution.
Now we have `IPROTO_ERROR_STACK` of a format:
```
IPROTO_ERROR_STACK <MP_ARRAY>: [
<MP_MAP> {
IPROTO_ERROR_CODE: <MP_UINT>,
IPROTO_ERROR_MESSAGE: <MP_STR>,
},
...
]
```
MP_ERROR format is going to be this:
```
<MP_EXT>:
MP_ERROR <MP_MAP>: {
MP_ERROR_STACK <MP_ARRAY>: [
<MP_MAP> {
MP_ERROR_TYPE: <MP_STR>,
MP_ERROR_FILE: <MP_STR>,
MP_ERROR_LINE: <MP_UINT>,
MP_ERROR_REASON: <MP_STR>,
MP_ERROR_ERRNO: <MP_UINT>,
MP_ERROR_FIELDS: <MP_MAP> {
<MP_STR>: ...,
<MP_STR>: ...,
...
},
},
...
}
}
```
I propose to make these formats the same, not counting `MP_EXT`.
For that we rename the old `IPROTO_ERROR` to `IPROTO_ERROR_24`
(similar to `IPROTO_CALL_16`), `IPROTO_ERROR_STACK` rename to
`IPROTO_ERROR`. Renames are ok, since the underlying numbers
are not going to change. So IProto error will look like this:
```
IPROTO_ERROR <MP_MAP>: {
IPROTO_ERROR_STACK <MP_ARRAY>: [
<MP_MAP> {
IPROTO_ERROR_TYPE: <MP_STR>,
IPROTO_ERROR_FILE: <MP_STR>,
IPROTO_ERROR_LINE: <MP_UINT>,
IPROTO_ERROR_REASON: <MP_STR>,
IPROTO_ERROR_ERRNO: <MP_UINT>,
IPROTO_ERROR_FIELDS: <MP_MAP> {
<MP_STR>: ...,
<MP_STR>: ...,
...
},
},
...
}
}
```
+ `IPROTO_ERROR_24`, like it is done in the stacked diag now.
Essentially, it looks exactly the same as `MP_ERROR` - `MP_EXT`
header.
Then we do that all keys `IPROTO_ERROR_*` and `MP_ERROR_*` have
the same values. So any `IPROTO_ERROR_<name>` == `MP_ERROR_<name>`.
For example, `MP_ERROR_STACK = 0x00`, and `IPROTO_ERROR_STACK = 0x00`.
Now all packing/unpacking can be done by the same functions for
IProto and for normal returns. IProto will encode it as `MP_MAP` in
its header, normal serialization will be via `MP_EXT`.
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH V5 3/6] session: add offset to SQL session settings array
2020-04-18 15:29 [Tarantool-patches] [PATCH V5 0/6] Extending error functionality Leonid Vasiliev
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 1/6] error: add custom error type Leonid Vasiliev
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 2/6] error: send custom type in IProto Leonid Vasiliev
@ 2020-04-18 15:29 ` Leonid Vasiliev
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 4/6] error: add session setting for error type marshaling Leonid Vasiliev
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Leonid Vasiliev @ 2020-04-18 15:29 UTC (permalink / raw)
To: v.shpilevoy, alexander.turenko; +Cc: tarantool-patches
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Session settings are stored in a monolithic array. Submodules
can define a range of settings in there. For example, SQL. It
occupies settings from 0 to 8. There is a second array of only
SQL settings in build.c, of the same size, and it uses the same
indexes.
But if something will be added before SQL settings, so as they
won't start from 0, it will break the equal indexes assumption.
SQL should normalize all setting identifiers by
SESSION_SETTING_SQL_BEGIN.
---
src/box/sql/build.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index aecba41..b1f9fed 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3367,7 +3367,8 @@ sql_session_setting_get(int id, const char **mp_pair, const char **mp_pair_end)
assert(id >= SESSION_SETTING_SQL_BEGIN && id < SESSION_SETTING_SQL_END);
struct session *session = current_session();
uint32_t flags = session->sql_flags;
- struct sql_option_metadata *opt = &sql_session_opts[id];
+ struct sql_option_metadata *opt =
+ &sql_session_opts[id - SESSION_SETTING_SQL_BEGIN];
uint32_t mask = opt->mask;
const char *name = session_setting_strs[id];
size_t name_len = strlen(name);
@@ -3404,7 +3405,8 @@ static int
sql_set_boolean_option(int id, bool value)
{
struct session *session = current_session();
- struct sql_option_metadata *option = &sql_session_opts[id];
+ struct sql_option_metadata *option =
+ &sql_session_opts[id - SESSION_SETTING_SQL_BEGIN];
assert(option->field_type == FIELD_TYPE_BOOLEAN);
#ifdef NDEBUG
if ((session->sql_flags & SQL_SqlTrace) == 0) {
@@ -3431,7 +3433,8 @@ sql_set_boolean_option(int id, bool value)
static int
sql_set_string_option(int id, const char *value)
{
- assert(sql_session_opts[id].field_type = FIELD_TYPE_STRING);
+ assert(sql_session_opts[id - SESSION_SETTING_SQL_BEGIN].field_type =
+ FIELD_TYPE_STRING);
assert(id == SESSION_SETTING_SQL_DEFAULT_ENGINE);
(void)id;
enum sql_storage_engine engine = STR2ENUM(sql_storage_engine, value);
@@ -3448,7 +3451,8 @@ sql_session_setting_set(int id, const char *mp_value)
{
assert(id >= SESSION_SETTING_SQL_BEGIN && id < SESSION_SETTING_SQL_END);
enum mp_type mtype = mp_typeof(*mp_value);
- enum field_type stype = sql_session_opts[id].field_type;
+ enum field_type stype =
+ sql_session_opts[id - SESSION_SETTING_SQL_BEGIN].field_type;
uint32_t len;
const char *tmp;
switch(stype) {
@@ -3473,10 +3477,10 @@ sql_session_setting_set(int id, const char *mp_value)
void
sql_session_settings_init()
{
- for (int id = SESSION_SETTING_SQL_BEGIN; id < SESSION_SETTING_SQL_END;
- ++id) {
+ for (int i = 0, id = SESSION_SETTING_SQL_BEGIN;
+ id < SESSION_SETTING_SQL_END; ++id, ++i) {
struct session_setting *setting = &session_settings[id];
- setting->field_type = sql_session_opts[id].field_type;
+ setting->field_type = sql_session_opts[i].field_type;
setting->get = sql_session_setting_get;
setting->set = sql_session_setting_set;
}
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [Tarantool-patches] [PATCH V5 4/6] error: add session setting for error type marshaling
2020-04-18 15:29 [Tarantool-patches] [PATCH V5 0/6] Extending error functionality Leonid Vasiliev
` (2 preceding siblings ...)
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 3/6] session: add offset to SQL session settings array Leonid Vasiliev
@ 2020-04-18 15:29 ` Leonid Vasiliev
2020-04-18 20:40 ` Vladislav Shpilevoy
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 5/6] error: update constructors of some errors Leonid Vasiliev
` (2 subsequent siblings)
6 siblings, 1 reply; 13+ messages in thread
From: Leonid Vasiliev @ 2020-04-18 15:29 UTC (permalink / raw)
To: v.shpilevoy, alexander.turenko; +Cc: tarantool-patches
Errors are encoded as a string when serialized to MessagePack to
be sent over IProto or when just saved into a buffer via Lua
modules msgpackffi and msgpack.
That is not very useful on client-side, because most of the error
metadata is lost: code, type, trace - everything except the
message.
Next commits are going to dedicate a new MP_EXT type to error
objects so as everything could be encoded, and on client side it
would be possible to restore types.
But this is a breaking change in case some users use old
connectors when work with newer Tarantool instances. So to smooth
the upgrade there is a new session setting -
'error_marshaling_enabled'.
By default it is false. When it is true, all fibers of the given
session will serialize error objects as MP_EXT.
Co-authored-by: Vladislav Shpilevoy<v.shpilevoy@tarantool.org>
Needed for #4398
---
src/box/lua/call.c | 33 ++++++++++++++---------
src/box/lua/execute.c | 2 +-
src/box/lua/tuple.c | 12 ++++-----
src/box/session.cc | 5 +++-
src/box/session.h | 5 ++--
src/box/session_settings.c | 56 ++++++++++++++++++++++++++++++++++++++++
src/box/session_settings.h | 1 +
src/box/sql/func.c | 4 ++-
src/lua/msgpack.c | 25 +++++++++---------
src/lua/msgpack.h | 8 +++---
src/lua/utils.c | 11 +++++---
src/lua/utils.h | 8 +++---
src/serializer_opts.h | 44 +++++++++++++++++++++++++++++++
test/box/session_settings.result | 3 ++-
14 files changed, 171 insertions(+), 46 deletions(-)
create mode 100644 src/serializer_opts.h
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index 1d29494..b7ebec3 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -46,6 +46,7 @@
#include "small/obuf.h"
#include "trivia/util.h"
#include "mpstream/mpstream.h"
+#include "box/session.h"
/**
* A helper to find a Lua function by name and put it
@@ -174,7 +175,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
*/
for (int i = 1; i <= nrets; ++i) {
struct luaL_field field;
- if (luaL_tofield(L, cfg, i, &field) < 0)
+ if (luaL_tofield(L, cfg, NULL, i, &field) < 0)
return luaT_error(L);
struct tuple *tuple;
if (field.type == MP_EXT &&
@@ -188,11 +189,11 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
*/
lua_pushvalue(L, i);
mpstream_encode_array(stream, 1);
- luamp_encode_r(L, cfg, stream, &field, 0);
+ luamp_encode_r(L, cfg, NULL, stream, &field, 0);
lua_pop(L, 1);
} else {
/* `return ..., array, ...` */
- luamp_encode(L, cfg, stream, i);
+ luamp_encode(L, cfg, NULL, stream, i);
}
}
return nrets;
@@ -203,7 +204,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
* Inspect the first result
*/
struct luaL_field root;
- if (luaL_tofield(L, cfg, 1, &root) < 0)
+ if (luaL_tofield(L, cfg, NULL, 1, &root) < 0)
return luaT_error(L);
struct tuple *tuple;
if (root.type == MP_EXT && (tuple = luaT_istuple(L, 1)) != NULL) {
@@ -217,7 +218,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
*/
mpstream_encode_array(stream, 1);
assert(lua_gettop(L) == 1);
- luamp_encode_r(L, cfg, stream, &root, 0);
+ luamp_encode_r(L, cfg, NULL, stream, &root, 0);
return 1;
}
@@ -233,7 +234,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
for (uint32_t t = 1; t <= root.size; t++) {
lua_rawgeti(L, 1, t);
struct luaL_field field;
- if (luaL_tofield(L, cfg, -1, &field) < 0)
+ if (luaL_tofield(L, cfg, NULL, -1, &field) < 0)
return luaT_error(L);
if (field.type == MP_EXT && (tuple = luaT_istuple(L, -1))) {
tuple_to_mpstream(tuple, stream);
@@ -249,13 +250,13 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
* Encode the first field of tuple using
* existing information from luaL_tofield
*/
- luamp_encode_r(L, cfg, stream, &field, 0);
+ luamp_encode_r(L, cfg, NULL, stream, &field, 0);
lua_pop(L, 1);
assert(lua_gettop(L) == 1);
/* Encode remaining fields as usual */
for (uint32_t f = 2; f <= root.size; f++) {
lua_rawgeti(L, 1, f);
- luamp_encode(L, cfg, stream, -1);
+ luamp_encode(L, cfg, NULL, stream, -1);
lua_pop(L, 1);
}
return 1;
@@ -265,10 +266,10 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
* { tuple/array, ..., { scalar }, ... }`
*/
mpstream_encode_array(stream, 1);
- luamp_encode_r(L, cfg, stream, &field, 0);
+ luamp_encode_r(L, cfg, NULL, stream, &field, 0);
} else {
/* `return { tuple/array, ..., tuple/array, ... }` */
- luamp_encode_r(L, cfg, stream, &field, 0);
+ luamp_encode_r(L, cfg, NULL, stream, &field, 0);
}
lua_pop(L, 1);
assert(lua_gettop(L) == 1);
@@ -379,6 +380,11 @@ execute_lua_eval(lua_State *L)
struct encode_lua_ctx {
struct port_lua *port;
struct mpstream *stream;
+ /**
+ * Lua serializer additional options.
+ * To set the session specific serialization options.
+ */
+ struct serializer_opts *serializer_opts;
};
static int
@@ -393,8 +399,10 @@ encode_lua_call(lua_State *L)
*/
struct luaL_serializer *cfg = luaL_msgpack_default;
int size = lua_gettop(ctx->port->L);
- for (int i = 1; i <= size; ++i)
- luamp_encode(ctx->port->L, cfg, ctx->stream, i);
+ for (int i = 1; i <= size; ++i) {
+ luamp_encode(ctx->port->L, cfg, ctx->serializer_opts,
+ ctx->stream, i);
+ }
ctx->port->size = size;
mpstream_flush(ctx->stream);
return 0;
@@ -429,6 +437,7 @@ port_lua_do_dump(struct port *base, struct mpstream *stream,
struct encode_lua_ctx ctx;
ctx.port = port;
ctx.stream = stream;
+ ctx.serializer_opts = ¤t_session()->meta.serializer_opts;
struct lua_State *L = tarantool_L;
int top = lua_gettop(L);
if (lua_cpcall(L, handler, &ctx) != 0) {
diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
index b4843b2..b5db3c9 100644
--- a/src/box/lua/execute.c
+++ b/src/box/lua/execute.c
@@ -328,7 +328,7 @@ lua_sql_bind_decode(struct lua_State *L, struct sql_bind *bind, int idx, int i)
bind->name = NULL;
bind->name_len = 0;
}
- if (luaL_tofield(L, luaL_msgpack_default, -1, &field) < 0)
+ if (luaL_tofield(L, luaL_msgpack_default, NULL, -1, &field) < 0)
return -1;
switch (field.type) {
case MP_UINT:
diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index 4b0701e..aba906d 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -120,7 +120,7 @@ luaT_tuple_new(struct lua_State *L, int idx, box_tuple_format_t *format)
int argc = lua_gettop(L);
mpstream_encode_array(&stream, argc);
for (int k = 1; k <= argc; ++k) {
- luamp_encode(L, luaL_msgpack_default, &stream, k);
+ luamp_encode(L, luaL_msgpack_default, NULL, &stream, k);
}
} else {
/* Create the tuple from a Lua table. */
@@ -252,18 +252,18 @@ luamp_convert_key(struct lua_State *L, struct luaL_serializer *cfg,
return tuple_to_mpstream(tuple, stream);
struct luaL_field field;
- if (luaL_tofield(L, cfg, index, &field) < 0)
+ if (luaL_tofield(L, cfg, NULL, index, &field) < 0)
luaT_error(L);
if (field.type == MP_ARRAY) {
lua_pushvalue(L, index);
- luamp_encode_r(L, cfg, stream, &field, 0);
+ luamp_encode_r(L, cfg, NULL, stream, &field, 0);
lua_pop(L, 1);
} else if (field.type == MP_NIL) {
mpstream_encode_array(stream, 0);
} else {
mpstream_encode_array(stream, 1);
lua_pushvalue(L, index);
- luamp_encode_r(L, cfg, stream, &field, 0);
+ luamp_encode_r(L, cfg, NULL, stream, &field, 0);
lua_pop(L, 1);
}
}
@@ -275,7 +275,7 @@ luamp_encode_tuple(struct lua_State *L, struct luaL_serializer *cfg,
struct tuple *tuple = luaT_istuple(L, index);
if (tuple != NULL) {
return tuple_to_mpstream(tuple, stream);
- } else if (luamp_encode(L, cfg, stream, index) != MP_ARRAY) {
+ } else if (luamp_encode(L, cfg, NULL, stream, index) != MP_ARRAY) {
diag_set(ClientError, ER_TUPLE_NOT_ARRAY);
luaT_error(L);
}
@@ -435,7 +435,7 @@ lbox_tuple_transform(struct lua_State *L)
mpstream_encode_array(&stream, 3);
mpstream_encode_str(&stream, "!");
mpstream_encode_uint(&stream, offset);
- luamp_encode(L, luaL_msgpack_default, &stream, i);
+ luamp_encode(L, luaL_msgpack_default, NULL, &stream, i);
}
mpstream_flush(&stream);
diff --git a/src/box/session.cc b/src/box/session.cc
index b557eed..08a1092 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -275,6 +275,9 @@ session_find(uint64_t sid)
mh_i64ptr_node(session_registry, k)->val;
}
+extern "C" void
+session_settings_init(void);
+
void
session_init()
{
@@ -283,7 +286,7 @@ session_init()
panic("out of memory");
mempool_create(&session_pool, &cord()->slabc, sizeof(struct session));
credentials_create(&admin_credentials, admin_user);
- sql_session_settings_init();
+ session_settings_init();
}
void
diff --git a/src/box/session.h b/src/box/session.h
index 8693799..500a88b 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -36,13 +36,12 @@
#include "fiber.h"
#include "user.h"
#include "authentication.h"
+#include "serializer_opts.h"
#if defined(__cplusplus)
extern "C" {
#endif /* defined(__cplusplus) */
-extern void sql_session_settings_init();
-
struct port;
struct session_vtab;
@@ -90,6 +89,8 @@ struct session_meta {
};
/** Console output format. */
enum output_format output_format;
+ /** Session-specific serialization options. */
+ struct serializer_opts serializer_opts;
};
/**
diff --git a/src/box/session_settings.c b/src/box/session_settings.c
index 79c4b8d..dbbbf24 100644
--- a/src/box/session_settings.c
+++ b/src/box/session_settings.c
@@ -42,6 +42,7 @@ struct session_setting session_settings[SESSION_SETTING_COUNT] = {};
/** Corresponding names of session settings. */
const char *session_setting_strs[SESSION_SETTING_COUNT] = {
+ "error_marshaling_enabled",
"sql_default_engine",
"sql_defer_foreign_keys",
"sql_full_column_names",
@@ -449,3 +450,58 @@ session_setting_find(const char *name) {
else
return -1;
}
+
+/* Module independent session settings. */
+
+static void
+session_setting_error_marshaling_enabled_get(int id, const char **mp_pair,
+ const char **mp_pair_end)
+{
+ assert(id == SESSION_SETTING_ERROR_MARSHALING_ENABLED);
+ struct session *session = current_session();
+ const char *name = session_setting_strs[id];
+ size_t name_len = strlen(name);
+ bool value = session->meta.serializer_opts.error_marshaling_enabled;
+ size_t size = mp_sizeof_array(2) + mp_sizeof_str(name_len) +
+ mp_sizeof_bool(value);
+
+ char *pos = (char*)static_alloc(size);
+ assert(pos != NULL);
+ char *pos_end = mp_encode_array(pos, 2);
+ pos_end = mp_encode_str(pos_end, name, name_len);
+ pos_end = mp_encode_bool(pos_end, value);
+ *mp_pair = pos;
+ *mp_pair_end = pos_end;
+}
+
+static int
+session_setting_error_marshaling_enabled_set(int id, const char *mp_value)
+{
+ assert(id == SESSION_SETTING_ERROR_MARSHALING_ENABLED);
+ enum mp_type mtype = mp_typeof(*mp_value);
+ enum field_type stype = session_settings[id].field_type;
+ if (mtype != MP_BOOL) {
+ diag_set(ClientError, ER_SESSION_SETTING_INVALID_VALUE,
+ session_setting_strs[id], field_type_strs[stype]);
+ return -1;
+ }
+ struct session *session = current_session();
+ session->meta.serializer_opts.error_marshaling_enabled =
+ mp_decode_bool(&mp_value);
+ return 0;
+}
+
+extern void
+sql_session_settings_init();
+
+void
+session_settings_init(void)
+{
+ struct session_setting *s =
+ &session_settings[SESSION_SETTING_ERROR_MARSHALING_ENABLED];
+ s->field_type = FIELD_TYPE_BOOLEAN;
+ s->get = session_setting_error_marshaling_enabled_get;
+ s->set = session_setting_error_marshaling_enabled_set;
+
+ sql_session_settings_init();
+}
diff --git a/src/box/session_settings.h b/src/box/session_settings.h
index e2adc52..6560b8c 100644
--- a/src/box/session_settings.h
+++ b/src/box/session_settings.h
@@ -42,6 +42,7 @@
* space iterator will not be sorted properly.
*/
enum {
+ SESSION_SETTING_ERROR_MARSHALING_ENABLED,
SESSION_SETTING_SQL_BEGIN,
SESSION_SETTING_SQL_DEFAULT_ENGINE = SESSION_SETTING_SQL_BEGIN,
SESSION_SETTING_SQL_DEFER_FOREIGN_KEYS,
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index e3a6e88..3768372 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -257,8 +257,10 @@ port_lua_get_vdbemem(struct port *base, uint32_t *size)
return NULL;
for (int i = 0; i < argc; i++) {
struct luaL_field field;
- if (luaL_tofield(L, luaL_msgpack_default, -1 - i, &field) < 0)
+ if (luaL_tofield(L, luaL_msgpack_default,
+ NULL, -1 - i, &field) < 0) {
goto error;
+ }
switch (field.type) {
case MP_BOOL:
mem_set_bool(&val[i], field.bval);
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index e4fb0cf..b14361f 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -109,8 +109,8 @@ luamp_set_decode_extension(luamp_decode_extension_f handler)
enum mp_type
luamp_encode_r(struct lua_State *L, struct luaL_serializer *cfg,
- struct mpstream *stream, struct luaL_field *field,
- int level)
+ struct serializer_opts *opts, struct mpstream *stream,
+ struct luaL_field *field, int level)
{
int top = lua_gettop(L);
enum mp_type type;
@@ -155,13 +155,13 @@ restart: /* used by MP_EXT of unidentified subtype */
lua_pushnil(L); /* first key */
while (lua_next(L, top) != 0) {
lua_pushvalue(L, -2); /* push a copy of key to top */
- if (luaL_tofield(L, cfg, lua_gettop(L), field) < 0)
+ if (luaL_tofield(L, cfg, opts, lua_gettop(L), field) < 0)
return luaT_error(L);
- luamp_encode_r(L, cfg, stream, field, level + 1);
+ luamp_encode_r(L, cfg, opts, stream, field, level + 1);
lua_pop(L, 1); /* pop a copy of key */
- if (luaL_tofield(L, cfg, lua_gettop(L), field) < 0)
+ if (luaL_tofield(L, cfg, opts, lua_gettop(L), field) < 0)
return luaT_error(L);
- luamp_encode_r(L, cfg, stream, field, level + 1);
+ luamp_encode_r(L, cfg, opts, stream, field, level + 1);
lua_pop(L, 1); /* pop value */
}
assert(lua_gettop(L) == top);
@@ -180,9 +180,9 @@ restart: /* used by MP_EXT of unidentified subtype */
mpstream_encode_array(stream, size);
for (uint32_t i = 0; i < size; i++) {
lua_rawgeti(L, top, i + 1);
- if (luaL_tofield(L, cfg, top + 1, field) < 0)
+ if (luaL_tofield(L, cfg, opts, top + 1, field) < 0)
return luaT_error(L);
- luamp_encode_r(L, cfg, stream, field, level + 1);
+ luamp_encode_r(L, cfg, opts, stream, field, level + 1);
lua_pop(L, 1);
}
assert(lua_gettop(L) == top);
@@ -213,7 +213,8 @@ restart: /* used by MP_EXT of unidentified subtype */
enum mp_type
luamp_encode(struct lua_State *L, struct luaL_serializer *cfg,
- struct mpstream *stream, int index)
+ struct serializer_opts *opts, struct mpstream *stream,
+ int index)
{
int top = lua_gettop(L);
if (index < 0)
@@ -225,9 +226,9 @@ luamp_encode(struct lua_State *L, struct luaL_serializer *cfg,
}
struct luaL_field field;
- if (luaL_tofield(L, cfg, lua_gettop(L), &field) < 0)
+ if (luaL_tofield(L, cfg, opts, lua_gettop(L), &field) < 0)
return luaT_error(L);
- enum mp_type top_type = luamp_encode_r(L, cfg, stream, &field, 0);
+ enum mp_type top_type = luamp_encode_r(L, cfg, opts, stream, &field, 0);
if (!on_top) {
lua_remove(L, top + 1); /* remove a value copy */
@@ -369,7 +370,7 @@ lua_msgpack_encode(lua_State *L)
mpstream_init(&stream, buf, ibuf_reserve_cb, ibuf_alloc_cb,
luamp_error, L);
- luamp_encode(L, cfg, &stream, 1);
+ luamp_encode(L, cfg, NULL, &stream, 1);
mpstream_flush(&stream);
if (index > 1) {
diff --git a/src/lua/msgpack.h b/src/lua/msgpack.h
index d0ade30..279e056 100644
--- a/src/lua/msgpack.h
+++ b/src/lua/msgpack.h
@@ -43,6 +43,7 @@ extern "C" {
struct luaL_serializer;
struct mpstream;
+struct serializer_opts;
/**
* Default instance of msgpack serializer (msgpack = require('msgpack')).
@@ -63,12 +64,13 @@ enum { LUAMP_ALLOC_FACTOR = 256 };
/* low-level function needed for execute_lua_call() */
enum mp_type
luamp_encode_r(struct lua_State *L, struct luaL_serializer *cfg,
- struct mpstream *stream, struct luaL_field *field,
- int level);
+ struct serializer_opts *opts, struct mpstream *stream,
+ struct luaL_field *field, int level);
enum mp_type
luamp_encode(struct lua_State *L, struct luaL_serializer *cfg,
- struct mpstream *stream, int index);
+ struct serializer_opts *opts, struct mpstream *stream,
+ int index);
void
luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 667365f..bd320f3 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -452,7 +452,7 @@ lua_field_inspect_ucdata(struct lua_State *L, struct luaL_serializer *cfg,
lua_pcall(L, 1, 1, 0);
/* replace obj with the unpacked value */
lua_replace(L, idx);
- if (luaL_tofield(L, cfg, idx, field) < 0)
+ if (luaL_tofield(L, cfg, NULL, idx, field) < 0)
luaT_error(L);
} /* else ignore lua_gettable exceptions */
lua_settop(L, top); /* remove temporary objects */
@@ -515,7 +515,7 @@ lua_field_try_serialize(struct lua_State *L)
/* copy object itself */
lua_pushvalue(L, 1);
lua_call(L, 1, 1);
- s->is_error = (luaL_tofield(L, cfg, -1, field) != 0);
+ s->is_error = (luaL_tofield(L, cfg, NULL, -1, field) != 0);
s->is_value_returned = true;
return 1;
}
@@ -641,14 +641,17 @@ lua_field_tostring(struct lua_State *L, struct luaL_serializer *cfg, int idx,
lua_call(L, 1, 1);
lua_replace(L, idx);
lua_settop(L, top);
- if (luaL_tofield(L, cfg, idx, field) < 0)
+ if (luaL_tofield(L, cfg, NULL, idx, field) < 0)
luaT_error(L);
}
int
-luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, int index,
+luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
+ struct serializer_opts *opts, int index,
struct luaL_field *field)
{
+ /* opts will be used for encode MP_ERROR in the future */
+ (void)opts;
if (index < 0)
index = lua_gettop(L) + index + 1;
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 4bc0417..0024b09 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -59,6 +59,7 @@ extern "C" {
#include "lib/core/mp_extension_types.h"
#include "lib/core/decimal.h" /* decimal_t */
+struct serializer_opts;
struct lua_State;
struct ibuf;
struct tt_uuid;
@@ -366,6 +367,7 @@ struct luaL_field {
*
* @param L stack
* @param cfg configuration
+ * @param opts the Lua serializer additional options.
* @param index stack index
* @param field conversion result
*
@@ -373,8 +375,8 @@ struct luaL_field {
* @retval -1 Error.
*/
int
-luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, int index,
- struct luaL_field *field);
+luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
+ struct serializer_opts *opts, int index, struct luaL_field *field);
/**
* @brief Try to convert userdata/cdata values using defined conversion logic.
@@ -412,7 +414,7 @@ static inline void
luaL_checkfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
struct luaL_field *field)
{
- if (luaL_tofield(L, cfg, idx, field) < 0)
+ if (luaL_tofield(L, cfg, NULL, idx, field) < 0)
luaT_error(L);
if (field->type != MP_EXT || field->ext_type != MP_UNKNOWN_EXTENSION)
return;
diff --git a/src/serializer_opts.h b/src/serializer_opts.h
new file mode 100644
index 0000000..9e2c15e
--- /dev/null
+++ b/src/serializer_opts.h
@@ -0,0 +1,44 @@
+#pragma once
+/*
+ * Copyright 2010-2020, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+/**
+ * Serializer options which can regulate how to serialize
+ * something in scope of one session.
+ */
+struct serializer_opts {
+ /**
+ * When enabled, error objects get their own MP_EXT
+ * MessagePack type and therefore can be type-safely
+ * transmitted over the network.
+ */
+ bool error_marshaling_enabled;
+};
diff --git a/test/box/session_settings.result b/test/box/session_settings.result
index ea6302d..149cc4b 100644
--- a/test/box/session_settings.result
+++ b/test/box/session_settings.result
@@ -52,7 +52,8 @@ s:replace({'sql_defer_foreign_keys', true})
--
s:select()
| ---
- | - - ['sql_default_engine', 'memtx']
+ | - - ['error_marshaling_enabled', false]
+ | - ['sql_default_engine', 'memtx']
| - ['sql_defer_foreign_keys', false]
| - ['sql_full_column_names', false]
| - ['sql_full_metadata', false]
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH V5 4/6] error: add session setting for error type marshaling
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 4/6] error: add session setting for error type marshaling Leonid Vasiliev
@ 2020-04-18 20:40 ` Vladislav Shpilevoy
0 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-18 20:40 UTC (permalink / raw)
To: Leonid Vasiliev, alexander.turenko; +Cc: tarantool-patches
Thanks for the patch!
I force pushed the following diff:
====================
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index b7ebec3b6..6c8c353ab 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -380,11 +380,6 @@ execute_lua_eval(lua_State *L)
struct encode_lua_ctx {
struct port_lua *port;
struct mpstream *stream;
- /**
- * Lua serializer additional options.
- * To set the session specific serialization options.
- */
- struct serializer_opts *serializer_opts;
};
static int
@@ -398,11 +393,10 @@ encode_lua_call(lua_State *L)
* TODO: forbid explicit yield from __serialize or __index here
*/
struct luaL_serializer *cfg = luaL_msgpack_default;
+ struct serializer_opts *opts = ¤t_session()->meta.serializer_opts;
int size = lua_gettop(ctx->port->L);
- for (int i = 1; i <= size; ++i) {
- luamp_encode(ctx->port->L, cfg, ctx->serializer_opts,
- ctx->stream, i);
- }
+ for (int i = 1; i <= size; ++i)
+ luamp_encode(ctx->port->L, cfg, opts, ctx->stream, i);
ctx->port->size = size;
mpstream_flush(ctx->stream);
return 0;
@@ -437,7 +431,6 @@ port_lua_do_dump(struct port *base, struct mpstream *stream,
struct encode_lua_ctx ctx;
ctx.port = port;
ctx.stream = stream;
- ctx.serializer_opts = ¤t_session()->meta.serializer_opts;
struct lua_State *L = tarantool_L;
int top = lua_gettop(L);
if (lua_cpcall(L, handler, &ctx) != 0) {
====================
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH V5 5/6] error: update constructors of some errors
2020-04-18 15:29 [Tarantool-patches] [PATCH V5 0/6] Extending error functionality Leonid Vasiliev
` (3 preceding siblings ...)
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 4/6] error: add session setting for error type marshaling Leonid Vasiliev
@ 2020-04-18 15:29 ` Leonid Vasiliev
2020-04-18 20:39 ` Vladislav Shpilevoy
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 6/6] error: add error MsgPack encoding Leonid Vasiliev
2020-04-18 21:14 ` [Tarantool-patches] [PATCH V5 5.5/6] box: move Lua MP_EXT decoder from tuple.c Vladislav Shpilevoy
6 siblings, 1 reply; 13+ messages in thread
From: Leonid Vasiliev @ 2020-04-18 15:29 UTC (permalink / raw)
To: v.shpilevoy, alexander.turenko; +Cc: tarantool-patches
We want to have a transparent marshalling through net.box
for errors. To do this, we need to recreate the error
on the client side with the same parameters as on the server.
For convenience, we update AccessDeniedError constructor
which has points to static strings and add the XlogGapError
constructor that does not require vclock.
Needed for #4398
---
src/box/error.cc | 25 ++++++++++++++-----------
src/box/error.h | 11 ++++++++---
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/src/box/error.cc b/src/box/error.cc
index 277727d..0d2ba83 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -255,6 +255,13 @@ XlogGapError::XlogGapError(const char *file, unsigned line,
(long long) vclock_sum(to), s_to ? s_to : "");
}
+XlogGapError::XlogGapError(const char *file, unsigned line,
+ const char *msg)
+ : XlogError(&type_XlogGapError, file, line)
+{
+ error_format_msg(this, "%s", msg);
+}
+
struct error *
BuildXlogGapError(const char *file, unsigned line,
const struct vclock *from, const struct vclock *to)
@@ -283,23 +290,19 @@ AccessDeniedError::AccessDeniedError(const char *file, unsigned int line,
const char *access_type,
const char *object_type,
const char *object_name,
- const char *user_name)
+ const char *user_name,
+ bool run_trigers)
:ClientError(&type_AccessDeniedError, file, line, ER_ACCESS_DENIED)
{
error_format_msg(this, tnt_errcode_desc(m_errcode),
access_type, object_type, object_name, user_name);
struct on_access_denied_ctx ctx = {access_type, object_type, object_name};
- trigger_run(&on_access_denied, (void *) &ctx);
- /*
- * We want to use ctx parameters as error parameters
- * later, so we have to alloc space for it.
- * As m_access_type and m_object_type are constant
- * literals they are statically allocated. We must copy
- * only m_object_name.
- */
- m_object_type = object_type;
- m_access_type = access_type;
+ /* Don't run the triggers when create after marshaling through net */
+ if (run_trigers)
+ trigger_run(&on_access_denied, (void *) &ctx);
+ m_object_type = strdup(object_type);
+ m_access_type = strdup(access_type);
m_object_name = strdup(object_name);
}
diff --git a/src/box/error.h b/src/box/error.h
index 10f282f..9bef8a9 100644
--- a/src/box/error.h
+++ b/src/box/error.h
@@ -240,11 +240,14 @@ class AccessDeniedError: public ClientError
public:
AccessDeniedError(const char *file, unsigned int line,
const char *access_type, const char *object_type,
- const char *object_name, const char *user_name);
+ const char *object_name, const char *user_name,
+ bool run_trigers = true);
~AccessDeniedError()
{
free(m_object_name);
+ free(m_object_type);
+ free(m_access_type);
}
const char *
@@ -267,11 +270,11 @@ public:
private:
/** Type of object the required access was denied to */
- const char *m_object_type;
+ char *m_object_type;
/** Name of object the required access was denied to */
char *m_object_name;
/** Type of declined access */
- const char *m_access_type;
+ char *m_access_type;
};
/**
@@ -301,6 +304,8 @@ struct XlogGapError: public XlogError
{
XlogGapError(const char *file, unsigned line,
const struct vclock *from, const struct vclock *to);
+ XlogGapError(const char *file, unsigned line,
+ const char *msg);
virtual void raise() { throw this; }
};
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH V5 5/6] error: update constructors of some errors
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 5/6] error: update constructors of some errors Leonid Vasiliev
@ 2020-04-18 20:39 ` Vladislav Shpilevoy
0 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-18 20:39 UTC (permalink / raw)
To: Leonid Vasiliev, alexander.turenko; +Cc: tarantool-patches
Thanks for the patch!
On 18/04/2020 17:29, Leonid Vasiliev wrote:
> We want to have a transparent marshalling through net.box
> for errors. To do this, we need to recreate the error
> on the client side with the same parameters as on the server.
> For convenience, we update AccessDeniedError constructor
> which has points to static strings and add the XlogGapError
I force pushed 'points' -> 'pointers'.
> constructor that does not require vclock.
>
> Needed for #4398
> ---
> src/box/error.cc | 25 ++++++++++++++-----------
> src/box/error.h | 11 ++++++++---
> 2 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/src/box/error.cc b/src/box/error.cc
> index 277727d..0d2ba83 100644
> --- a/src/box/error.cc
> +++ b/src/box/error.cc
> @@ -283,23 +290,19 @@ AccessDeniedError::AccessDeniedError(const char *file, unsigned int line,
> const char *access_type,
> const char *object_type,
> const char *object_name,
> - const char *user_name)
> + const char *user_name,
> + bool run_trigers)
> :ClientError(&type_AccessDeniedError, file, line, ER_ACCESS_DENIED)
> {
> error_format_msg(this, tnt_errcode_desc(m_errcode),
> access_type, object_type, object_name, user_name);
>
> struct on_access_denied_ctx ctx = {access_type, object_type, object_name};
> - trigger_run(&on_access_denied, (void *) &ctx);
> - /*
> - * We want to use ctx parameters as error parameters
> - * later, so we have to alloc space for it.
> - * As m_access_type and m_object_type are constant
> - * literals they are statically allocated. We must copy
> - * only m_object_name.
> - */
> - m_object_type = object_type;
> - m_access_type = access_type;
> + /* Don't run the triggers when create after marshaling through net */
I force pushed the following diff:
====================
diff --git a/src/box/error.cc b/src/box/error.cc
index 0d2ba83b7..c3c2af3ab 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -298,7 +298,10 @@ AccessDeniedError::AccessDeniedError(const char *file, unsigned int line,
access_type, object_type, object_name, user_name);
struct on_access_denied_ctx ctx = {access_type, object_type, object_name};
- /* Don't run the triggers when create after marshaling through net */
+ /*
+ * Don't run the triggers when create after marshaling
+ * through network.
+ */
if (run_trigers)
trigger_run(&on_access_denied, (void *) &ctx);
m_object_type = strdup(object_type);
====================
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH V5 6/6] error: add error MsgPack encoding
2020-04-18 15:29 [Tarantool-patches] [PATCH V5 0/6] Extending error functionality Leonid Vasiliev
` (4 preceding siblings ...)
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 5/6] error: update constructors of some errors Leonid Vasiliev
@ 2020-04-18 15:29 ` Leonid Vasiliev
2020-04-18 20:39 ` Vladislav Shpilevoy
2020-04-18 21:14 ` [Tarantool-patches] [PATCH V5 5.5/6] box: move Lua MP_EXT decoder from tuple.c Vladislav Shpilevoy
6 siblings, 1 reply; 13+ messages in thread
From: Leonid Vasiliev @ 2020-04-18 15:29 UTC (permalink / raw)
To: v.shpilevoy, alexander.turenko; +Cc: tarantool-patches
Co-authored-by: Vladislav Shpilevoy<v.shpilevoy@tarantool.org>
Closes #4398
@TarantoolBot document
Title: msgpack format for MP_ERROR
For sent an error over IProto MP_ERROR(0x03) subtype
has been added to MsgPack MP_EXT.
Now, if session setting 'error_marshaling_enabled'
is true error will be encoded as:
```
+--------+----------+========+
| MP_EXT | MP_ERROR | MP_MAP |
+--------+----------+========+
```
MP_ERROR: <MP_MAP> {
MP_ERROR_STACK: <MP_ARRAY> {
<MP_MAP> {
MP_ERROR_TYPE: <MP_STR>,
MP_ERROR_FILE: <MP_STR>,
MP_ERROR_LINE: <MP_UINT>,
MP_ERROR_REASON: <MP_STR>,
MP_ERROR_ERRNO: <MP_STR>,
MP_ERROR_FIELDS: <MP_MAP> {
<MP_STR>: ...,
<MP_STR>: ...,
...
},
},
}
}
MP_ERROR_STACK = 0x00
At the map passes all necessary error details (depending
from the error type) to create a copy of the error
on the client side.
The necessary fields:
MP_ERROR_TYPE = 0x00 - error type(string).
MP_ERROR_FILE = 0x01 - file name from trace.
MP_ERROR_LINE = 0x02 - line of the file from trace.
MP_ERROR_REASON = 0x03 - error message.
MP_ERROR_ERRNO = 0x04 - saved errno.
MP_ERROR_FIELDS = 0x05 - additional fields passes at MAP (MP_ERROR_FIELDS).
Now additional fields may content:
- ClientError code
- Custom type
- Access Denied object type
- Access Denied object name
- Access Denied access type
---
src/box/CMakeLists.txt | 1 +
src/box/lua/init.c | 56 ++++
src/box/lua/tuple.c | 16 -
src/box/mp_error.cc | 552 +++++++++++++++++++++++++++++++++++
src/box/mp_error.h | 60 ++++
src/lib/core/mp_extension_types.h | 1 +
src/lua/error.c | 2 -
src/lua/error.h | 3 +-
src/lua/msgpack.c | 3 +
src/lua/utils.c | 8 +-
test/box-tap/extended_error.test.lua | 136 +++++++++
11 files changed, 817 insertions(+), 21 deletions(-)
create mode 100644 src/box/mp_error.cc
create mode 100644 src/box/mp_error.h
create mode 100755 test/box-tap/extended_error.test.lua
diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 6688303..81f6400 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -136,6 +136,7 @@ add_library(box STATIC
wal.c
call.c
merger.c
+ mp_error.cc
${lua_sources}
lua/init.c
lua/call.c
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 63e8b82..beb59fe 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -36,11 +36,14 @@
#include "lua/utils.h" /* luaT_error() */
#include "lua/trigger.h"
+#include "lua/msgpack.h"
#include "box/box.h"
#include "box/txn.h"
#include "box/func.h"
#include "box/vclock.h"
+#include "box/session.h"
+#include "box/mp_error.h"
#include "box/lua/error.h"
#include "box/lua/tuple.h"
@@ -63,6 +66,8 @@
#include "box/lua/key_def.h"
#include "box/lua/merger.h"
+#include "mpstream/mpstream.h"
+
static uint32_t CTID_STRUCT_TXN_SAVEPOINT_PTR = 0;
extern char session_lua[],
@@ -386,6 +391,54 @@ static const struct luaL_Reg boxlib_backup[] = {
{NULL, NULL}
};
+/**
+ * A MsgPack extensions handler that supports tuples and errors encode.
+ */
+static enum mp_type
+luamp_encode_extension_box(struct lua_State *L, int idx,
+ struct mpstream *stream)
+{
+ struct tuple *tuple = luaT_istuple(L, idx);
+ if (tuple != NULL) {
+ tuple_to_mpstream(tuple, stream);
+ return MP_ARRAY;
+ }
+
+ if (current_session()->meta.serializer_opts.error_marshaling_enabled) {
+ struct error *err = luaL_iserror(L, idx);
+ if (err != NULL)
+ error_to_mpstream(err, stream);
+ }
+
+ return MP_EXT;
+}
+
+/**
+ * A MsgPack extensions handler that supports errors decode.
+ */
+static void
+luamp_decode_extension_box(struct lua_State *L, const char **data)
+{
+ assert(mp_typeof(**data) == MP_EXT);
+ int8_t ext_type;
+ uint32_t len = mp_decode_extl(data, &ext_type);
+
+ if (ext_type != MP_ERROR) {
+ luaL_error(L, "Unsuported MsgPack extension type: %u",
+ ext_type);
+ return;
+ }
+
+ struct error *err = error_unpack(data, len);
+ if (err == NULL) {
+ luaL_error(L, "Can not parse an error from MsgPack");
+ return;
+ }
+
+ luaT_pusherror(L, err);
+ return;
+}
+
#include "say.h"
void
@@ -426,6 +479,9 @@ box_lua_init(struct lua_State *L)
luaopen_merger(L);
lua_pop(L, 1);
+ luamp_set_encode_extension(luamp_encode_extension_box);
+ luamp_set_decode_extension(luamp_decode_extension_box);
+
/* Load Lua extension */
for (const char **s = lua_sources; *s; s += 2) {
const char *modname = *s;
diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index aba906d..03b4b8a 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -290,20 +290,6 @@ tuple_to_mpstream(struct tuple *tuple, struct mpstream *stream)
mpstream_advance(stream, bsize);
}
-/* A MsgPack extensions handler that supports tuples */
-static enum mp_type
-luamp_encode_extension_box(struct lua_State *L, int idx,
- struct mpstream *stream)
-{
- struct tuple *tuple = luaT_istuple(L, idx);
- if (tuple != NULL) {
- tuple_to_mpstream(tuple, stream);
- return MP_ARRAY;
- }
-
- return MP_EXT;
-}
-
/**
* Convert a tuple into lua table. Named fields are stored as
* {name = value} pairs. Not named fields are stored as
@@ -582,8 +568,6 @@ box_lua_tuple_init(struct lua_State *L)
luaL_register_module(L, tuplelib_name, lbox_tuplelib);
lua_pop(L, 1);
- luamp_set_encode_extension(luamp_encode_extension_box);
-
tuple_serializer_update_options();
trigger_create(&tuple_serializer.update_trigger,
on_msgpack_serializer_update, NULL, NULL);
diff --git a/src/box/mp_error.cc b/src/box/mp_error.cc
new file mode 100644
index 0000000..348b7a7
--- /dev/null
+++ b/src/box/mp_error.cc
@@ -0,0 +1,552 @@
+/*
+ * Copyright 2010-2020, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include "box/mp_error.h"
+#include "box/error.h"
+#include "mpstream/mpstream.h"
+#include "msgpuck.h"
+#include "mp_extension_types.h"
+#include "fiber.h"
+
+/**
+ * MP_ERROR format:
+ *
+ * MP_ERROR: <MP_MAP> {
+ * MP_ERROR_STACK: <MP_ARRAY> {
+ * <MP_MAP> {
+ * MP_ERROR_TYPE: <MP_STR>,
+ * MP_ERROR_FILE: <MP_STR>,
+ * MP_ERROR_LINE: <MP_UINT>,
+ * MP_ERROR_REASON: <MP_STR>,
+ * MP_ERROR_ERRNO: <MP_STR>,
+ * MP_ERROR_FIELDS: <MP_MAP> {
+ * <MP_STR>: ...,
+ * <MP_STR>: ...,
+ * ...
+ * },
+ * },
+ * }
+ * }
+ */
+
+/**
+ * MP_ERROR keys
+ */
+enum {
+ MP_ERROR_STACK = 0x00
+};
+
+/**
+ * Keys used for error encode/decode to MP_ERROR.
+ */
+enum {
+ /* Error type */
+ MP_ERROR_TYPE = 0x00,
+ /* File name from trace */
+ MP_ERROR_FILE = 0x01,
+ /* Line from trace */
+ MP_ERROR_LINE = 0x02,
+ /* Error message */
+ MP_ERROR_REASON = 0x03,
+ /* Saved errno */
+ MP_ERROR_ERRNO = 0x04,
+ /*
+ * Key uses for error type-specific fields which
+ * presents here as map: string key - value.
+ */
+ MP_ERROR_FIELDS = 0x05
+};
+
+/**
+ * The structure is used for storing parameters
+ * during decoding MP_ERROR.
+ */
+struct mp_error {
+ uint32_t error_code;
+ uint32_t line;
+ uint32_t saved_errno;
+ const char *error_type;
+ const char *file;
+ const char *reason;
+ const char *custom_type;
+ const char *ad_obj_type;
+ const char *ad_obj_name;
+ const char *ad_access_type;
+};
+
+static void
+mp_error_create(struct mp_error *mp_error)
+{
+ memset(mp_error, 0, sizeof(*mp_error));
+}
+
+static uint32_t
+encode_error_size(const struct error *error)
+{
+ uint32_t errcode = 0;
+
+ bool is_client = false;
+ bool is_custom = false;
+ bool is_access_denied = false;
+
+ if (strcmp(error->type->name, "ClientError") == 0) {
+ is_client = true;
+ } else if (strcmp(error->type->name, "CustomError") == 0) {
+ is_custom = true;
+ } else if (strcmp(error->type->name, "AccessDeniedError") == 0) {
+ is_access_denied = true;
+ }
+
+ uint32_t details_num = 5;
+ uint32_t data_size = 0;
+
+ data_size += mp_sizeof_uint(MP_ERROR_TYPE);
+ data_size += mp_sizeof_str(strlen(error->type->name));
+ data_size += mp_sizeof_uint(MP_ERROR_LINE);
+ data_size += mp_sizeof_uint(error->line);
+ data_size += mp_sizeof_uint(MP_ERROR_FILE);
+ data_size += mp_sizeof_str(strlen(error->file));
+ data_size += mp_sizeof_uint(MP_ERROR_REASON);
+ data_size += mp_sizeof_str(strlen(error->errmsg));
+ data_size += mp_sizeof_uint(MP_ERROR_ERRNO);
+ data_size += mp_sizeof_uint(error->saved_errno);
+
+ if (is_client) {
+ ++details_num;
+ data_size += mp_sizeof_uint(MP_ERROR_FIELDS);
+ errcode = box_error_code(error);
+ data_size += mp_sizeof_map(1);
+ data_size += mp_sizeof_str(strlen("code"));
+ data_size += mp_sizeof_uint(errcode);
+ } else if (is_access_denied) {
+ ++details_num;
+ data_size += mp_sizeof_uint(MP_ERROR_FIELDS);
+ errcode = box_error_code(error);
+ data_size += mp_sizeof_map(4);
+ data_size += mp_sizeof_str(strlen("code"));
+ data_size += mp_sizeof_uint(errcode);
+ AccessDeniedError *ad_err = type_cast(AccessDeniedError, error);
+ data_size += mp_sizeof_str(strlen("AD_OBJ_TYPE"));
+ data_size += mp_sizeof_str(strlen(ad_err->object_type()));
+ data_size += mp_sizeof_str(strlen("AD_OBJ_NAME"));
+ data_size += mp_sizeof_str(strlen(ad_err->object_name()));
+ data_size += mp_sizeof_str(strlen("AD_ACCESS_TYPE"));
+ data_size += mp_sizeof_str(strlen(ad_err->access_type()));
+ } else if (is_custom) {
+ ++details_num;
+ data_size += mp_sizeof_uint(MP_ERROR_FIELDS);
+ errcode = box_error_code(error);
+ data_size += mp_sizeof_map(2);
+ data_size += mp_sizeof_str(strlen("code"));
+ data_size += mp_sizeof_uint(errcode);
+ data_size += mp_sizeof_str(strlen("custom"));
+ data_size +=
+ mp_sizeof_str(strlen(box_error_custom_type(error)));
+ }
+
+ data_size += mp_sizeof_map(details_num);
+
+ return data_size;
+}
+
+static void
+encode_error(const struct error *error, char **data)
+{
+ uint32_t errcode = 0;
+
+ bool is_client = false;
+ bool is_custom = false;
+ bool is_access_denied = false;
+
+ if (strcmp(error->type->name, "ClientError") == 0) {
+ is_client = true;
+ } else if (strcmp(error->type->name, "CustomError") == 0) {
+ is_custom = true;
+ } else if (strcmp(error->type->name, "AccessDeniedError") == 0) {
+ is_access_denied = true;
+ }
+
+ uint32_t details_num = 5;
+
+ if (is_client || is_access_denied || is_custom)
+ ++details_num;
+
+ *data = mp_encode_map(*data, details_num);
+ *data = mp_encode_uint(*data, MP_ERROR_TYPE);
+ *data = mp_encode_str(*data, error->type->name,
+ strlen(error->type->name));
+ *data = mp_encode_uint(*data, MP_ERROR_LINE);
+ *data = mp_encode_uint(*data, error->line);
+ *data = mp_encode_uint(*data, MP_ERROR_FILE);
+ *data = mp_encode_str(*data, error->file, strlen(error->file));
+ *data = mp_encode_uint(*data, MP_ERROR_REASON);
+ *data = mp_encode_str(*data, error->errmsg, strlen(error->errmsg));
+ *data = mp_encode_uint(*data, MP_ERROR_ERRNO);
+ *data = mp_encode_uint(*data, error->saved_errno);
+
+ if (is_client) {
+ *data = mp_encode_uint(*data, MP_ERROR_FIELDS);
+ errcode = box_error_code(error);
+ *data = mp_encode_map(*data, 1);
+ *data = mp_encode_str(*data, "code",strlen("code"));
+ *data = mp_encode_uint(*data, errcode);
+ } else if (is_access_denied) {
+ *data = mp_encode_uint(*data, MP_ERROR_FIELDS);
+ errcode = box_error_code(error);
+ *data = mp_encode_map(*data, 4);
+ *data = mp_encode_str(*data, "code",strlen("code"));
+ *data = mp_encode_uint(*data, errcode);
+ AccessDeniedError *ad_err = type_cast(AccessDeniedError, error);
+ *data = mp_encode_str(*data, "AD_OBJ_TYPE",
+ strlen("AD_OBJ_TYPE"));
+ *data = mp_encode_str(*data, ad_err->object_type(),
+ strlen(ad_err->object_type()));
+ *data = mp_encode_str(*data, "AD_OBJ_NAME",
+ strlen("AD_OBJ_NAME"));
+ *data = mp_encode_str(*data, ad_err->object_name(),
+ strlen(ad_err->object_name()));
+ *data = mp_encode_str(*data, "AD_ACCESS_TYPE",
+ strlen("AD_ACCESS_TYPE"));
+ *data = mp_encode_str(*data, ad_err->access_type(),
+ strlen(ad_err->access_type()));
+ } else if (is_custom) {
+ *data = mp_encode_uint(*data, MP_ERROR_FIELDS);
+ errcode = box_error_code(error);
+ *data = mp_encode_map(*data, 2);
+ *data = mp_encode_str(*data, "code",strlen("code"));
+ *data = mp_encode_uint(*data, errcode);
+ *data = mp_encode_str(*data, "custom", strlen("custom"));
+ const char *custom = box_error_custom_type(error);
+ *data = mp_encode_str(*data, custom, strlen(custom));
+ }
+}
+
+static struct error *
+build_error_xc(struct mp_error *mp_error)
+{
+ /*
+ * To create an error the "raw" constructor using
+ * because OOM error must be thrown in OOM case.
+ * Builders returns a pointer to the static OOM error
+ * in OOM case.
+ */
+ struct error *err = NULL;
+
+ if (strcmp(mp_error->error_type, "ClientError") == 0) {
+ ClientError *e = new ClientError(mp_error->file, mp_error->line,
+ ER_UNKNOWN);
+ e->m_errcode = mp_error->error_code;
+ err = e;
+ } else if (strcmp(mp_error->error_type, "CustomError") == 0) {
+ err = new CustomError(mp_error->file, mp_error->line,
+ mp_error->custom_type,
+ mp_error->error_code);
+ } else if (strcmp(mp_error->error_type, "AccessDeniedError") == 0) {
+ err = new AccessDeniedError(mp_error->file, mp_error->line,
+ mp_error->ad_access_type,
+ mp_error->ad_obj_type,
+ mp_error->ad_obj_name, "", false);
+ } else if (strcmp(mp_error->error_type, "XlogError") == 0) {
+ err = new XlogError(&type_XlogError, mp_error->file,
+ mp_error->line);
+ error_format_msg(err, "%s", mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "XlogGapError") == 0) {
+ err = new XlogGapError(mp_error->file, mp_error->line,
+ mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "SystemError") == 0) {
+ err = new SystemError(mp_error->file, mp_error->line,
+ "%s", mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "SocketError") == 0) {
+ err = new SocketError(mp_error->file, mp_error->line, "", "");
+ error_format_msg(err, "%s", mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "OutOfMemory") == 0) {
+ err = new OutOfMemory(mp_error->file, mp_error->line,
+ 0, "", "");
+ error_format_msg(err, "%s", mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "TimedOut") == 0) {
+ err = new TimedOut(mp_error->file, mp_error->line);
+ } else if (strcmp(mp_error->error_type, "ChannelIsClosed") == 0) {
+ err = new ChannelIsClosed(mp_error->file, mp_error->line);
+ } else if (strcmp(mp_error->error_type, "FiberIsCancelled") == 0) {
+ err = new FiberIsCancelled(mp_error->file, mp_error->line);
+ } else if (strcmp(mp_error->error_type, "LuajitError") == 0) {
+ err = new LuajitError(mp_error->file, mp_error->line,
+ mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "IllegalParams") == 0) {
+ err = new IllegalParams(mp_error->file, mp_error->line,
+ "%s", mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "CollationError") == 0) {
+ err = new CollationError(mp_error->file, mp_error->line,
+ "%s", mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "SwimError") == 0) {
+ err = new SwimError(mp_error->file, mp_error->line,
+ "%s", mp_error->reason);
+ } else if (strcmp(mp_error->error_type, "CryptoError") == 0) {
+ err = new CryptoError(mp_error->file, mp_error->line,
+ "%s", mp_error->reason);
+ } else {
+ err = new ClientError(mp_error->file, mp_error->line,
+ ER_UNKNOWN);
+ }
+
+ if (err) {
+ err->saved_errno = mp_error->saved_errno;
+ error_format_msg(err, "%s", mp_error->reason);
+ }
+
+ return err;
+}
+
+static inline const char *
+region_strdup(struct region *region, const char *str, uint32_t len)
+{
+ char *res = (char *)region_alloc(region, len + 1);
+ if (res == NULL) {
+ diag_set(OutOfMemory, len + 1, "region_alloc", "res");
+ return NULL;
+ }
+ memcpy(res, str, len);
+ res[len] = 0;
+ return res;
+}
+
+static int
+decode_additional_fields(const char **data, struct region *region,
+ struct mp_error *mp_err)
+{
+ uint32_t map_sz = mp_decode_map(data);
+ const char *key;
+ uint32_t key_len;
+ const char *str;
+ uint32_t str_len;
+ for (uint32_t i = 0; i < map_sz; ++i) {
+ if (mp_typeof(**data) != MP_STR)
+ return -1;
+ key = mp_decode_str(data, &key_len);
+
+ if (strncmp(key, "code", key_len) == 0) {
+ if (mp_typeof(**data) != MP_UINT)
+ return -1;
+ mp_err->error_code = mp_decode_uint(data);
+ } else if (strncmp(key, "AD_OBJ_TYPE", key_len) == 0) {
+ if (mp_typeof(**data) != MP_STR)
+ return -1;
+ str = mp_decode_str(data, &str_len);
+ mp_err->ad_obj_type = region_strdup(region, str,
+ str_len);
+ if (mp_err->ad_obj_type == NULL)
+ return -1;
+ } else if (strncmp(key, "AD_OBJ_NAME", key_len) == 0) {
+ if (mp_typeof(**data) != MP_STR)
+ return -1;
+ str = mp_decode_str(data, &str_len);
+ mp_err->ad_obj_name = region_strdup(region, str,
+ str_len);
+ if (mp_err->ad_obj_name == NULL)
+ return -1;
+ } else if (strncmp(key, "AD_ACCESS_TYPE", key_len) == 0) {
+ if (mp_typeof(**data) != MP_STR)
+ return -1;
+ str = mp_decode_str(data, &str_len);
+ mp_err->ad_access_type = region_strdup(region, str,
+ str_len);
+ if (mp_err->ad_access_type == NULL)
+ return -1;
+ } else if (strncmp(key, "custom", key_len) == 0) {
+ if (mp_typeof(**data) != MP_STR)
+ return -1;
+ str = mp_decode_str(data, &str_len);
+ mp_err->custom_type = region_strdup(region, str,
+ str_len);
+ if (mp_err->custom_type == NULL)
+ return -1;
+ } else {
+ mp_next(data);
+ }
+ }
+ return 0;
+}
+
+static struct error *
+decode_error(const char **data)
+{
+ if (mp_typeof(**data) != MP_MAP) {
+ diag_set(ClientError, ER_INVALID_MSGPACK,
+ "Invalid MP_ERROR format");
+ return NULL;
+ }
+
+ struct mp_error mp_err;
+ mp_error_create(&mp_err);
+ struct region *region = &fiber()->gc;
+ uint32_t region_svp = region_used(region);
+ uint32_t map_size = mp_decode_map(data);
+
+ struct error *err = NULL;
+ for (uint32_t i = 0; i < map_size; ++i) {
+ if (mp_typeof(**data) != MP_UINT) {
+ diag_set(ClientError, ER_INVALID_MSGPACK,
+ "Invalid MP_ERROR MsgPack format");
+ goto finish;
+ }
+
+ uint64_t key = mp_decode_uint(data);
+ const char *str;
+ uint32_t str_len;
+ switch(key) {
+ case MP_ERROR_TYPE:
+ if (mp_typeof(**data) != MP_STR)
+ goto error;
+ str = mp_decode_str(data, &str_len);
+ mp_err.error_type = region_strdup(region, str, str_len);
+ if (mp_err.error_type == NULL)
+ goto finish;
+ break;
+ case MP_ERROR_FILE:
+ if (mp_typeof(**data) != MP_STR)
+ goto error;
+ str = mp_decode_str(data, &str_len);
+ mp_err.file = region_strdup(region, str, str_len);
+ if (mp_err.file == NULL)
+ goto finish;
+ break;
+ case MP_ERROR_LINE:
+ if (mp_typeof(**data) != MP_UINT)
+ goto error;
+ mp_err.line = mp_decode_uint(data);
+ break;
+ case MP_ERROR_REASON:
+ if (mp_typeof(**data) != MP_STR)
+ goto error;
+ str = mp_decode_str(data, &str_len);
+ mp_err.reason = region_strdup(region, str, str_len);
+ if (mp_err.reason == NULL)
+ goto finish;
+ break;
+ case MP_ERROR_ERRNO:
+ if (mp_typeof(**data) != MP_UINT)
+ goto error;
+ mp_err.saved_errno = mp_decode_uint(data);
+ break;
+ case MP_ERROR_FIELDS:
+ if (decode_additional_fields(data, region, &mp_err) != 0) {
+ goto error;
+ }
+ break;
+ default:
+ mp_next(data);
+ }
+ }
+
+ try {
+ err = build_error_xc(&mp_err);
+ } catch (OutOfMemory *e) {
+ diag_set_error(&fiber()->diag, e);
+ }
+finish:
+ region_truncate(region, region_svp);
+ return err;
+
+error:
+ diag_set(ClientError, ER_INVALID_MSGPACK,
+ "Invalid MP_ERROR MsgPack format");
+ goto finish;
+}
+
+void
+error_to_mpstream(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 += encode_error_size(it);
+ }
+
+ data_size += mp_sizeof_array(err_cnt);
+ uint32_t data_size_ext = mp_sizeof_ext(data_size);
+ char *ptr = mpstream_reserve(stream, data_size_ext);
+ char *data = ptr;
+ data = mp_encode_extl(data, MP_ERROR, data_size);
+ 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) {
+ encode_error(it, &data);
+ }
+
+ assert(data == ptr + data_size_ext);
+ mpstream_advance(stream, data_size_ext);
+}
+
+struct error *
+error_unpack(const char **data, uint32_t len)
+{
+ const char *end = *data + len;
+ struct error *err = NULL;
+
+ uint32_t map_size = mp_decode_map(data);
+ for (uint32_t i = 0; i < map_size; ++i) {
+ if (mp_typeof(**data) != MP_UINT) {
+ diag_set(ClientError, ER_INVALID_MSGPACK,
+ "Invalid MP_ERROR MsgPack format");
+ break;
+ }
+ uint64_t key = mp_decode_uint(data);
+ switch(key) {
+ case MP_ERROR_STACK:
+ {
+ uint32_t stack_sz = mp_decode_array(data);
+ struct error *effect = NULL;
+ for (uint32_t i = 0; i < stack_sz; i++) {
+ struct error *cur = decode_error(data);
+ if (cur == NULL)
+ return NULL;
+ if (err == NULL) {
+ err = cur;
+ effect = cur;
+ continue;
+ }
+
+ error_set_prev(effect, cur);
+ effect = cur;
+ }
+ break;
+ }
+ default:
+ mp_next(data);
+ }
+ }
+
+ (void)end;
+ assert(*data == end);
+ return err;
+}
diff --git a/src/box/mp_error.h b/src/box/mp_error.h
new file mode 100644
index 0000000..5c746d9
--- /dev/null
+++ b/src/box/mp_error.h
@@ -0,0 +1,60 @@
+#pragma once
+/*
+ * Copyright 2010-2020, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the
+ * following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+#include <stdint.h>
+
+struct mpstream;
+
+/**
+ * @brief Encode error to mpstream as MP_ERROR.
+ * @param error pointer to struct error for encoding.
+ * @param stream pointer to output stream.
+ */
+void
+error_to_mpstream(const struct error *error, struct mpstream *stream);
+
+/**
+ * @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);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
diff --git a/src/lib/core/mp_extension_types.h b/src/lib/core/mp_extension_types.h
index 7d42f21..e3ff9f5 100644
--- a/src/lib/core/mp_extension_types.h
+++ b/src/lib/core/mp_extension_types.h
@@ -43,6 +43,7 @@ enum mp_extension_type {
MP_UNKNOWN_EXTENSION = 0,
MP_DECIMAL = 1,
MP_UUID = 2,
+ MP_ERROR = 3,
mp_extension_type_MAX,
};
diff --git a/src/lua/error.c b/src/lua/error.c
index 18a990a..4a9ffc8 100644
--- a/src/lua/error.c
+++ b/src/lua/error.c
@@ -34,8 +34,6 @@
#include <fiber.h>
#include "utils.h"
-static int CTID_CONST_STRUCT_ERROR_REF = 0;
-
struct error *
luaL_iserror(struct lua_State *L, int narg)
{
diff --git a/src/lua/error.h b/src/lua/error.h
index 16cdaf7..4e4dc04 100644
--- a/src/lua/error.h
+++ b/src/lua/error.h
@@ -37,7 +37,6 @@
extern "C" {
#endif /* defined(__cplusplus) */
-
/** \cond public */
struct error;
@@ -62,6 +61,8 @@ void
luaT_pusherror(struct lua_State *L, struct error *e);
/** \endcond public */
+extern uint32_t CTID_CONST_STRUCT_ERROR_REF;
+
struct error *
luaL_iserror(struct lua_State *L, int narg);
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index b14361f..91560ef 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -195,6 +195,8 @@ restart: /* used by MP_EXT of unidentified subtype */
case MP_UUID:
mpstream_encode_uuid(stream, field->uuidval);
break;
+ case MP_ERROR:
+ return luamp_encode_extension(L, top, stream);
default:
/* Run trigger if type can't be encoded */
type = luamp_encode_extension(L, top, stream);
@@ -237,6 +239,7 @@ luamp_encode(struct lua_State *L, struct luaL_serializer *cfg,
return top_type;
}
+
void
luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
const char **data)
diff --git a/src/lua/utils.c b/src/lua/utils.c
index bd320f3..7d62c2d 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -37,6 +37,8 @@
#include <diag.h>
#include <fiber.h>
+#include "serializer_opts.h"
+
int luaL_nil_ref = LUA_REFNIL;
int luaL_map_metatable_ref = LUA_REFNIL;
int luaL_array_metatable_ref = LUA_REFNIL;
@@ -47,6 +49,7 @@ static uint32_t CTID_CHAR_PTR;
static uint32_t CTID_CONST_CHAR_PTR;
static uint32_t CTID_UUID;
uint32_t CTID_DECIMAL;
+uint32_t CTID_CONST_STRUCT_ERROR_REF;
void *
@@ -650,8 +653,6 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
struct serializer_opts *opts, int index,
struct luaL_field *field)
{
- /* opts will be used for encode MP_ERROR in the future */
- (void)opts;
if (index < 0)
index = lua_gettop(L) + index + 1;
@@ -759,6 +760,9 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
} else if (cd->ctypeid == CTID_UUID) {
field->ext_type = MP_UUID;
field->uuidval = (struct tt_uuid *) cdata;
+ } else if (cd->ctypeid == CTID_CONST_STRUCT_ERROR_REF &&
+ opts && opts->error_marshaling_enabled) {
+ field->ext_type = MP_ERROR;
} else {
field->ext_type = MP_UNKNOWN_EXTENSION;
}
diff --git a/test/box-tap/extended_error.test.lua b/test/box-tap/extended_error.test.lua
new file mode 100755
index 0000000..bd60a02
--- /dev/null
+++ b/test/box-tap/extended_error.test.lua
@@ -0,0 +1,136 @@
+#! /usr/bin/env tarantool
+
+local netbox = require('net.box')
+local os = require('os')
+local tap = require('tap')
+
+local test = tap.test('check an extended error')
+test:plan(4)
+
+function error_func()
+ box.error(box.error.PROC_LUA, "An old good error")
+end
+
+function custom_error_func()
+ box.error("My Custom Type", "A modern custom error")
+end
+
+function custom_error_func_2()
+ local err = box.error.new("My Custom Type", "A modern custom error")
+ return err
+end
+
+local function grant_func(user, name)
+ box.schema.func.create(name, {if_not_exists = true})
+ box.schema.user.grant(user, 'execute', 'function', name, {
+ if_not_exists = true
+ })
+end
+
+local function check_error(err, check_list)
+ if type(check_list) ~= 'table' then
+ return false
+ end
+
+ for k, v in pairs(check_list) do
+ if k == 'type' then
+ if err.base_type ~= v then
+ return false
+ end
+ elseif k == 'custom_type' then
+ if err.type ~= v then
+ return false
+ end
+ elseif k == 'message' then
+ if err.message ~= v then
+ return false
+ end
+ elseif k == 'trace' then
+ local trace = "File " .. err.trace[1]['file']
+ .. "\nLine " .. err.trace[1]['line']
+ if not string.match(trace, v) then
+ return false
+ end
+ elseif k == 'errno' then
+ if err.errno ~= v then
+ return false
+ end
+ elseif k == 'is_custom' then
+ if (err.base_type == 'CustomError') ~= v then
+ return false
+ end
+ else
+ return false
+ end
+ end
+
+ return true
+end
+
+local function test_old_transmission()
+ grant_func('guest', 'error_func')
+ grant_func('guest', 'custom_error_func_2')
+
+ local connection = netbox.connect(box.cfg.listen)
+ local _, err = pcall(connection.call, connection, 'error_func')
+ local err_2 = connection:call('custom_error_func_2')
+
+ local check_list = {
+ type = 'ClientError',
+ message = 'An old good error',
+ trace = '^File builtin/box/net_box.lua\nLine %d+$',
+ is_custom = false
+ }
+
+ local check_result = check_error(err, check_list)
+ local check_result_2 = type(err_2) == 'string' and
+ err_2 == 'A modern custom error'
+
+ test:ok(check_result, 'Check the old transmission type(IPROTO_ERROR)')
+ test:ok(check_result_2, 'Check the old transmission type(IPROTO_OK)')
+
+ connection:close()
+end
+
+local function test_extended_transmission()
+ grant_func('guest', 'custom_error_func')
+ grant_func('guest', 'custom_error_func_2')
+ box.schema.user.grant('guest','read,write', 'space', '_session_settings')
+
+ local connection = netbox.connect(box.cfg.listen)
+ connection.space._session_settings:update('error_marshaling_enabled',
+ {{'=', 2, true}})
+ local _, err = pcall(connection.call, connection, 'custom_error_func')
+ local err_2 = connection:call('custom_error_func_2')
+
+ local check_list = {
+ type = 'CustomError',
+ custom_type = 'My Custom Type',
+ message = 'A modern custom error',
+ trace = '^File builtin/box/net_box.lua\nLine %d+$',
+ is_custom = true
+ }
+
+ local check_list_2 = {
+ type = 'CustomError',
+ custom_type = 'My Custom Type',
+ message = 'A modern custom error',
+ trace = '.*extended_error.test.lua\nLine 19$',
+ is_custom = true
+ }
+ local check_result = check_error(err, check_list)
+ local check_result_2 = check_error(err_2, check_list_2)
+ test:ok(check_result, 'Check the extended transmission type(IPROTO_ERROR)')
+ test:ok(check_result_2, 'Check the extended transmission type(IPROTO_OK)')
+
+ connection:close()
+end
+
+box.cfg{
+ listen = os.getenv('LISTEN')
+}
+
+test_extended_transmission()
+test_old_transmission()
+
+os.exit(test:check() and 0 or 1)
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [Tarantool-patches] [PATCH V5 6/6] error: add error MsgPack encoding
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 6/6] error: add error MsgPack encoding Leonid Vasiliev
@ 2020-04-18 20:39 ` Vladislav Shpilevoy
0 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-18 20:39 UTC (permalink / raw)
To: Leonid Vasiliev, alexander.turenko; +Cc: tarantool-patches
Thanks for the patch!
On 18/04/2020 17:29, Leonid Vasiliev wrote:
> Co-authored-by: Vladislav Shpilevoy<v.shpilevoy@tarantool.org>
>
> Closes #4398
>
> @TarantoolBot document
> Title: msgpack format for MP_ERROR
>
> For sent an error over IProto MP_ERROR(0x03) subtype
> has been added to MsgPack MP_EXT.
> Now, if session setting 'error_marshaling_enabled'
> is true error will be encoded as:
> ```
> +--------+----------+========+
> | MP_EXT | MP_ERROR | MP_MAP |
> +--------+----------+========+
> ```
>
> MP_ERROR: <MP_MAP> {
> MP_ERROR_STACK: <MP_ARRAY> {
> <MP_MAP> {
> MP_ERROR_TYPE: <MP_STR>,
> MP_ERROR_FILE: <MP_STR>,
> MP_ERROR_LINE: <MP_UINT>,
> MP_ERROR_REASON: <MP_STR>,
> MP_ERROR_ERRNO: <MP_STR>,
> MP_ERROR_FIELDS: <MP_MAP> {
> <MP_STR>: ...,
> <MP_STR>: ...,
> ...
> },
> },
> }
> }
>
> MP_ERROR_STACK = 0x00
>
> At the map passes all necessary error details (depending
> from the error type) to create a copy of the error
> on the client side.
> The necessary fields:
> MP_ERROR_TYPE = 0x00 - error type(string).
> MP_ERROR_FILE = 0x01 - file name from trace.
> MP_ERROR_LINE = 0x02 - line of the file from trace.
> MP_ERROR_REASON = 0x03 - error message.
> MP_ERROR_ERRNO = 0x04 - saved errno.
> MP_ERROR_FIELDS = 0x05 - additional fields passes at MAP (MP_ERROR_FIELDS).
> Now additional fields may content:
> - ClientError code
> - Custom type
> - Access Denied object type
> - Access Denied object name
> - Access Denied access type
I rewrote the commit message, force pushed. Also I added
MP_ERROR_CODE to the root map. Because I realized a code is
available for every error via public box_error_code()
function in C.
Also I renamed REASON to MESSAGE. Reason is a legacy name
used only in Lua and only in error constructor. It is
visible as <error_object>.message after error creation.
====================
@TarantoolBot document
Title: Error objects encoding in MessagePack
Until now an error sent over IProto, or serialized into
MessagePack was turned into a string consisting of the error
message. As a result, all other error object attributes were lost,
including type of the object. On client side seeing a string it
was not possible to tell whether the string is a real string, or
it is a serialized error.
To deal with that the error objects encoding is reworked from the
scratch. Now, when session setting `error_marshaling_enabled` is
true, all fibers of that session will encode error objects as a
new MP_EXT type - MP_ERROR (0x03).
```
+--------+----------+========+
| MP_EXT | MP_ERROR | MP_MAP |
+--------+----------+========+
MP_ERROR: <MP_MAP> {
MP_ERROR_STACK: <MP_ARRAY> [
<MP_MAP> {
MP_ERROR_TYPE: <MP_STR>,
MP_ERROR_FILE: <MP_STR>,
MP_ERROR_LINE: <MP_UINT>,
MP_ERROR_MESSAGE: <MP_STR>,
MP_ERROR_ERRNO: <MP_UINT>,
MP_ERROR_CODE: <MP_UINT>,
MP_ERROR_FIELDS: <MP_MAP> {
<MP_STR>: ...,
<MP_STR>: ...,
...
},
...
},
...
]
}
```
On the top level there is a single key: `MP_ERROR_STACK = 0x00`.
More keys can be added in future, and a client should ignore all
unknown keys to keep compatibility with new versions.
Every error in the stack is a map with the following keys:
* `MP_ERROR_TYPE = 0x00` - error type. This is what is visible in
`<error_object>.base_type` field;
* `MP_ERROR_FILE = 0x01` - file name from `<error_object>.trace`;
* `MP_ERROR_LINE = 0x02` - line from `<error_object>.trace`;
* `MP_ERROR_MESSAGE = 0x03` - error message from
`<error_object>.message`;
* `MP_ERROR_ERRNO = 0x04` - errno saved at the moment of the error
creation. Visible in `<error_object>.errno`;
* `MP_ERROR_CODE = 0x05` - error code. Visible in
`<error_object>.code` and in C function `box_error_code()`.
* `MP_ERROR_FIELDS = 0x06` - additional fields depending on error
type. For example, AccessDenied error type stores here fields
`access_type`, `object_type`, `object_name`. Connector's code
should ignore unknown keys met here, and be ready, that for some
existing errors new fields can be added, old can be dropped.
====================
> ---
> src/box/CMakeLists.txt | 1 +
> src/box/lua/init.c | 56 ++++
> src/box/lua/tuple.c | 16 -
> src/box/mp_error.cc | 552 +++++++++++++++++++++++++++++++++++
> src/box/mp_error.h | 60 ++++
> src/lib/core/mp_extension_types.h | 1 +
> src/lua/error.c | 2 -
> src/lua/error.h | 3 +-
> src/lua/msgpack.c | 3 +
> src/lua/utils.c | 8 +-
> test/box-tap/extended_error.test.lua | 136 +++++++++
> 11 files changed, 817 insertions(+), 21 deletions(-)
> create mode 100644 src/box/mp_error.cc
> create mode 100644 src/box/mp_error.h
> create mode 100755 test/box-tap/extended_error.test.lua
>
> diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
> index 6688303..81f6400 100644
> --- a/src/box/CMakeLists.txt
> +++ b/src/box/CMakeLists.txt
> @@ -136,6 +136,7 @@ add_library(box STATIC
> wal.c
> call.c
> merger.c
> + mp_error.cc
I moved it to box_error library.
====================
diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 81f64001a..c82ceb56c 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -26,7 +26,7 @@ set_property(DIRECTORY PROPERTY ADDITIONAL_MAKE_CLEAN_FILES ${lua_sources})
include_directories(${ZSTD_INCLUDE_DIRS})
include_directories(${CMAKE_BINARY_DIR}/src/box/sql)
-add_library(box_error STATIC error.cc errcode.c vclock.c)
+add_library(box_error STATIC error.cc errcode.c vclock.c mp_error.cc)
-target_link_libraries(box_error core stat)
+target_link_libraries(box_error core stat mpstream)
add_library(vclock STATIC vclock.c)
target_link_libraries(vclock core)
add_library(xrow STATIC xrow.c iproto_constants.c)
target_link_libraries(xrow server core small vclock misc box_error
- scramble mpstream ${MSGPUCK_LIBRARIES})
+ scramble ${MSGPUCK_LIBRARIES})
@@ -136,7 +136,6 @@ add_library(box STATIC
wal.c
call.c
merger.c
- mp_error.cc
${lua_sources}
lua/init.c
lua/call.c
====================
I reviewed rest of the code and it looks mostly fine except
things I mentioned above and which I fixed silently. See my diff
below. It is already applied and force pushed.
====================
diff --git a/src/box/mp_error.cc b/src/box/mp_error.cc
index 348b7a70b..86cb8dc23 100644
--- a/src/box/mp_error.cc
+++ b/src/box/mp_error.cc
@@ -39,20 +39,23 @@
* MP_ERROR format:
*
* MP_ERROR: <MP_MAP> {
- * MP_ERROR_STACK: <MP_ARRAY> {
+ * MP_ERROR_STACK: <MP_ARRAY> [
* <MP_MAP> {
- * MP_ERROR_TYPE: <MP_STR>,
- * MP_ERROR_FILE: <MP_STR>,
- * MP_ERROR_LINE: <MP_UINT>,
- * MP_ERROR_REASON: <MP_STR>,
- * MP_ERROR_ERRNO: <MP_STR>,
+ * MP_ERROR_TYPE: <MP_STR>,
+ * MP_ERROR_FILE: <MP_STR>,
+ * MP_ERROR_LINE: <MP_UINT>,
+ * MP_ERROR_MESSAGE: <MP_STR>,
+ * MP_ERROR_ERRNO: <MP_UINT>,
+ * MP_ERROR_CODE: <MP_UINT>,
* MP_ERROR_FIELDS: <MP_MAP> {
* <MP_STR>: ...,
* <MP_STR>: ...,
* ...
* },
+ * ...
* },
- * }
+ * ...
+ * ]
* }
*/
@@ -64,24 +67,26 @@ enum {
};
/**
- * Keys used for error encode/decode to MP_ERROR.
+ * Keys of individual error in the stack.
*/
enum {
- /* Error type */
+ /** Error type. */
MP_ERROR_TYPE = 0x00,
- /* File name from trace */
+ /** File name from trace. */
MP_ERROR_FILE = 0x01,
- /* Line from trace */
+ /** Line from trace. */
MP_ERROR_LINE = 0x02,
- /* Error message */
- MP_ERROR_REASON = 0x03,
- /* Saved errno */
+ /** Error message. */
+ MP_ERROR_MESSAGE = 0x03,
+ /** Errno at the moment of error creation. */
MP_ERROR_ERRNO = 0x04,
+ /** Error code. */
+ MP_ERROR_CODE = 0x05,
/*
- * Key uses for error type-specific fields which
- * presents here as map: string key - value.
+ * Type-specific fields stored as a map
+ * {string key = value}.
*/
- MP_ERROR_FIELDS = 0x05
+ MP_ERROR_FIELDS = 0x06
};
/**
@@ -89,15 +94,15 @@ enum {
* during decoding MP_ERROR.
*/
struct mp_error {
- uint32_t error_code;
+ uint32_t code;
uint32_t line;
uint32_t saved_errno;
- const char *error_type;
+ const char *type;
const char *file;
- const char *reason;
+ const char *message;
const char *custom_type;
- const char *ad_obj_type;
- const char *ad_obj_name;
+ const char *ad_object_type;
+ const char *ad_object_name;
const char *ad_access_type;
};
@@ -110,21 +115,18 @@ mp_error_create(struct mp_error *mp_error)
static uint32_t
encode_error_size(const struct error *error)
{
- uint32_t errcode = 0;
+ uint32_t errcode = box_error_code(error);
- bool is_client = false;
bool is_custom = false;
bool is_access_denied = false;
- if (strcmp(error->type->name, "ClientError") == 0) {
- is_client = true;
- } else if (strcmp(error->type->name, "CustomError") == 0) {
+ if (strcmp(error->type->name, "CustomError") == 0) {
is_custom = true;
} else if (strcmp(error->type->name, "AccessDeniedError") == 0) {
is_access_denied = true;
}
- uint32_t details_num = 5;
+ uint32_t details_num = 6;
uint32_t data_size = 0;
data_size += mp_sizeof_uint(MP_ERROR_TYPE);
@@ -133,40 +135,29 @@ encode_error_size(const struct error *error)
data_size += mp_sizeof_uint(error->line);
data_size += mp_sizeof_uint(MP_ERROR_FILE);
data_size += mp_sizeof_str(strlen(error->file));
- data_size += mp_sizeof_uint(MP_ERROR_REASON);
+ data_size += mp_sizeof_uint(MP_ERROR_MESSAGE);
data_size += mp_sizeof_str(strlen(error->errmsg));
data_size += mp_sizeof_uint(MP_ERROR_ERRNO);
data_size += mp_sizeof_uint(error->saved_errno);
+ data_size += mp_sizeof_uint(MP_ERROR_CODE);
+ data_size += mp_sizeof_uint(errcode);
- if (is_client) {
+ if (is_access_denied) {
++details_num;
data_size += mp_sizeof_uint(MP_ERROR_FIELDS);
- errcode = box_error_code(error);
- data_size += mp_sizeof_map(1);
- data_size += mp_sizeof_str(strlen("code"));
- data_size += mp_sizeof_uint(errcode);
- } else if (is_access_denied) {
- ++details_num;
- data_size += mp_sizeof_uint(MP_ERROR_FIELDS);
- errcode = box_error_code(error);
- data_size += mp_sizeof_map(4);
- data_size += mp_sizeof_str(strlen("code"));
- data_size += mp_sizeof_uint(errcode);
+ data_size += mp_sizeof_map(3);
AccessDeniedError *ad_err = type_cast(AccessDeniedError, error);
- data_size += mp_sizeof_str(strlen("AD_OBJ_TYPE"));
+ data_size += mp_sizeof_str(strlen("object_type"));
data_size += mp_sizeof_str(strlen(ad_err->object_type()));
- data_size += mp_sizeof_str(strlen("AD_OBJ_NAME"));
+ data_size += mp_sizeof_str(strlen("object_name"));
data_size += mp_sizeof_str(strlen(ad_err->object_name()));
- data_size += mp_sizeof_str(strlen("AD_ACCESS_TYPE"));
+ data_size += mp_sizeof_str(strlen("access_type"));
data_size += mp_sizeof_str(strlen(ad_err->access_type()));
} else if (is_custom) {
++details_num;
data_size += mp_sizeof_uint(MP_ERROR_FIELDS);
- errcode = box_error_code(error);
- data_size += mp_sizeof_map(2);
- data_size += mp_sizeof_str(strlen("code"));
- data_size += mp_sizeof_uint(errcode);
- data_size += mp_sizeof_str(strlen("custom"));
+ data_size += mp_sizeof_map(1);
+ data_size += mp_sizeof_str(strlen("custom_type"));
data_size +=
mp_sizeof_str(strlen(box_error_custom_type(error)));
}
@@ -179,23 +170,19 @@ encode_error_size(const struct error *error)
static void
encode_error(const struct error *error, char **data)
{
- uint32_t errcode = 0;
+ uint32_t errcode = box_error_code(error);
- bool is_client = false;
bool is_custom = false;
bool is_access_denied = false;
- if (strcmp(error->type->name, "ClientError") == 0) {
- is_client = true;
- } else if (strcmp(error->type->name, "CustomError") == 0) {
+ if (strcmp(error->type->name, "CustomError") == 0) {
is_custom = true;
} else if (strcmp(error->type->name, "AccessDeniedError") == 0) {
is_access_denied = true;
}
- uint32_t details_num = 5;
-
- if (is_client || is_access_denied || is_custom)
+ uint32_t details_num = 6;
+ if (is_access_denied || is_custom)
++details_num;
*data = mp_encode_map(*data, details_num);
@@ -206,43 +193,34 @@ encode_error(const struct error *error, char **data)
*data = mp_encode_uint(*data, error->line);
*data = mp_encode_uint(*data, MP_ERROR_FILE);
*data = mp_encode_str(*data, error->file, strlen(error->file));
- *data = mp_encode_uint(*data, MP_ERROR_REASON);
+ *data = mp_encode_uint(*data, MP_ERROR_MESSAGE);
*data = mp_encode_str(*data, error->errmsg, strlen(error->errmsg));
*data = mp_encode_uint(*data, MP_ERROR_ERRNO);
*data = mp_encode_uint(*data, error->saved_errno);
+ *data = mp_encode_uint(*data, MP_ERROR_CODE);
+ *data = mp_encode_uint(*data, errcode);
- if (is_client) {
- *data = mp_encode_uint(*data, MP_ERROR_FIELDS);
- errcode = box_error_code(error);
- *data = mp_encode_map(*data, 1);
- *data = mp_encode_str(*data, "code",strlen("code"));
- *data = mp_encode_uint(*data, errcode);
- } else if (is_access_denied) {
+ if (is_access_denied) {
*data = mp_encode_uint(*data, MP_ERROR_FIELDS);
- errcode = box_error_code(error);
- *data = mp_encode_map(*data, 4);
- *data = mp_encode_str(*data, "code",strlen("code"));
- *data = mp_encode_uint(*data, errcode);
+ *data = mp_encode_map(*data, 3);
AccessDeniedError *ad_err = type_cast(AccessDeniedError, error);
- *data = mp_encode_str(*data, "AD_OBJ_TYPE",
- strlen("AD_OBJ_TYPE"));
+ *data = mp_encode_str(*data, "object_type",
+ strlen("object_type"));
*data = mp_encode_str(*data, ad_err->object_type(),
strlen(ad_err->object_type()));
- *data = mp_encode_str(*data, "AD_OBJ_NAME",
- strlen("AD_OBJ_NAME"));
+ *data = mp_encode_str(*data, "object_name",
+ strlen("object_name"));
*data = mp_encode_str(*data, ad_err->object_name(),
strlen(ad_err->object_name()));
- *data = mp_encode_str(*data, "AD_ACCESS_TYPE",
- strlen("AD_ACCESS_TYPE"));
+ *data = mp_encode_str(*data, "access_type",
+ strlen("access_type"));
*data = mp_encode_str(*data, ad_err->access_type(),
strlen(ad_err->access_type()));
} else if (is_custom) {
*data = mp_encode_uint(*data, MP_ERROR_FIELDS);
- errcode = box_error_code(error);
- *data = mp_encode_map(*data, 2);
- *data = mp_encode_str(*data, "code",strlen("code"));
- *data = mp_encode_uint(*data, errcode);
- *data = mp_encode_str(*data, "custom", strlen("custom"));
+ *data = mp_encode_map(*data, 1);
+ *data = mp_encode_str(*data, "custom_type",
+ strlen("custom_type"));
const char *custom = box_error_custom_type(error);
*data = mp_encode_str(*data, custom, strlen(custom));
}
@@ -252,75 +230,69 @@ static struct error *
build_error_xc(struct mp_error *mp_error)
{
/*
- * To create an error the "raw" constructor using
+ * To create an error the "raw" constructor is used
* because OOM error must be thrown in OOM case.
* Builders returns a pointer to the static OOM error
* in OOM case.
*/
struct error *err = NULL;
- if (strcmp(mp_error->error_type, "ClientError") == 0) {
+ if (strcmp(mp_error->type, "ClientError") == 0) {
ClientError *e = new ClientError(mp_error->file, mp_error->line,
ER_UNKNOWN);
- e->m_errcode = mp_error->error_code;
+ e->m_errcode = mp_error->code;
err = e;
- } else if (strcmp(mp_error->error_type, "CustomError") == 0) {
+ } else if (strcmp(mp_error->type, "CustomError") == 0) {
err = new CustomError(mp_error->file, mp_error->line,
- mp_error->custom_type,
- mp_error->error_code);
- } else if (strcmp(mp_error->error_type, "AccessDeniedError") == 0) {
+ mp_error->custom_type, mp_error->code);
+ } else if (strcmp(mp_error->type, "AccessDeniedError") == 0) {
err = new AccessDeniedError(mp_error->file, mp_error->line,
mp_error->ad_access_type,
- mp_error->ad_obj_type,
- mp_error->ad_obj_name, "", false);
- } else if (strcmp(mp_error->error_type, "XlogError") == 0) {
+ mp_error->ad_object_type,
+ mp_error->ad_object_name, "",
+ false);
+ } else if (strcmp(mp_error->type, "XlogError") == 0) {
err = new XlogError(&type_XlogError, mp_error->file,
mp_error->line);
- error_format_msg(err, "%s", mp_error->reason);
- } else if (strcmp(mp_error->error_type, "XlogGapError") == 0) {
+ } else if (strcmp(mp_error->type, "XlogGapError") == 0) {
err = new XlogGapError(mp_error->file, mp_error->line,
- mp_error->reason);
- } else if (strcmp(mp_error->error_type, "SystemError") == 0) {
+ mp_error->message);
+ } else if (strcmp(mp_error->type, "SystemError") == 0) {
err = new SystemError(mp_error->file, mp_error->line,
- "%s", mp_error->reason);
- } else if (strcmp(mp_error->error_type, "SocketError") == 0) {
+ "%s", mp_error->message);
+ } else if (strcmp(mp_error->type, "SocketError") == 0) {
err = new SocketError(mp_error->file, mp_error->line, "", "");
- error_format_msg(err, "%s", mp_error->reason);
- } else if (strcmp(mp_error->error_type, "OutOfMemory") == 0) {
+ error_format_msg(err, "%s", mp_error->message);
+ } else if (strcmp(mp_error->type, "OutOfMemory") == 0) {
err = new OutOfMemory(mp_error->file, mp_error->line,
0, "", "");
- error_format_msg(err, "%s", mp_error->reason);
- } else if (strcmp(mp_error->error_type, "TimedOut") == 0) {
+ } else if (strcmp(mp_error->type, "TimedOut") == 0) {
err = new TimedOut(mp_error->file, mp_error->line);
- } else if (strcmp(mp_error->error_type, "ChannelIsClosed") == 0) {
+ } else if (strcmp(mp_error->type, "ChannelIsClosed") == 0) {
err = new ChannelIsClosed(mp_error->file, mp_error->line);
- } else if (strcmp(mp_error->error_type, "FiberIsCancelled") == 0) {
+ } else if (strcmp(mp_error->type, "FiberIsCancelled") == 0) {
err = new FiberIsCancelled(mp_error->file, mp_error->line);
- } else if (strcmp(mp_error->error_type, "LuajitError") == 0) {
+ } else if (strcmp(mp_error->type, "LuajitError") == 0) {
err = new LuajitError(mp_error->file, mp_error->line,
- mp_error->reason);
- } else if (strcmp(mp_error->error_type, "IllegalParams") == 0) {
+ mp_error->message);
+ } else if (strcmp(mp_error->type, "IllegalParams") == 0) {
err = new IllegalParams(mp_error->file, mp_error->line,
- "%s", mp_error->reason);
- } else if (strcmp(mp_error->error_type, "CollationError") == 0) {
+ "%s", mp_error->message);
+ } else if (strcmp(mp_error->type, "CollationError") == 0) {
err = new CollationError(mp_error->file, mp_error->line,
- "%s", mp_error->reason);
- } else if (strcmp(mp_error->error_type, "SwimError") == 0) {
+ "%s", mp_error->message);
+ } else if (strcmp(mp_error->type, "SwimError") == 0) {
err = new SwimError(mp_error->file, mp_error->line,
- "%s", mp_error->reason);
- } else if (strcmp(mp_error->error_type, "CryptoError") == 0) {
+ "%s", mp_error->message);
+ } else if (strcmp(mp_error->type, "CryptoError") == 0) {
err = new CryptoError(mp_error->file, mp_error->line,
- "%s", mp_error->reason);
+ "%s", mp_error->message);
} else {
err = new ClientError(mp_error->file, mp_error->line,
ER_UNKNOWN);
}
-
- if (err) {
- err->saved_errno = mp_error->saved_errno;
- error_format_msg(err, "%s", mp_error->reason);
- }
-
+ err->saved_errno = mp_error->saved_errno;
+ error_format_msg(err, "%s", mp_error->message);
return err;
}
@@ -337,6 +309,25 @@ region_strdup(struct region *region, const char *str, uint32_t len)
return res;
}
+static inline const char *
+decode_and_copy_str(struct region *region, const char **data)
+{
+ if (mp_typeof(**data) != MP_STR) {
+ diag_set(ClientError, ER_INVALID_MSGPACK,
+ "Invalid MP_ERROR MsgPack format");
+ return NULL;
+ }
+ uint32_t str_len;
+ const char *str = mp_decode_str(data, &str_len);
+ return region_strdup(region, str, str_len);;
+}
+
+static inline bool
+str_nonterm_is_eq(const char *l, const char *r, uint32_t r_len)
+{
+ return r_len == strlen(l) && memcmp(l, r, r_len) == 0;
+}
+
static int
decode_additional_fields(const char **data, struct region *region,
struct mp_error *mp_err)
@@ -344,47 +335,28 @@ decode_additional_fields(const char **data, struct region *region,
uint32_t map_sz = mp_decode_map(data);
const char *key;
uint32_t key_len;
- const char *str;
- uint32_t str_len;
for (uint32_t i = 0; i < map_sz; ++i) {
if (mp_typeof(**data) != MP_STR)
return -1;
key = mp_decode_str(data, &key_len);
-
- if (strncmp(key, "code", key_len) == 0) {
- if (mp_typeof(**data) != MP_UINT)
- return -1;
- mp_err->error_code = mp_decode_uint(data);
- } else if (strncmp(key, "AD_OBJ_TYPE", key_len) == 0) {
- if (mp_typeof(**data) != MP_STR)
+ if (str_nonterm_is_eq("object_type", key, key_len)) {
+ mp_err->ad_object_type =
+ decode_and_copy_str(region, data);
+ if (mp_err->ad_object_type == NULL)
return -1;
- str = mp_decode_str(data, &str_len);
- mp_err->ad_obj_type = region_strdup(region, str,
- str_len);
- if (mp_err->ad_obj_type == NULL)
+ } else if (str_nonterm_is_eq("object_name", key, key_len)) {
+ mp_err->ad_object_name =
+ decode_and_copy_str(region, data);
+ if (mp_err->ad_object_name == NULL)
return -1;
- } else if (strncmp(key, "AD_OBJ_NAME", key_len) == 0) {
- if (mp_typeof(**data) != MP_STR)
- return -1;
- str = mp_decode_str(data, &str_len);
- mp_err->ad_obj_name = region_strdup(region, str,
- str_len);
- if (mp_err->ad_obj_name == NULL)
- return -1;
- } else if (strncmp(key, "AD_ACCESS_TYPE", key_len) == 0) {
- if (mp_typeof(**data) != MP_STR)
- return -1;
- str = mp_decode_str(data, &str_len);
- mp_err->ad_access_type = region_strdup(region, str,
- str_len);
+ } else if (str_nonterm_is_eq("access_type", key, key_len)) {
+ mp_err->ad_access_type =
+ decode_and_copy_str(region, data);
if (mp_err->ad_access_type == NULL)
return -1;
- } else if (strncmp(key, "custom", key_len) == 0) {
- if (mp_typeof(**data) != MP_STR)
- return -1;
- str = mp_decode_str(data, &str_len);
- mp_err->custom_type = region_strdup(region, str,
- str_len);
+ } else if (str_nonterm_is_eq("custom_type", key, key_len)) {
+ mp_err->custom_type =
+ decode_and_copy_str(region, data);
if (mp_err->custom_type == NULL)
return -1;
} else {
@@ -397,43 +369,30 @@ decode_additional_fields(const char **data, struct region *region,
static struct error *
decode_error(const char **data)
{
- if (mp_typeof(**data) != MP_MAP) {
- diag_set(ClientError, ER_INVALID_MSGPACK,
- "Invalid MP_ERROR format");
- return NULL;
- }
-
struct mp_error mp_err;
mp_error_create(&mp_err);
struct region *region = &fiber()->gc;
uint32_t region_svp = region_used(region);
- uint32_t map_size = mp_decode_map(data);
-
struct error *err = NULL;
+ uint32_t map_size;
+
+ if (mp_typeof(**data) != MP_MAP)
+ goto error;
+
+ map_size = mp_decode_map(data);
for (uint32_t i = 0; i < map_size; ++i) {
- if (mp_typeof(**data) != MP_UINT) {
- diag_set(ClientError, ER_INVALID_MSGPACK,
- "Invalid MP_ERROR MsgPack format");
- goto finish;
- }
+ if (mp_typeof(**data) != MP_UINT)
+ goto error;
uint64_t key = mp_decode_uint(data);
- const char *str;
- uint32_t str_len;
switch(key) {
case MP_ERROR_TYPE:
- if (mp_typeof(**data) != MP_STR)
- goto error;
- str = mp_decode_str(data, &str_len);
- mp_err.error_type = region_strdup(region, str, str_len);
- if (mp_err.error_type == NULL)
+ mp_err.type = decode_and_copy_str(region, data);
+ if (mp_err.type == NULL)
goto finish;
break;
case MP_ERROR_FILE:
- if (mp_typeof(**data) != MP_STR)
- goto error;
- str = mp_decode_str(data, &str_len);
- mp_err.file = region_strdup(region, str, str_len);
+ mp_err.file = decode_and_copy_str(region, data);
if (mp_err.file == NULL)
goto finish;
break;
@@ -442,12 +401,9 @@ decode_error(const char **data)
goto error;
mp_err.line = mp_decode_uint(data);
break;
- case MP_ERROR_REASON:
- if (mp_typeof(**data) != MP_STR)
- goto error;
- str = mp_decode_str(data, &str_len);
- mp_err.reason = region_strdup(region, str, str_len);
- if (mp_err.reason == NULL)
+ case MP_ERROR_MESSAGE:
+ mp_err.message = decode_and_copy_str(region, data);
+ if (mp_err.message == NULL)
goto finish;
break;
case MP_ERROR_ERRNO:
@@ -455,10 +411,15 @@ decode_error(const char **data)
goto error;
mp_err.saved_errno = mp_decode_uint(data);
break;
- case MP_ERROR_FIELDS:
- if (decode_additional_fields(data, region, &mp_err) != 0) {
+ case MP_ERROR_CODE:
+ if (mp_typeof(**data) != MP_UINT)
goto error;
- }
+ mp_err.code = mp_decode_uint(data);
+ break;
+ case MP_ERROR_FIELDS:
+ if (decode_additional_fields(data, region,
+ &mp_err) != 0)
+ goto finish;
break;
default:
mp_next(data);
@@ -468,7 +429,7 @@ decode_error(const char **data)
try {
err = build_error_xc(&mp_err);
} catch (OutOfMemory *e) {
- diag_set_error(&fiber()->diag, e);
+ assert(err == NULL && !diag_is_empty(diag_get()));
}
finish:
region_truncate(region, region_svp);
@@ -522,8 +483,7 @@ error_unpack(const char **data, uint32_t len)
}
uint64_t key = mp_decode_uint(data);
switch(key) {
- case MP_ERROR_STACK:
- {
+ case MP_ERROR_STACK: {
uint32_t stack_sz = mp_decode_array(data);
struct error *effect = NULL;
for (uint32_t i = 0; i < stack_sz; i++) {
diff --git a/src/lua/error.h b/src/lua/error.h
index 4e4dc048b..efaaa63fd 100644
--- a/src/lua/error.h
+++ b/src/lua/error.h
@@ -37,6 +37,7 @@
extern "C" {
#endif /* defined(__cplusplus) */
+
/** \cond public */
struct error;
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index 91560eff0..f2ae2d8c3 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -239,7 +239,6 @@ luamp_encode(struct lua_State *L, struct luaL_serializer *cfg,
return top_type;
}
-
void
luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
const char **data)
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Tarantool-patches] [PATCH V5 5.5/6] box: move Lua MP_EXT decoder from tuple.c
2020-04-18 15:29 [Tarantool-patches] [PATCH V5 0/6] Extending error functionality Leonid Vasiliev
` (5 preceding siblings ...)
2020-04-18 15:29 ` [Tarantool-patches] [PATCH V5 6/6] error: add error MsgPack encoding Leonid Vasiliev
@ 2020-04-18 21:14 ` Vladislav Shpilevoy
6 siblings, 0 replies; 13+ messages in thread
From: Vladislav Shpilevoy @ 2020-04-18 21:14 UTC (permalink / raw)
To: Leonid Vasiliev, alexander.turenko; +Cc: tarantool-patches
Lua C module 'msgpack' supports registration of custom extension
decoders for MP_EXT values. That is needed to make 'msgpack' not
depending on any modules which use it.
So far the only extension were tuples - struct tuple cdata needed
to be encoded as an array.
That is going to change in next commits, where struct error cdata
appears. So the decoder can't be located in src/box/lua/tuple.c.
It is moved to a more common place - src/box/lua/init.c.
Needed for #4398
---
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 63e8b8216..b05ff90be 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -36,6 +36,7 @@
#include "lua/utils.h" /* luaT_error() */
#include "lua/trigger.h"
+#include "lua/msgpack.h"
#include "box/box.h"
#include "box/txn.h"
@@ -386,6 +387,21 @@ static const struct luaL_Reg boxlib_backup[] = {
{NULL, NULL}
};
+/**
+ * A MsgPack extensions handler, for types defined in box.
+ */
+static enum mp_type
+luamp_encode_extension_box(struct lua_State *L, int idx,
+ struct mpstream *stream)
+{
+ struct tuple *tuple = luaT_istuple(L, idx);
+ if (tuple != NULL) {
+ tuple_to_mpstream(tuple, stream);
+ return MP_ARRAY;
+ }
+ return MP_EXT;
+}
+
#include "say.h"
void
@@ -426,6 +442,8 @@ box_lua_init(struct lua_State *L)
luaopen_merger(L);
lua_pop(L, 1);
+ luamp_set_encode_extension(luamp_encode_extension_box);
+
/* Load Lua extension */
for (const char **s = lua_sources; *s; s += 2) {
const char *modname = *s;
diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index aba906d1f..03b4b8a2e 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -290,20 +290,6 @@ tuple_to_mpstream(struct tuple *tuple, struct mpstream *stream)
mpstream_advance(stream, bsize);
}
-/* A MsgPack extensions handler that supports tuples */
-static enum mp_type
-luamp_encode_extension_box(struct lua_State *L, int idx,
- struct mpstream *stream)
-{
- struct tuple *tuple = luaT_istuple(L, idx);
- if (tuple != NULL) {
- tuple_to_mpstream(tuple, stream);
- return MP_ARRAY;
- }
-
- return MP_EXT;
-}
-
/**
* Convert a tuple into lua table. Named fields are stored as
* {name = value} pairs. Not named fields are stored as
@@ -582,8 +568,6 @@ box_lua_tuple_init(struct lua_State *L)
luaL_register_module(L, tuplelib_name, lbox_tuplelib);
lua_pop(L, 1);
- luamp_set_encode_extension(luamp_encode_extension_box);
-
tuple_serializer_update_options();
trigger_create(&tuple_serializer.update_trigger,
on_msgpack_serializer_update, NULL, NULL);
^ permalink raw reply [flat|nested] 13+ messages in thread