Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: tarantool-patches@dev.tarantool.org
Cc: v.shpilevoy@tarantool.org
Subject: [Tarantool-patches] [PATCH 3/7] box/error: don't set error created via box.error.new to diag
Date: Wed, 19 Feb 2020 17:16:52 +0300	[thread overview]
Message-ID: <f73a37259ffbe271e9353361f4e04c3190ee9a26.1582119629.git.korablev@tarantool.org> (raw)
In-Reply-To: <cover.1582119629.git.korablev@tarantool.org>
In-Reply-To: <cover.1582119629.git.korablev@tarantool.org>

To achieve this let's refactor luaT_error_create() to return error
object instead of setting it via box_error_set().
luaT_error_created() is used both to handle box.error() and
box.error.new() invocations, and box.error() is still expected to set
error to diagnostic area. So, luaT_error_call() which implements
box.error() processing at the end calls diag_set_error().
It is worth mentioning that net.box module relied on the fact that
box.error.new() set error to diagnostic area: otherwise request errors
don't get to diagnostic area on client side.

@TarantoolBot document
Title: Don't promote error created via box.error.new to diagnostic area

Now box.error.new() only creates error object, but doesn't set it to
Tarantool's diagnostic area:
box.error.clear()
e = box.error.new({code = 111, reason = "cause"})
assert(box.error.last() == nil)
---
- true
...

To set error in diagnostic area explicitly box.error.set() has been
introduced. It accepts error object which is set as last system error
(i.e. becomes available via box.error.last()).
Finally, box.error.new() does not longer accept error object as an
argument (this was undocumented feature).
Note that patch does not affect box.error(), which still pushed error to
diagnostic area. This fact is reflected in docs:
'''
Emulate a request error, with text based on one of the pre-defined
Tarantool errors...
'''

Needed for #1148
Closes #4778
---
 src/box/error.cc       | 16 +++++++++++++++
 src/box/error.h        |  8 ++++++++
 src/box/lua/error.cc   | 56 ++++++++++++++++++++++++++++----------------------
 test/box/misc.result   | 30 ++++++++++++++++++++++++++-
 test/box/misc.test.lua | 16 +++++++++++++++
 5 files changed, 101 insertions(+), 25 deletions(-)

diff --git a/src/box/error.cc b/src/box/error.cc
index 7dfe1b3ee..824a4617c 100644
--- a/src/box/error.cc
+++ b/src/box/error.cc
@@ -86,6 +86,22 @@ box_error_set(const char *file, unsigned line, uint32_t code,
 	return -1;
 }
 
+struct error *
+box_error_construct(const char *file, unsigned line, uint32_t code,
+		    const char *fmt, ...)
+{
+	struct error *e = BuildClientError(file, line, ER_UNKNOWN);
+	ClientError *client_error = type_cast(ClientError, e);
+	if (client_error != NULL) {
+		client_error->m_errcode = code;
+		va_list ap;
+		va_start(ap, fmt);
+		error_vformat_msg(e, fmt, ap);
+		va_end(ap);
+	}
+	return e;
+}
+
 /* }}} */
 
 struct rmean *rmean_error = NULL;
diff --git a/src/box/error.h b/src/box/error.h
index b8c7cf73d..42043ef80 100644
--- a/src/box/error.h
+++ b/src/box/error.h
@@ -129,6 +129,14 @@ int
 box_error_set(const char *file, unsigned line, uint32_t code,
 	      const char *format, ...);
 
+/**
+ * Construct error object without setting it in the diagnostics
+ * area. On the memory allocation fail returns NULL.
+ */
+struct error *
+box_error_construct(const char *file, unsigned line, uint32_t code,
+		    const char *fmt, ...);
+
 /**
  * A backward-compatible API define.
  */
diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
index 7311cb2ce..17d4de653 100644
--- a/src/box/lua/error.cc
+++ b/src/box/lua/error.cc
@@ -42,7 +42,14 @@ extern "C" {
 #include "lua/utils.h"
 #include "box/error.h"
 
-static void
+/**
+ * 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.
+ * In case one of arguments is missing it corresponding field
+ * in struct error is filled with default value.
+ */
+static struct error *
 luaT_error_create(lua_State *L, int top_base)
 {
 	uint32_t code = 0;
@@ -69,25 +76,19 @@ luaT_error_create(lua_State *L, int top_base)
 			reason = lua_tostring(L, -1);
 		} else if (strchr(reason, '%') != NULL) {
 			/* Missing arguments to format string */
-			luaL_error(L, "box.error(): bad arguments");
-		}
-	} else if (top == top_base) {
-		if (lua_istable(L, top_base)) {
-			/* A special case that rethrows raw error (used by net.box) */
-			lua_getfield(L, top_base, "code");
-			code = lua_tonumber(L, -1);
-			lua_pop(L, 1);
-			lua_getfield(L, top_base, "reason");
-			reason = lua_tostring(L, -1);
-			if (reason == NULL)
-				reason = "";
-			lua_pop(L, 1);
-		} else if (luaL_iserror(L, top_base)) {
-			lua_error(L);
-			return;
+			return NULL;
 		}
+	} else if (top == top_base && lua_istable(L, top_base)) {
+		lua_getfield(L, top_base, "code");
+		code = lua_tonumber(L, -1);
+		lua_pop(L, 1);
+		lua_getfield(L, top_base, "reason");
+		reason = lua_tostring(L, -1);
+		if (reason == NULL)
+			reason = "";
+		lua_pop(L, 1);
 	} else {
-		luaL_error(L, "box.error(): bad arguments");
+		return NULL;
 	}
 
 raise:
@@ -101,8 +102,7 @@ raise:
 		}
 		line = info.currentline;
 	}
-	say_debug("box.error() at %s:%i", file, line);
-	box_error_set(file, line, code, "%s", reason);
+	return box_error_construct(file, line, code, "%s", reason);
 }
 
 static int
@@ -111,10 +111,15 @@ luaT_error_call(lua_State *L)
 	if (lua_gettop(L) <= 1) {
 		/* Re-throw saved exceptions if any. */
 		if (box_error_last())
-			luaT_error(L);
+			return luaT_error(L);
 		return 0;
 	}
-	luaT_error_create(L, 2);
+	if (lua_gettop(L) == 2 && luaL_iserror(L, 2))
+		return lua_error(L);
+	struct error *e = luaT_error_create(L, 2);
+	if (e == NULL)
+		return luaL_error(L, "box.error(): bad arguments");
+	diag_set_error(&fiber()->diag, e);
 	return luaT_error(L);
 }
 
@@ -139,9 +144,12 @@ luaT_error_new(lua_State *L)
 {
 	if (lua_gettop(L) == 0)
 		return luaL_error(L, "Usage: box.error.new(code, args)");
-	luaT_error_create(L, 1);
+	struct error *e = luaT_error_create(L, 1);
+	if (e == NULL)
+		return luaL_error(L, "box.error.new(): bad arguments");
 	lua_settop(L, 0);
-	return luaT_error_last(L);
+	luaT_pusherror(L, e);
+	return 1;
 }
 
 static int
diff --git a/test/box/misc.result b/test/box/misc.result
index 48c00aadc..b0a81a055 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -298,7 +298,7 @@ err = box.error.new({code = 111, reason = "cause"})
 ...
 assert(box.error.last() ~= err)
 ---
-- error: assertion failed!
+- true
 ...
 box.error.set(err)
 ---
@@ -324,6 +324,34 @@ assert(box.error.last() == err)
 ---
 - true
 ...
+-- Check that box.error.new() does not set error to diag.
+--
+box.error.clear()
+---
+...
+err = box.error.new(1, "cause")
+---
+...
+assert(box.error.last() == nil)
+---
+- true
+...
+-- box.error.new() does not accept error objects.
+--
+box.error.new(err)
+---
+- error: 'box.error.new(): bad arguments'
+...
+-- box.error() is supposed to re-throw last diagnostic error.
+-- Make sure it does not fail if there's no errors at all
+-- (in diagnostics area).
+--
+box.error.clear()
+---
+...
+box.error()
+---
+...
 ----------------
 -- # box.stat
 ----------------
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index cb7b775fc..0351317dd 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -104,6 +104,22 @@ box.error.set(1)
 box.error.set(nil)
 box.error.set(box.error.last())
 assert(box.error.last() == err)
+-- Check that box.error.new() does not set error to diag.
+--
+box.error.clear()
+err = box.error.new(1, "cause")
+assert(box.error.last() == nil)
+
+-- box.error.new() does not accept error objects.
+--
+box.error.new(err)
+
+-- box.error() is supposed to re-throw last diagnostic error.
+-- Make sure it does not fail if there's no errors at all
+-- (in diagnostics area).
+--
+box.error.clear()
+box.error()
 
 ----------------
 -- # box.stat
-- 
2.15.1

  parent reply	other threads:[~2020-02-19 14:17 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-19 14:16 [Tarantool-patches] [PATCH 0/7] Stacked diagnostics area Nikita Pettik
2020-02-19 14:16 ` [Tarantool-patches] [PATCH 1/7] box: rename diag_add_error to diag_set_error Nikita Pettik
2020-02-19 14:16 ` [Tarantool-patches] [PATCH 2/7] box/error: introduce box.error.set() method Nikita Pettik
2020-02-19 14:26   ` Cyrill Gorcunov
2020-02-19 14:30     ` Nikita Pettik
2020-02-19 14:53       ` Cyrill Gorcunov
2020-02-19 14:16 ` Nikita Pettik [this message]
2020-02-22 17:18   ` [Tarantool-patches] [PATCH 3/7] box/error: don't set error created via box.error.new to diag Vladislav Shpilevoy
2020-03-25  1:02     ` Nikita Pettik
2020-03-26  0:22       ` Vladislav Shpilevoy
2020-03-26  1:03         ` Nikita Pettik
2020-02-19 14:16 ` [Tarantool-patches] [PATCH 4/7] box: introduce stacked diagnostic area Nikita Pettik
2020-02-19 21:10   ` Vladislav Shpilevoy
2020-02-20 11:53     ` Nikita Pettik
2020-02-20 18:29       ` Nikita Pettik
2020-02-23 17:43   ` Vladislav Shpilevoy
2020-03-25  1:34     ` Nikita Pettik
2020-02-19 14:16 ` [Tarantool-patches] [PATCH 5/7] box/error: clarify purpose of reference counting in struct error Nikita Pettik
2020-02-23 17:43   ` Vladislav Shpilevoy
2020-03-25  1:40     ` Nikita Pettik
2020-02-19 14:16 ` [Tarantool-patches] [PATCH 6/7] iproto: refactor error encoding with mpstream Nikita Pettik
2020-02-23 17:44   ` Vladislav Shpilevoy
2020-03-25  1:42     ` Nikita Pettik
2020-02-19 14:16 ` [Tarantool-patches] [PATCH 7/7] iproto: support error stacked diagnostic area Nikita Pettik
2020-02-23 17:43   ` Vladislav Shpilevoy
2020-03-25  1:38     ` Nikita Pettik
2020-02-22 17:18 ` [Tarantool-patches] [PATCH 0/7] Stacked diagnostics area Vladislav Shpilevoy

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=f73a37259ffbe271e9353361f4e04c3190ee9a26.1582119629.git.korablev@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 3/7] box/error: don'\''t set error created via box.error.new to diag' \
    /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