[Tarantool-patches] [PATCH V4 1/6] error: add custom error type
Leonid Vasiliev
lvasiliev at tarantool.org
Thu Apr 16 20:38:40 MSK 2020
Co-authored-by: Vladislav Shpilevoy<v.shpilevoy at 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 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
...
```
---
extra/exports | 1 +
src/box/errcode.h | 1 +
src/box/error.cc | 92 +++++++++++++++++++++++++++++++++----------
src/box/error.h | 36 +++++++++++++++--
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 | 66 +++++++++++++++++++++++++++++--
test/box/error.test.lua | 15 +++++++
test/engine/func_index.result | 14 ++++---
11 files changed, 240 insertions(+), 48 deletions(-)
diff --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/errcode.h b/src/box/errcode.h
index d1e4d02..e37b7bd 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -266,6 +266,7 @@ struct errcode_record {
/*211 */_(ER_WRONG_QUERY_ID, "Prepared statement with id %u does not exist") \
/*212 */_(ER_SEQUENCE_NOT_STARTED, "Sequence '%s' is not started") \
/*213 */_(ER_NO_SUCH_SESSION_SETTING, "Session setting %s doesn't exist") \
+ /*214 */_(ER_CUSTOM_ERROR, "%s") \
/*
* !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/error.cc b/src/box/error.cc
index 233b312..c8fadfb 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -86,35 +86,50 @@ 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);
+ 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 +140,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 +315,30 @@ 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)
+ :ClientError(&type_CustomError, file, line, ER_CUSTOM_ERROR)
+{
+ error_format_msg(this, tnt_errcode_desc(m_errcode), custom_type);
+ strncpy(m_custom_type, custom_type, sizeof(m_custom_type) - 1);
+ m_custom_type[sizeof(m_custom_type) - 1] = '\0';
+}
+
+struct error *
+BuildCustomError(const char *file, unsigned int line, const char *custom_type)
+{
+ try {
+ return new CustomError(file, line, custom_type);
+ } catch (OutOfMemory *e) {
+ return e;
+ }
+}
diff --git a/src/box/error.h b/src/box/error.h
index ca5d5b2..311d543 100644
--- a/src/box/error.h
+++ b/src/box/error.h
@@ -53,6 +53,9 @@ 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);
+
/** \cond public */
struct error;
@@ -138,11 +141,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 +164,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 +304,22 @@ 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);
+
+ 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..68703d8 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,13 +60,21 @@ 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);
+ 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);
+ } else {
+ code = ER_CUSTOM_ERROR;
+ custom_type = lua_tostring(L, top_base);
+ }
reason = tnt_errcode_desc(code);
if (top > top_base) {
/* Call string.format(reason, ...) to format message */
@@ -78,14 +93,19 @@ 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);
+ else
+ code = ER_CUSTOM_ERROR;
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 +122,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 +169,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..82db453 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)'
| ...
--
@@ -431,6 +432,7 @@ t;
| 211: box.error.WRONG_QUERY_ID
| 212: box.error.SEQUENCE_NOT_STARTED
| 213: box.error.NO_SUCH_SESSION_SETTING
+ | 214: box.error.CUSTOM_ERROR
| ...
test_run:cmd("setopt delimiter ''");
@@ -489,7 +491,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 +833,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: 214
+ | 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: 214
+ | 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: 214
+ | 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
More information about the Tarantool-patches
mailing list