Tarantool development patches archive
 help / color / mirror / Atom feed
From: Leonid Vasiliev <lvasiliev@tarantool.org>
To: v.shpilevoy@tarantool.org
Cc: tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH V3 2/7] error: add custom error type
Date: Wed, 15 Apr 2020 12:31:57 +0300	[thread overview]
Message-ID: <f78f059fc08baf77053c9014aa573dccd9e4067a.1586934134.git.lvasiliev@tarantool.org> (raw)
In-Reply-To: <cover.1586934134.git.lvasiliev@tarantool.org>
In-Reply-To: <cover.1586934134.git.lvasiliev@tarantool.org>

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 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              | 93 +++++++++++++++++++++++++++++++++----------
 src/box/error.h               | 34 +++++++++++++++-
 src/box/lua/error.cc          | 44 ++++++++++++++------
 src/box/xrow.c                |  2 +-
 src/lua/error.lua             | 14 ++++++-
 test/app/fiber.result         |  7 ++--
 test/box/error.result         | 69 +++++++++++++++++++++++++++++---
 test/box/error.test.lua       | 16 ++++++++
 test/engine/func_index.result | 14 ++++---
 11 files changed, 245 insertions(+), 50 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..d854b86 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -86,35 +86,51 @@ 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) {
+			custom_error->m_errcode = code;
+			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 +141,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 +316,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..540008f 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,7 +164,7 @@ 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
@@ -159,11 +172,12 @@ box_error_add(const char *file, unsigned line, uint32_t code,
  */
 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 a2facf0..2279d63 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,6 +60,7 @@ 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;
 	bool is_traceback_enabled = false;
 	bool is_traceback_specified = false;
@@ -60,8 +68,15 @@ luaT_error_create(lua_State *L, int top_base)
 	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 */
@@ -80,15 +95,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_pop(L, 1);
+		lua_getfield(L, top_base, "type");
+		if (!lua_isnil(L, -1))
+			custom_type = lua_tostring(L, -1);
 		lua_getfield(L, top_base, "traceback");
 		if (lua_isboolean(L, -1)) {
 			is_traceback_enabled = lua_toboolean(L, -1);
@@ -111,7 +130,8 @@ raise:
 		line = info.currentline;
 	}
 
-	struct error *err = box_error_new(file, line, code, "%s", reason);
+	struct error *err = box_error_new(file, line, code, custom_type,
+					  "%s", reason);
 	/*
 	 * Explicit traceback option overrides the global setting.
 	 */
@@ -164,11 +184,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 93fd1b9..80df657 100644
--- a/src/lua/error.lua
+++ b/src/lua/error.lua
@@ -37,6 +37,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 = {}
@@ -77,10 +80,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
@@ -138,6 +149,7 @@ local error_fields = {
     ["errno"]       = error_errno;
     ["prev"]        = error_prev;
     ["traceback"]   = error_traceback;
+    ["base_type"]   = error_base_type,
 }
 
 local function error_unpack(err)
diff --git a/test/app/fiber.result b/test/app/fiber.result
index debfc67..fa7b20a 100644
--- a/test/app/fiber.result
+++ b/test/app/fiber.result
@@ -1038,12 +1038,13 @@ st;
 ...
 e:unpack();
 ---
-- type: ClientError
-  code: 1
-  message: Illegal parameters, oh my
+- code: 1
   trace:
   - file: '[string "function err() box.error(box.error.ILLEGAL_PA..."]'
     line: 1
+  type: ClientError
+  message: Illegal parameters, oh my
+  base_type: ClientError
 ...
 flag = false;
 ---
diff --git a/test/box/error.result b/test/box/error.result
index 2502d88..b717a4f 100644
--- a/test/box/error.result
+++ b/test/box/error.result
@@ -34,12 +34,13 @@ e
  | ...
 e:unpack()
  | ---
- | - type: ClientError
- |   code: 1
- |   message: Illegal parameters, bla bla
+ | - code: 1
  |   trace:
  |   - file: '[C]'
  |     line: 4294967295
+ |   type: ClientError
+ |   message: Illegal parameters, bla bla
+ |   base_type: ClientError
  | ...
 e.type
  | ---
@@ -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.
@@ -890,3 +892,60 @@ check_trace(e:unpack().traceback)
  | ---
  | - true
  | ...
+
+--
+-- gh-4398: custom error type.
+--
+-- Try no code.
+e = box.error.new({type = 'TestType', reason = 'Test reason'})
+ | ---
+ | ...
+e:unpack()
+ | ---
+ | - code: 214
+ |   trace:
+ |   - file: '[string "e = box.error.new({type = ''TestType'', reason ..."]'
+ |     line: 1
+ |   type: TestType
+ |   custom_type: TestType
+ |   message: Test reason
+ |   base_type: CustomError
+ | ...
+-- Try code not the same as used by default.
+e = box.error.new({type = 'TestType', reason = 'Test reason', code = 123})
+ | ---
+ | ...
+e:unpack()
+ | ---
+ | - code: 123
+ |   trace:
+ |   - file: '[string "e = box.error.new({type = ''TestType'', reason ..."]'
+ |     line: 1
+ |   type: TestType
+ |   custom_type: TestType
+ |   message: Test reason
+ |   base_type: CustomError
+ | ...
+-- Try to omit message.
+e = box.error.new({type = 'TestType'})
+ | ---
+ | ...
+e:unpack()
+ | ---
+ | - code: 214
+ |   trace:
+ |   - file: '[string "e = box.error.new({type = ''TestType''}) "]'
+ |     line: 1
+ |   type: TestType
+ |   custom_type: TestType
+ |   message: 
+ |   base_type: CustomError
+ | ...
+-- 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 6f12716..fe40518 100644
--- a/test/box/error.test.lua
+++ b/test/box/error.test.lua
@@ -262,3 +262,19 @@ check_trace(t3(nil, false):unpack().traceback)
 box.error.cfg{traceback_enable = false}
 _, e = pcall(t3, true, true)
 check_trace(e:unpack().traceback)
+
+--
+-- 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..12fb0ea 100644
--- a/test/engine/func_index.result
+++ b/test/engine/func_index.result
@@ -281,22 +281,24 @@ e:unpack()
  |   - file: <filename>
  |     line: <line>
  |   type: ClientError
- |   message: 'Failed to build a key for functional index ''idx'' of space ''withdata'':
- |     can''t evaluate function'
  |   prev: '[string "return function(tuple)                 local ..."]:1: attempt to
  |     call global ''require'' (a nil value)'
+ |   message: 'Failed to build a key for functional index ''idx'' of space ''withdata'':
+ |     can''t evaluate function'
+ |   base_type: ClientError
  | ...
 e = e.prev
  | ---
  | ...
 e:unpack()
  | ---
- | - type: LuajitError
- |   message: '[string "return function(tuple)                 local ..."]:1: attempt
- |     to call global ''require'' (a nil value)'
- |   trace:
+ | - trace:
  |   - file: <filename>
  |     line: <line>
+ |   type: LuajitError
+ |   message: '[string "return function(tuple)                 local ..."]:1: attempt
+ |     to call global ''require'' (a nil value)'
+ |   base_type: LuajitError
  | ...
 e = e.prev
  | ---
-- 
2.7.4

  parent reply	other threads:[~2020-04-15  9:32 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-15  9:31 [Tarantool-patches] [PATCH V3 0/7] Extending error functionality Leonid Vasiliev
2020-04-15  9:31 ` [Tarantool-patches] [PATCH V3 1/7] error: add a Lua traceback to error Leonid Vasiliev
2020-04-15  9:31 ` Leonid Vasiliev [this message]
2020-04-15  9:31 ` [Tarantool-patches] [PATCH V3 3/7] error: send custom type in IProto Leonid Vasiliev
2020-04-15  9:31 ` [Tarantool-patches] [PATCH V3 4/7] session: add offset to SQL session settings array Leonid Vasiliev
2020-04-15  9:32 ` [Tarantool-patches] [PATCH V3 5/7] error: add session setting for error type marshaling Leonid Vasiliev
2020-04-15  9:32 ` [Tarantool-patches] [PATCH V3 6/7] error: update constructors of some errors Leonid Vasiliev
2020-04-15  9:32 ` [Tarantool-patches] [PATCH V3 7/7] error: add error MsgPack encoding Leonid Vasiliev
2020-04-15 15:08   ` lvasiliev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f78f059fc08baf77053c9014aa573dccd9e4067a.1586934134.git.lvasiliev@tarantool.org \
    --to=lvasiliev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH V3 2/7] error: add custom error type' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox