Tarantool development patches archive
 help / color / mirror / Atom feed
From: Ilya Markov <imarkov@tarantool.org>
To: georgy@tarantool.org
Cc: tarantool-patches@freelists.org
Subject: [tarantool-patches] [error 2/3] error: Add lua traceback
Date: Fri,  4 May 2018 17:07:19 +0300	[thread overview]
Message-ID: <7c054d519fbaa94a752f213aed64330e6c66b558.1525442633.git.imarkov@tarantool.org> (raw)
In-Reply-To: <cover.1525442633.git.imarkov@tarantool.org>
In-Reply-To: <cover.1525442633.git.imarkov@tarantool.org>

* Extend error structure with traceback structure
* Add function generating traceback in case of custom (luaT_call)
 error call
* Replace lua pcall with custom implementation of pcall,
generating trace.
* Replace lua error function with custom implementation of error,
handling tarantool error objects.

Relates #677
---
 src/diag.c                    |   2 +
 src/diag.h                    |  15 ++++-
 src/lua/error.c               | 150 ++++++++++++++++++++++++++++++++++++++++++
 src/lua/error.h               |   3 +
 src/lua/error.lua             |  16 ++---
 src/lua/init.lua              |  11 +++-
 src/lua/utils.c               |  16 ++++-
 test/app-tap/trigger.test.lua |   2 +-
 test/app/traceback.result     | 124 ++++++++++++++++++++++++++++++++++
 test/app/traceback.test.lua   |  40 +++++++++++
 test/box-tap/session.test.lua |  16 ++---
 test/box/misc.result          |  42 +++++++++++-
 test/box/misc.test.lua        |   1 +
 13 files changed, 415 insertions(+), 23 deletions(-)
 create mode 100644 test/app/traceback.result
 create mode 100644 test/app/traceback.test.lua

diff --git a/src/diag.c b/src/diag.c
index 248277e..91d1b2e 100644
--- a/src/diag.c
+++ b/src/diag.c
@@ -52,6 +52,8 @@ error_create(struct error *e,
 		e->line = 0;
 	}
 	e->errmsg[0] = '\0';
+	e->frames_count = 0;
+	rlist_create(&e->frames);
 }
 
 struct diag *
diff --git a/src/diag.h b/src/diag.h
index dc6c132..bee47f2 100644
--- a/src/diag.h
+++ b/src/diag.h
@@ -44,12 +44,20 @@ extern "C" {
 
 enum {
 	DIAG_ERRMSG_MAX = 512,
-	DIAG_FILENAME_MAX = 256
+	DIAG_FILENAME_MAX = 256,
+	DIAG_FUNCNAME_MAX = 256,
 };
 
 struct type_info;
 struct error;
 
+struct diag_frame {
+	int line;
+	char filename[DIAG_FILENAME_MAX];
+	char func_name[DIAG_FUNCNAME_MAX];
+	struct rlist link;
+};
+
 typedef void (*error_f)(struct error *e);
 
 /**
@@ -78,8 +86,13 @@ struct error {
 	char file[DIAG_FILENAME_MAX];
 	/* Error description. */
 	char errmsg[DIAG_ERRMSG_MAX];
+	/** Error traceback */
+	struct rlist frames;
+	int frames_count;
 };
 
+
+
 static inline void
 error_ref(struct error *e)
 {
diff --git a/src/lua/error.c b/src/lua/error.c
index d660e4c..e6ccd32 100644
--- a/src/lua/error.c
+++ b/src/lua/error.c
@@ -104,6 +104,145 @@ luaT_pusherror(struct lua_State *L, struct error *e)
 	luaL_setcdatagc(L, -2);
 }
 
+static int
+traceback_error(struct lua_State *L, struct error *e)
+{
+	lua_Debug ar;
+	int level = 0;
+	while (lua_getstack(L, level++, &ar) > 0) {
+		lua_getinfo(L, "Sln", &ar);
+		struct diag_frame *frame =
+				(struct diag_frame *) malloc(sizeof(*frame));
+		if (frame == NULL) {
+			luaT_pusherror(L, e);
+			return 1;
+		}
+		if (*ar.what == 'L' || *ar.what == 'm') {
+			strcpy(frame->filename, ar.short_src);
+			frame->line = ar.currentline;
+			if (*ar.namewhat != '\0') {
+				strcpy(frame->func_name, ar.name);
+			} else {
+				frame->func_name[0] = 0;
+			}
+			e->frames_count++;
+		} else if (*ar.what == 'C') {
+			if (*ar.namewhat != '\0') {
+				strcpy(frame->func_name, ar.name);
+			} else {
+				frame->func_name[0] = 0;
+			}
+			frame->filename[0] = 0;
+			frame->line = 0;
+			e->frames_count++;
+		}
+		rlist_add_entry(&e->frames, frame, link);
+	}
+	luaT_pusherror(L, e);
+	return 1;
+}
+
+int
+luaT_traceback(struct lua_State *L)
+{
+	struct error* e = luaL_iserror(L, -1);
+	if (e == NULL) {
+		const char *msg = lua_tostring(L, -1);
+		if (msg == NULL) {
+			say_error("pcall calls error handler on empty error");
+			return 0;
+		} else {
+			e = BuildLuajitError(__FILE__, __LINE__, msg);
+		}
+	}
+	return traceback_error(L, e);
+}
+
+int
+lua_error_gettraceback(struct lua_State *L)
+{
+	struct error *e = luaL_iserror(L, -1);
+	if (!e) {
+		return 0;
+	}
+	lua_newtable(L);
+	if (e->frames_count <= 0) {
+		return 1;
+	}
+	struct diag_frame *frame;
+	int index = 1;
+	rlist_foreach_entry(frame, &e->frames, link) {
+		if (frame->func_name[0] != 0 || frame->line > 0 ||
+		    frame->filename[0] != 0) {
+			/* push index */
+			lua_pushnumber(L, index++);
+			/* push value - table of filename and line */
+			lua_newtable(L);
+			if (frame->func_name[0] != 0) {
+				lua_pushstring(L, "function");
+				lua_pushstring(L, frame->func_name);
+				lua_settable(L, -3);
+			}
+			if (frame->filename[0] != 0) {
+				lua_pushstring(L, "file");
+				lua_pushstring(L, frame->filename);
+				lua_settable(L, -3);
+			}
+			if (frame->line > 0) {
+				lua_pushstring(L, "line");
+				lua_pushinteger(L, frame->line);
+				lua_settable(L, -3);
+			}
+			lua_settable(L, -3);
+		}
+	}
+	return 1;
+}
+
+/**
+ * Function replacing lua pcall function.
+ * We handle lua errors, creating tarantool error objects and
+ * saving traceback inside.
+ */
+static int
+luaB_pcall(struct lua_State *L)
+{
+	int status;
+	luaL_checkany(L, 1);
+	status = luaT_call(L, lua_gettop(L) - 1, LUA_MULTRET);
+	lua_pushboolean(L, (status == 0));
+	lua_insert(L, 1);
+	return lua_gettop(L);  /* return status + all results */
+}
+
+/**
+ * Function replacing lua error function.
+ * We have to handle tarantool error objects, converting them to string
+ * for generating string errors with path in case of call error(msg, level),
+ * where level > 0.
+ */
+static int
+luaB_error (lua_State *L) {
+	int level = luaL_optint(L, 2, 1);
+	lua_settop(L, 1);
+	if (lua_type(L, 1) == LUA_TCDATA) {
+		assert(CTID_CONST_STRUCT_ERROR_REF != 0);
+		uint32_t ctypeid;
+		void *data = luaL_checkcdata(L, 1, &ctypeid);
+		if (ctypeid != (uint32_t) CTID_CONST_STRUCT_ERROR_REF)
+			return lua_error(L);
+
+		struct error *e = *(struct error **) data;
+		lua_pushstring(L, e->errmsg);
+	}
+	if (lua_isstring(L, -1) && level > 0) {  /* add extra information? */
+		luaL_where(L, level);
+		lua_insert(L, lua_gettop(L) - 1);
+		lua_concat(L, 2);
+	}
+	return lua_error(L);
+}
+
 void
 tarantool_lua_error_init(struct lua_State *L)
 {
@@ -114,4 +253,15 @@ tarantool_lua_error_init(struct lua_State *L)
 	(void) rc;
 	CTID_CONST_STRUCT_ERROR_REF = luaL_ctypeid(L, "const struct error &");
 	assert(CTID_CONST_STRUCT_ERROR_REF != 0);
+	lua_pushcfunction(L, luaB_pcall);
+	lua_setglobal(L, "pcall");
+	lua_pushcfunction(L, luaB_error);
+	lua_setglobal(L, "error");
+
+	static const luaL_Reg errorslib[] = {
+		{"get_traceback", lua_error_gettraceback},
+		{ NULL, NULL}
+	};
+	luaL_register_module(L, "error", errorslib);
+	lua_pop(L, 1);
 }
diff --git a/src/lua/error.h b/src/lua/error.h
index ee489da..9b126b4 100644
--- a/src/lua/error.h
+++ b/src/lua/error.h
@@ -57,6 +57,9 @@ luaT_pusherror(struct lua_State *L, struct error *e);
 struct error *
 luaL_iserror(struct lua_State *L, int narg);
 
+int
+luaT_traceback(struct lua_State *L);
+
 void
 tarantool_lua_error_init(struct lua_State *L);
 
diff --git a/src/lua/error.lua b/src/lua/error.lua
index a402378..303a48c 100644
--- a/src/lua/error.lua
+++ b/src/lua/error.lua
@@ -25,6 +25,8 @@ exception_get_int(struct error *e, const struct method_info *method);
 
 ]]
 
+local error = require("error")
+
 local REFLECTION_CACHE = {}
 
 local function reflection_enumerate(err)
@@ -67,17 +69,15 @@ local function error_type(err)
     return ffi.string(err._type.name)
 end
 
-local function error_message(err)
-    return ffi.string(err._errmsg)
-end
-
 local function error_trace(err)
-    if err._file[0] == 0 then
+    if err.depth_traceback == 0 then
         return {}
     end
-    return {
-        { file = ffi.string(err._file), line = tonumber(err._line) };
-    }
+    return error.get_traceback(err)
+end
+
+local function error_message(err)
+    return ffi.string(err._errmsg)
 end
 
 
diff --git a/src/lua/init.lua b/src/lua/init.lua
index 5d08151..af2f0dc 100644
--- a/src/lua/init.lua
+++ b/src/lua/init.lua
@@ -20,11 +20,17 @@ struct type_info {
 
 enum {
     DIAG_ERRMSG_MAX = 512,
-    DIAG_FILENAME_MAX = 256
+    DIAG_FILENAME_MAX = 256,
+    DIAG_FUNCNAME_MAX = 256,
 };
 
 typedef void (*error_f)(struct error *e);
 
+struct rlist {
+    struct rlist *prev;
+    struct rlist *next;
+};
+
 struct error {
     error_f _destroy;
     error_f _raise;
@@ -37,6 +43,9 @@ struct error {
     char _file[DIAG_FILENAME_MAX];
     /* Error description. */
     char _errmsg[DIAG_ERRMSG_MAX];
+    /** Error traceback */
+    struct rlist frames;
+    int frames_count;
 };
 
 double
diff --git a/src/lua/utils.c b/src/lua/utils.c
index bf1548b..9b051ba 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -861,8 +861,20 @@ lbox_catch(lua_State *L)
 int
 luaT_call(struct lua_State *L, int nargs, int nreturns)
 {
-	if (lua_pcall(L, nargs, nreturns, 0))
-		return lbox_catch(L);
+	lua_pushcfunction(L, luaT_traceback);
+	int save_pos = lua_gettop(L) - nargs - 1;
+	lua_insert(L, save_pos);
+	if (lua_pcall(L, nargs, nreturns, lua_gettop(L) - nargs - 1)) {
+		lua_remove(L, save_pos);
+		struct error *e = luaL_iserror(L, -1);
+		if (e != NULL) {
+			diag_add_error(&fiber()->diag, e);
+		} else {
+			say_error("pcall returned with empty error");
+		}
+		return 1;
+	}
+	lua_remove(L, save_pos);
 	return 0;
 }
 
diff --git a/test/app-tap/trigger.test.lua b/test/app-tap/trigger.test.lua
index a31d45e..36e5475 100755
--- a/test/app-tap/trigger.test.lua
+++ b/test/app-tap/trigger.test.lua
@@ -47,7 +47,7 @@ test:test("simple trigger test", function(test)
     -- Check that we've failed to delete trigger
     local stat, err = pcall(getmetatable(trigger_list).__call, trigger_list,
                             nil, trigger_cnt)
-    test:ok(string.find(err, "is not found"), "check error")
+    test:ok(string.find(tostring(err), "is not found"), "check error")
 end)
 
 test:test("errored trigger test", function(test)
diff --git a/test/app/traceback.result b/test/app/traceback.result
new file mode 100644
index 0000000..bb54573
--- /dev/null
+++ b/test/app/traceback.result
@@ -0,0 +1,124 @@
+s, err = pcall(box.error, 1, "err")
+---
+...
+err.type
+---
+- ClientError
+...
+err.trace[1].file
+---
+- builtin/socket.lua
+...
+err.trace[1].line
+---
+- 997
+...
+#err.trace
+---
+- 8
+...
+s, err = pcall(error, "oh no" )
+---
+...
+err = err:unpack()
+---
+...
+err.type
+---
+- LuajitError
+...
+err.message
+---
+- oh no
+...
+#err.trace
+---
+- 8
+...
+nil_var=nil
+---
+...
+s, err = pcall(function() return nil_var.b end)
+---
+...
+function fail() return nil_var.b end
+---
+...
+s, err = pcall(fail)
+---
+...
+err = err:unpack()
+---
+...
+err.type
+---
+- LuajitError
+...
+err.message
+---
+- '[string "function fail() return nil_var.b end "]:1: attempt to index global ''nil_var''
+  (a nil value)'
+...
+#err.trace
+---
+- 10
+...
+box.begin()
+---
+...
+s, err = pcall(box.schema.user.create, "user" )
+---
+...
+err = err:unpack()
+---
+...
+err.type
+---
+- ClientError
+...
+err.message
+---
+- Space _user does not support multi-statement transactions
+...
+#err.trace
+---
+- 10
+...
+errinj = box.error.injection
+---
+...
+space = box.schema.space.create('tweedledum')
+---
+...
+index = space:create_index('primary', { type = 'hash' })
+---
+...
+errinj.set("ERRINJ_TESTING", true)
+---
+- ok
+...
+s, err = pcall(space.select, space, {222444})
+---
+...
+errinj.set("ERRINJ_TESTING", false)
+---
+- ok
+...
+err = err:unpack()
+---
+...
+err.type
+---
+- ClientError
+...
+err.message
+---
+- Error injection 'ERRINJ_TESTING'
+...
+#err.trace
+---
+- 8
+...
+space:drop()
+---
+...
diff --git a/test/app/traceback.test.lua b/test/app/traceback.test.lua
new file mode 100644
index 0000000..51af677
--- /dev/null
+++ b/test/app/traceback.test.lua
@@ -0,0 +1,40 @@
+s, err = pcall(box.error, 1, "err")
+err.type
+err.trace[1].file
+err.trace[1].line
+#err.trace
+
+s, err = pcall(error, "oh no" )
+err = err:unpack()
+err.type
+err.message
+#err.trace
+nil_var=nil
+s, err = pcall(function() return nil_var.b end)
+
+function fail() return nil_var.b end
+
+s, err = pcall(fail)
+err = err:unpack()
+err.type
+err.message
+#err.trace
+
+box.begin()
+s, err = pcall(box.schema.user.create, "user" )
+err = err:unpack()
+err.type
+err.message
+#err.trace
+
+errinj = box.error.injection
+space = box.schema.space.create('tweedledum')
+index = space:create_index('primary', { type = 'hash' })
+errinj.set("ERRINJ_TESTING", true)
+s, err = pcall(space.select, space, {222444})
+errinj.set("ERRINJ_TESTING", false)
+err = err:unpack()
+err.type
+err.message
+#err.trace
+space:drop()
diff --git a/test/box-tap/session.test.lua b/test/box-tap/session.test.lua
index 6fddced..c54ad68 100755
--- a/test/box-tap/session.test.lua
+++ b/test/box-tap/session.test.lua
@@ -23,9 +23,9 @@ test:plan(53)
 test:ok(session.exists(session.id()), "session is created")
 test:isnil(session.peer(session.id()), "session.peer")
 local ok, err = pcall(session.exists)
-test:is(err, "session.exists(sid): bad arguments", "exists bad args #1")
+test:is(tostring(err), "session.exists(sid): bad arguments", "exists bad args #1")
 ok, err = pcall(session.exists, 1, 2, 3)
-test:is(err, "session.exists(sid): bad arguments", "exists bad args #2")
+test:is(tostring(err), "session.exists(sid): bad arguments", "exists bad args #2")
 test:ok(not session.exists(1234567890), "session doesn't exist")
 
 -- check session.id()
@@ -51,19 +51,19 @@ test:is(type(session.on_connect()), "table", "type of trigger on_connect, no arg
 test:is(type(session.on_disconnect()), "table", "type of trigger on_disconnect, no args")
 
 ok, err = pcall(session.on_connect, function() end, function() end)
-test:is(err,"trigger reset: Trigger is not found", "on_connect trigger not found")
+test:is(tostring(err),"trigger reset: Trigger is not found", "on_connect trigger not found")
 ok, err = pcall(session.on_disconnect, function() end, function() end)
-test:is(err,"trigger reset: Trigger is not found", "on_disconnect trigger not found")
+test:is(tostring(err),"trigger reset: Trigger is not found", "on_disconnect trigger not found")
 
 ok, err = pcall(session.on_connect, 1, 2)
-test:is(err, "trigger reset: incorrect arguments", "on_connect bad args #1")
+test:is(tostring(err), "trigger reset: incorrect arguments", "on_connect bad args #1")
 ok, err = pcall(session.on_disconnect, 1, 2)
-test:is(err, "trigger reset: incorrect arguments", "on_disconnect bad args #1")
+test:is(tostring(err), "trigger reset: incorrect arguments", "on_disconnect bad args #1")
 
 ok, err = pcall(session.on_connect, 1)
-test:is(err, "trigger reset: incorrect arguments", "on_connect bad args #2")
+test:is(tostring(err), "trigger reset: incorrect arguments", "on_connect bad args #2")
 ok, err = pcall(session.on_disconnect, 1)
-test:is(err, "trigger reset: incorrect arguments", "on_disconnect bad args #2")
+test:is(tostring(err), "trigger reset: incorrect arguments", "on_disconnect bad args #2")
 
 -- use of nil to clear the trigger
 session.on_connect(nil, fail)
diff --git a/test/box/misc.result b/test/box/misc.result
index cd3af7f..a5cad07 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -8,6 +8,10 @@ test_run:cmd("push filter 'table: .*' to 'table: <address>'")
 ---
 - true
 ...
+test_run:cmd("push filter 'line: .*' to 'line: <number>'")
+---
+- true
+...
 -- gh-266: box.info() crash on uncofigured box
 package.loaded['box.space'] == nil
 ---
@@ -126,8 +130,42 @@ e:unpack()
   code: 1
   message: Illegal parameters, bla bla
   trace:
-  - file: '[C]'
-    line: 4294967295
+  - file: builtin/socket.lua
+    line: <number>
+  - function: pcall
+  - file: builtin/box/console.lua
+    line: <number>
+  - file: builtin/box/console.lua
+    function: repl
+    line: <number>
+  - file: builtin/box/console.lua
+    function: eval
+    line: <number>
+  - function: pcall
+  - file: builtin/socket.lua
+    line: <number>
+  - function: pcall
+  - file: builtin/box/console.lua
+    line: <number>
+  - file: builtin/box/console.lua
+    function: repl
+    line: <number>
+  - file: builtin/box/console.lua
+    function: eval
+    line: <number>
+  - function: pcall
+  - file: builtin/socket.lua
+    line: <number>
+  - function: pcall
+  - file: builtin/box/console.lua
+    line: <number>
+  - file: builtin/box/console.lua
+    function: repl
+    line: <number>
+  - file: builtin/box/console.lua
+    function: eval
+    line: <number>
+  - function: pcall
 ...
 e.type
 ---
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index b7bf600..38d96a0 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -1,6 +1,7 @@
 env = require('test_run')
 test_run = env.new()
 test_run:cmd("push filter 'table: .*' to 'table: <address>'")
+test_run:cmd("push filter 'line: .*' to 'line: <number>'")
 
 -- gh-266: box.info() crash on uncofigured box
 package.loaded['box.space'] == nil
-- 
2.7.4

  parent reply	other threads:[~2018-05-04 14:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <14848871.eLyv8AkjAN@home.lan>
2018-05-04 14:07 ` [tarantool-patches] [error 0/3] Introduce error traceback Ilya Markov
2018-05-04 14:07   ` [tarantool-patches] [error 1/3] lua: moving lua error functions to separate file Ilya Markov
2018-05-04 14:07   ` Ilya Markov [this message]
2018-05-04 14:07   ` [tarantool-patches] [error 3/3] error: Add C frames in error.traceback Ilya Markov

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=7c054d519fbaa94a752f213aed64330e6c66b558.1525442633.git.imarkov@tarantool.org \
    --to=imarkov@tarantool.org \
    --cc=georgy@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [error 2/3] error: Add lua traceback' \
    /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