Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [error 0/3] Introduce error traceback
       [not found] <14848871.eLyv8AkjAN@home.lan>
@ 2018-05-04 14:07 ` Ilya Markov
  2018-05-04 14:07   ` [tarantool-patches] [error 1/3] lua: moving lua error functions to separate file Ilya Markov
                     ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Ilya Markov @ 2018-05-04 14:07 UTC (permalink / raw)
  To: georgy; +Cc: tarantool-patches



Ilya Markov (3):
  lua: moving lua error functions to separate file
  error: Add lua traceback
  error: Add C frames in error.traceback

  branch: gh-677-augment-exceptions

 src/CMakeLists.txt                |   3 +
 src/backtrace.cc                  |  10 +-
 src/box/func.c                    |   1 +
 src/box/lua/call.c                |   1 +
 src/box/lua/cfg.cc                |   3 +-
 src/box/lua/ctl.c                 |   1 +
 src/box/lua/error.cc              |   1 +
 src/box/lua/index.c               |   1 +
 src/box/lua/init.c                |   1 +
 src/box/lua/misc.cc               |   1 +
 src/box/lua/sequence.c            |   1 +
 src/box/lua/session.c             |   1 +
 src/box/lua/tuple.c               |   1 +
 src/box/lua/xlog.c                |   1 +
 src/diag.c                        |   2 +
 src/diag.h                        |  15 +-
 src/exception.cc                  |  49 ++++++
 src/lua/error.c                   | 353 ++++++++++++++++++++++++++++++++++++++
 src/lua/error.h                   |  70 ++++++++
 src/lua/error.lua                 | 156 +++++++++++++++++
 src/lua/fiber.c                   |   1 +
 src/lua/fio.c                     |   1 +
 src/lua/httpc.c                   |   1 +
 src/lua/init.c                    |   4 +
 src/lua/init.lua                  | 161 ++---------------
 src/lua/pickle.c                  |   1 +
 src/lua/socket.c                  |   1 +
 src/lua/utils.c                   |  93 ++--------
 src/lua/utils.h                   |  14 --
 test/app-tap/trigger.test.lua     |   2 +-
 test/app/traceback.result         | 155 +++++++++++++++++
 test/app/traceback.test.lua       |  50 ++++++
 test/box-tap/session.test.lua     |  16 +-
 test/box/misc.result              |  58 ++++++-
 test/box/misc.test.lua            |   1 +
 test/replication/wal_off.result   |   2 +-
 test/replication/wal_off.test.lua |   2 +-
 37 files changed, 978 insertions(+), 257 deletions(-)
 create mode 100644 src/lua/error.c
 create mode 100644 src/lua/error.h
 create mode 100644 src/lua/error.lua
 create mode 100644 test/app/traceback.result
 create mode 100644 test/app/traceback.test.lua

-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] [error 1/3] lua: moving lua error functions to separate file
  2018-05-04 14:07 ` [tarantool-patches] [error 0/3] Introduce error traceback Ilya Markov
@ 2018-05-04 14:07   ` Ilya Markov
  2018-05-04 14:07   ` [tarantool-patches] [error 2/3] error: Add lua traceback Ilya Markov
  2018-05-04 14:07   ` [tarantool-patches] [error 3/3] error: Add C frames in error.traceback Ilya Markov
  2 siblings, 0 replies; 4+ messages in thread
From: Ilya Markov @ 2018-05-04 14:07 UTC (permalink / raw)
  To: georgy; +Cc: tarantool-patches

Refactoring. Move lua error functions to separate file.

Prerequisite #677
---
 src/CMakeLists.txt     |   3 +
 src/box/func.c         |   1 +
 src/box/lua/call.c     |   1 +
 src/box/lua/cfg.cc     |   3 +-
 src/box/lua/ctl.c      |   1 +
 src/box/lua/error.cc   |   1 +
 src/box/lua/index.c    |   1 +
 src/box/lua/init.c     |   1 +
 src/box/lua/misc.cc    |   1 +
 src/box/lua/sequence.c |   1 +
 src/box/lua/session.c  |   1 +
 src/box/lua/tuple.c    |   1 +
 src/box/lua/xlog.c     |   1 +
 src/lua/error.c        | 117 +++++++++++++++++++++++++++++++++++++
 src/lua/error.h        |  67 +++++++++++++++++++++
 src/lua/error.lua      | 156 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/lua/fiber.c        |   1 +
 src/lua/fio.c          |   1 +
 src/lua/httpc.c        |   1 +
 src/lua/init.c         |   4 ++
 src/lua/init.lua       | 150 +----------------------------------------------
 src/lua/pickle.c       |   1 +
 src/lua/socket.c       |   1 +
 src/lua/utils.c        |  77 +-----------------------
 src/lua/utils.h        |  14 -----
 25 files changed, 368 insertions(+), 239 deletions(-)
 create mode 100644 src/lua/error.c
 create mode 100644 src/lua/error.h
 create mode 100644 src/lua/error.lua

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 8ab09e9..f5c5bec 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -24,6 +24,7 @@ lua_source(lua_sources lua/fiber.lua)
 lua_source(lua_sources lua/buffer.lua)
 lua_source(lua_sources lua/uuid.lua)
 lua_source(lua_sources lua/crypto.lua)
+lua_source(lua_sources lua/error.lua)
 lua_source(lua_sources lua/digest.lua)
 lua_source(lua_sources lua/msgpackffi.lua)
 lua_source(lua_sources lua/uri.lua)
@@ -159,6 +160,7 @@ set (server_sources
      lua/msgpack.c
      lua/utils.c
      lua/errno.c
+     lua/error.c
      lua/socket.c
      lua/pickle.c
      lua/fio.c
@@ -180,6 +182,7 @@ set(api_headers
     ${CMAKE_SOURCE_DIR}/src/coio.h
     ${CMAKE_SOURCE_DIR}/src/coio_task.h
     ${CMAKE_SOURCE_DIR}/src/lua/utils.h
+    ${CMAKE_SOURCE_DIR}/src/lua/error.h
     ${CMAKE_SOURCE_DIR}/src/box/txn.h
     ${CMAKE_SOURCE_DIR}/src/box/key_def.h
     ${CMAKE_SOURCE_DIR}/src/box/field_def.h
diff --git a/src/box/func.c b/src/box/func.c
index dfbc5f3..72336c1 100644
--- a/src/box/func.c
+++ b/src/box/func.c
@@ -32,6 +32,7 @@
 #include "trivia/config.h"
 #include "assoc.h"
 #include "lua/utils.h"
+#include <lua/error.h>
 #include "error.h"
 #include "diag.h"
 #include <dlfcn.h>
diff --git a/src/box/lua/call.c b/src/box/lua/call.c
index be13812..dbab02b 100644
--- a/src/box/lua/call.c
+++ b/src/box/lua/call.c
@@ -28,6 +28,7 @@
  * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
+#include <lua/error.h>
 #include "box/lua/call.h"
 #include "box/call.h"
 #include "box/error.h"
diff --git a/src/box/lua/cfg.cc b/src/box/lua/cfg.cc
index 5e88ca3..99fa63e 100644
--- a/src/box/lua/cfg.cc
+++ b/src/box/lua/cfg.cc
@@ -33,8 +33,9 @@
 
 #include "exception.h"
 #include <cfg.h>
-#include "main.h"
 #include "lua/utils.h"
+#include <lua/error.h>
+#include "main.h"
 
 #include "box/box.h"
 #include "libeio/eio.h"
diff --git a/src/box/lua/ctl.c b/src/box/lua/ctl.c
index 9a105ed..2ede16a 100644
--- a/src/box/lua/ctl.c
+++ b/src/box/lua/ctl.c
@@ -35,6 +35,7 @@
 #include <lua.h>
 #include <lauxlib.h>
 #include <lualib.h>
+#include <lua/error.h>
 
 #include "lua/utils.h"
 
diff --git a/src/box/lua/error.cc b/src/box/lua/error.cc
index 3149074..2adf123 100644
--- a/src/box/lua/error.cc
+++ b/src/box/lua/error.cc
@@ -38,6 +38,7 @@ extern "C" {
 
 #include <fiber.h>
 #include <errinj.h>
+#include <lua/error.h>
 
 #include "lua/utils.h"
 #include "box/error.h"
diff --git a/src/box/lua/index.c b/src/box/lua/index.c
index 6dfa648..116c369 100644
--- a/src/box/lua/index.c
+++ b/src/box/lua/index.c
@@ -28,6 +28,7 @@
  * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
+#include <lua/error.h>
 #include "box/lua/index.h"
 #include "lua/utils.h"
 #include "box/box.h"
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 7547758..0fdaaf9 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -33,6 +33,7 @@
 #include <lua.h>
 #include <lauxlib.h>
 #include <lualib.h>
+#include <lua/error.h>
 
 #include "lua/utils.h" /* luaT_error() */
 
diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index bc76065..f3e6021 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -33,6 +33,7 @@
 #include "fiber.h" /* fiber->gc() */
 #include <small/region.h>
 #include "lua/utils.h"
+#include <lua/error.h>
 #include "lua/msgpack.h"
 
 #include "box/box.h"
diff --git a/src/box/lua/sequence.c b/src/box/lua/sequence.c
index 2fead2e..9e8322a 100644
--- a/src/box/lua/sequence.c
+++ b/src/box/lua/sequence.c
@@ -28,6 +28,7 @@
  * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
+#include <lua/error.h>
 #include "box/lua/sequence.h"
 #include "box/lua/tuple.h"
 #include "lua/utils.h"
diff --git a/src/box/lua/session.c b/src/box/lua/session.c
index 51caf19..277b710 100644
--- a/src/box/lua/session.c
+++ b/src/box/lua/session.c
@@ -36,6 +36,7 @@
 #include <lauxlib.h>
 #include <lualib.h>
 #include <sio.h>
+#include <lua/error.h>
 
 #include "box/box.h"
 #include "box/session.h"
diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
index 7ca4299..cb4969e 100644
--- a/src/box/lua/tuple.c
+++ b/src/box/lua/tuple.c
@@ -36,6 +36,7 @@
 #include <small/ibuf.h>
 #include <small/region.h>
 #include <fiber.h>
+#include <lua/error.h>
 
 #include "box/tuple.h"
 #include "box/tuple_convert.h"
diff --git a/src/box/lua/xlog.c b/src/box/lua/xlog.c
index 030f5c2..583cc28 100644
--- a/src/box/lua/xlog.c
+++ b/src/box/lua/xlog.c
@@ -43,6 +43,7 @@
 #include <box/lua/tuple.h>
 #include <lua/msgpack.h>
 #include <lua/utils.h>
+#include <lua/error.h>
 #include "box/memtx_tuple.h"
 
 /* {{{ Helpers */
diff --git a/src/lua/error.c b/src/lua/error.c
new file mode 100644
index 0000000..d660e4c
--- /dev/null
+++ b/src/lua/error.c
@@ -0,0 +1,117 @@
+/*
+ * Copyright 2010-2018, 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 <diag.h>
+#include <fiber.h>
+#include "utils.h"
+#include "error.h"
+
+static int CTID_CONST_STRUCT_ERROR_REF = 0;
+
+int
+luaT_error(lua_State *L)
+{
+	struct error *e = diag_last_error(&fiber()->diag);
+	assert(e != NULL);
+	error_ref(e);
+	/*
+	 * gh-1955 luaT_pusherror allocates Lua objects, thus it may trigger
+	 * GC. GC may invoke finalizers which are arbitrary Lua code,
+	 * potentially invalidating last error object, hence error_ref
+	 * below.
+	 */
+	luaT_pusherror(L, e);
+	error_unref(e);
+	lua_error(L);
+	unreachable();
+	return 0;
+}
+
+struct error *
+luaL_iserror(struct lua_State *L, int narg)
+{
+	assert(CTID_CONST_STRUCT_ERROR_REF != 0);
+	if (lua_type(L, narg) != LUA_TCDATA)
+		return NULL;
+
+	uint32_t ctypeid;
+	void *data = luaL_checkcdata(L, narg, &ctypeid);
+	if (ctypeid != (uint32_t) CTID_CONST_STRUCT_ERROR_REF)
+		return NULL;
+
+	struct error *e = *(struct error **) data;
+	assert(e->refs);
+	return e;
+}
+
+static struct error *
+luaL_checkerror(struct lua_State *L, int narg)
+{
+	struct error *error = luaL_iserror(L, narg);
+	if (error == NULL)  {
+		luaL_error(L, "Invalid argument #%d (error expected, got %s)",
+			   narg, lua_typename(L, lua_type(L, narg)));
+	}
+	return error;
+}
+
+static int
+luaL_error_gc(struct lua_State *L)
+{
+	struct error *error = luaL_checkerror(L, 1);
+	error_unref(error);
+	return 0;
+}
+
+void
+luaT_pusherror(struct lua_State *L, struct error *e)
+{
+	assert(CTID_CONST_STRUCT_ERROR_REF != 0);
+	struct error **ptr = (struct error **)
+		luaL_pushcdata(L, CTID_CONST_STRUCT_ERROR_REF);
+	*ptr = e;
+	/* The order is important - first reference the error, then set gc */
+	error_ref(e);
+	lua_pushcfunction(L, luaL_error_gc);
+	luaL_setcdatagc(L, -2);
+}
+
+void
+tarantool_lua_error_init(struct lua_State *L)
+{
+
+	/* Get CTypeID for `struct error *' */
+	int rc = luaL_cdef(L, "struct error;");
+	assert(rc == 0);
+	(void) rc;
+	CTID_CONST_STRUCT_ERROR_REF = luaL_ctypeid(L, "const struct error &");
+	assert(CTID_CONST_STRUCT_ERROR_REF != 0);
+}
diff --git a/src/lua/error.h b/src/lua/error.h
new file mode 100644
index 0000000..ee489da
--- /dev/null
+++ b/src/lua/error.h
@@ -0,0 +1,67 @@
+#ifndef TARANTOOL_ERROR_H
+#define TARANTOOL_ERROR_H
+/*
+ * Copyright 2010-2018, 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 <lua.h>
+#include <lauxlib.h> /* luaL_error */
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+
+/** \cond public */
+struct error;
+
+/**
+ * Re-throws the last Tarantool error as a Lua object.
+ * \sa lua_error()
+ * \sa box_error_last()
+ */
+LUA_API int
+luaT_error(lua_State *L);
+
+void
+luaT_pusherror(struct lua_State *L, struct error *e);
+/** \endcond public */
+
+
+struct error *
+luaL_iserror(struct lua_State *L, int narg);
+
+void
+tarantool_lua_error_init(struct lua_State *L);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
+
+#endif //TARANTOOL_ERROR_H
diff --git a/src/lua/error.lua b/src/lua/error.lua
new file mode 100644
index 0000000..a402378
--- /dev/null
+++ b/src/lua/error.lua
@@ -0,0 +1,156 @@
+-- error.lua (internal file)
+
+local ffi = require('ffi')
+ffi.cdef[[
+enum { METHOD_ARG_MAX = 8 };
+
+struct method_info {
+    const struct type_info *owner;
+    const char *name;
+    enum ctype rtype;
+    enum ctype atype[METHOD_ARG_MAX];
+    int nargs;
+    bool isconst;
+
+    union {
+        /* Add extra space to get proper struct size in C */
+        void *_spacer[2];
+    };
+};
+
+char *
+exception_get_string(struct error *e, const struct method_info *method);
+int
+exception_get_int(struct error *e, const struct method_info *method);
+
+]]
+
+local REFLECTION_CACHE = {}
+
+local function reflection_enumerate(err)
+    local key = tostring(err._type)
+    local result = REFLECTION_CACHE[key]
+    if result ~= nil then
+        return result
+    end
+    result = {}
+    -- See type_foreach_method() in reflection.h
+    local t = err._type
+    while t ~= nil do
+        local m = t.methods
+        while m.name ~= nil do
+            result[ffi.string(m.name)] = m
+            m = m + 1
+        end
+        t = t.parent
+    end
+    REFLECTION_CACHE[key] = result
+    return result
+end
+
+local function reflection_get(err, method)
+    if method.nargs ~= 0 then
+        return nil -- NYI
+    end
+    if method.rtype == ffi.C.CTYPE_INT then
+        return tonumber(ffi.C.exception_get_int(err, method))
+    elseif method.rtype == ffi.C.CTYPE_CONST_CHAR_PTR then
+        local str = ffi.C.exception_get_string(err, method)
+        if str == nil then
+            return nil
+        end
+        return ffi.string(str)
+    end
+end
+
+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
+        return {}
+    end
+    return {
+        { file = ffi.string(err._file), line = tonumber(err._line) };
+    }
+end
+
+
+local error_fields = {
+    ["type"]        = error_type;
+    ["message"]     = error_message;
+    ["trace"]       = error_trace;
+}
+
+local function error_unpack(err)
+    if not ffi.istype('struct error', err) then
+        error("Usage: error:unpack()")
+    end
+    local result = {}
+    for key, getter in pairs(error_fields)  do
+        result[key] = getter(err)
+    end
+    for key, getter in pairs(reflection_enumerate(err)) do
+        local value = reflection_get(err, getter)
+        if value ~= nil then
+            result[key] = value
+        end
+    end
+    return result
+end
+
+local function error_raise(err)
+    if not ffi.istype('struct error', err) then
+        error("Usage: error:raise()")
+    end
+    error(err)
+end
+
+local function error_match(err, ...)
+    if not ffi.istype('struct error', err) then
+        error("Usage: error:match()")
+    end
+    return string.match(error_message(err), ...)
+end
+
+local function error_serialize(err)
+    -- Return an error message only in admin console to keep compatibility
+    return error_message(err)
+end
+
+local error_methods = {
+    ["unpack"] = error_unpack;
+    ["raise"] = error_raise;
+    ["match"] = error_match; -- Tarantool 1.6 backward compatibility
+    ["__serialize"] = error_serialize;
+}
+
+local function error_index(err, key)
+    local getter = error_fields[key]
+    if getter ~= nil then
+        return getter(err)
+    end
+    getter = reflection_enumerate(err)[key]
+    if getter ~= nil and getter.nargs == 0 then
+        local val = reflection_get(err, getter)
+        if val ~= nil then
+            return val
+        end
+    end
+    return error_methods[key]
+end
+
+
+local error_mt = {
+    __index = error_index;
+    __tostring = error_message;
+};
+
+ffi.metatype('struct error', error_mt);
+
+return error
\ No newline at end of file
diff --git a/src/lua/fiber.c b/src/lua/fiber.c
index 83b5825..7ab3808 100644
--- a/src/lua/fiber.c
+++ b/src/lua/fiber.c
@@ -31,6 +31,7 @@
 #include "lua/fiber.h"
 
 #include <fiber.h>
+#include "lua/error.h"
 #include "lua/utils.h"
 #include "backtrace.h"
 
diff --git a/src/lua/fio.c b/src/lua/fio.c
index 806f425..a58dbf3 100644
--- a/src/lua/fio.c
+++ b/src/lua/fio.c
@@ -46,6 +46,7 @@
 
 #include "lua/utils.h"
 #include "coio_file.h"
+#include "lua/error.h"
 
 static inline void
 lbox_fio_pushsyserror(struct lua_State *L)
diff --git a/src/lua/httpc.c b/src/lua/httpc.c
index 45abb98..203e929 100644
--- a/src/lua/httpc.c
+++ b/src/lua/httpc.c
@@ -40,6 +40,7 @@
 #include "lua/utils.h"
 #include "lua/httpc.h"
 #include "src/fiber.h"
+#include "lua/error.h"
 
 /** Internal util functions
  * {{{
diff --git a/src/lua/init.c b/src/lua/init.c
index a0a7f63..257c6c9 100644
--- a/src/lua/init.c
+++ b/src/lua/init.c
@@ -58,6 +58,7 @@
 #include "lua/fio.h"
 #include "lua/httpc.h"
 #include "digest.h"
+#include "error.h"
 #include <small/ibuf.h>
 
 #include <ctype.h>
@@ -95,6 +96,7 @@ extern char strict_lua[],
 	help_en_US_lua[],
 	tap_lua[],
 	fio_lua[],
+	error_lua[],
 	argparse_lua[],
 	iconv_lua[],
 	/* jit.* library */
@@ -134,6 +136,7 @@ static const char *lua_modules[] = {
 	"log", log_lua,
 	"uri", uri_lua,
 	"fio", fio_lua,
+	"error", error_lua,
 	"csv", csv_lua,
 	"clock", clock_lua,
 	"socket", socket_lua,
@@ -404,6 +407,7 @@ tarantool_lua_init(const char *tarantool_bin, int argc, char **argv)
 	tarantool_lua_fiber_cond_init(L);
 	tarantool_lua_fiber_channel_init(L);
 	tarantool_lua_errno_init(L);
+	tarantool_lua_error_init(L);
 	tarantool_lua_fio_init(L);
 	tarantool_lua_socket_init(L);
 	tarantool_lua_pickle_init(L);
diff --git a/src/lua/init.lua b/src/lua/init.lua
index 83f2c8d..5d08151 100644
--- a/src/lua/init.lua
+++ b/src/lua/init.lua
@@ -1,10 +1,10 @@
+
 -- init.lua -- internal file
 
 local ffi = require('ffi')
 ffi.cdef[[
 struct type_info;
 struct method_info;
-struct error;
 
 enum ctype {
     CTYPE_VOID = 0,
@@ -39,27 +39,6 @@ struct error {
     char _errmsg[DIAG_ERRMSG_MAX];
 };
 
-enum { METHOD_ARG_MAX = 8 };
-
-struct method_info {
-    const struct type_info *owner;
-    const char *name;
-    enum ctype rtype;
-    enum ctype atype[METHOD_ARG_MAX];
-    int nargs;
-    bool isconst;
-
-    union {
-        /* Add extra space to get proper struct size in C */
-        void *_spacer[2];
-    };
-};
-
-char *
-exception_get_string(struct error *e, const struct method_info *method);
-int
-exception_get_int(struct error *e, const struct method_info *method);
-
 double
 tarantool_uptime(void);
 typedef int32_t pid_t;
@@ -68,132 +47,6 @@ pid_t getpid(void);
 
 local fio = require("fio")
 
-local REFLECTION_CACHE = {}
-
-local function reflection_enumerate(err)
-    local key = tostring(err._type)
-    local result = REFLECTION_CACHE[key]
-    if result ~= nil then
-        return result
-    end
-    result = {}
-    -- See type_foreach_method() in reflection.h
-    local t = err._type
-    while t ~= nil do
-        local m = t.methods
-        while m.name ~= nil do
-            result[ffi.string(m.name)] = m
-            m = m + 1
-        end
-        t = t.parent
-    end
-    REFLECTION_CACHE[key] = result
-    return result
-end
-
-local function reflection_get(err, method)
-    if method.nargs ~= 0 then
-        return nil -- NYI
-    end
-    if method.rtype == ffi.C.CTYPE_INT then
-        return tonumber(ffi.C.exception_get_int(err, method))
-    elseif method.rtype == ffi.C.CTYPE_CONST_CHAR_PTR then
-        local str = ffi.C.exception_get_string(err, method)
-        if str == nil then
-            return nil
-        end
-        return ffi.string(str)
-    end
-end
-
-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
-        return {}
-    end
-    return {
-        { file = ffi.string(err._file), line = tonumber(err._line) };
-    }
-end
-
-local error_fields = {
-    ["type"]        = error_type;
-    ["message"]     = error_message;
-    ["trace"]       = error_trace;
-}
-
-local function error_unpack(err)
-    if not ffi.istype('struct error', err) then
-        error("Usage: error:unpack()")
-    end
-    local result = {}
-    for key, getter in pairs(error_fields)  do
-        result[key] = getter(err)
-    end
-    for key, getter in pairs(reflection_enumerate(err)) do
-        local value = reflection_get(err, getter)
-        if value ~= nil then
-            result[key] = value
-        end
-    end
-    return result
-end
-
-local function error_raise(err)
-    if not ffi.istype('struct error', err) then
-        error("Usage: error:raise()")
-    end
-    error(err)
-end
-
-local function error_match(err, ...)
-    if not ffi.istype('struct error', err) then
-        error("Usage: error:match()")
-    end
-    return string.match(error_message(err), ...)
-end
-
-local function error_serialize(err)
-    -- Return an error message only in admin console to keep compatibility
-    return error_message(err)
-end
-
-local error_methods = {
-    ["unpack"] = error_unpack;
-    ["raise"] = error_raise;
-    ["match"] = error_match; -- Tarantool 1.6 backward compatibility
-    ["__serialize"] = error_serialize;
-}
-
-local function error_index(err, key)
-    local getter = error_fields[key]
-    if getter ~= nil then
-        return getter(err)
-    end
-    getter = reflection_enumerate(err)[key]
-    if getter ~= nil and getter.nargs == 0 then
-        local val = reflection_get(err, getter)
-        if val ~= nil then
-            return val
-        end
-    end
-    return error_methods[key]
-end
-
-local error_mt = {
-    __index = error_index;
-    __tostring = error_message;
-};
-
-ffi.metatype('struct error', error_mt);
-
 dostring = function(s, ...)
     local chunk, message = loadstring(s)
     if chunk == nil then
@@ -343,6 +196,7 @@ table.insert(package.loaders, 5, rocks_loader_func(true))
 -- croot          8
 
 package.search = search
+
 return {
     uptime = uptime;
     pid = pid;
diff --git a/src/lua/pickle.c b/src/lua/pickle.c
index e47ac11..4238491 100644
--- a/src/lua/pickle.c
+++ b/src/lua/pickle.c
@@ -41,6 +41,7 @@
 #include "lua/msgpack.h" /* luaL_msgpack_default */
 #include <fiber.h>
 #include "bit/bit.h"
+#include "lua/error.h"
 
 static inline void
 luaL_region_dup(struct lua_State *L, struct region *region,
diff --git a/src/lua/socket.c b/src/lua/socket.c
index 14d49d1..1ed8021 100644
--- a/src/lua/socket.c
+++ b/src/lua/socket.c
@@ -53,6 +53,7 @@
 #include <coio_task.h> /* coio_getaddrinfo() */
 #include <fiber.h>
 #include "lua/utils.h"
+#include "lua/error.h"
 #include "lua/fiber.h"
 
 extern int coio_wait(int fd, int event, double timeout);
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 2094bcc..bf1548b 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -29,6 +29,7 @@
  * SUCH DAMAGE.
  */
 #include "lua/utils.h"
+#include "error.h"
 
 #include <assert.h>
 #include <errno.h>
@@ -40,7 +41,6 @@
 int luaL_nil_ref = LUA_REFNIL;
 int luaL_map_metatable_ref = LUA_REFNIL;
 int luaL_array_metatable_ref = LUA_REFNIL;
-static int CTID_CONST_STRUCT_ERROR_REF = 0;
 
 void *
 luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
@@ -843,73 +843,6 @@ luaL_toint64(struct lua_State *L, int idx)
 	return 0;
 }
 
-struct error *
-luaL_iserror(struct lua_State *L, int narg)
-{
-	assert(CTID_CONST_STRUCT_ERROR_REF != 0);
-	if (lua_type(L, narg) != LUA_TCDATA)
-		return NULL;
-
-	uint32_t ctypeid;
-	void *data = luaL_checkcdata(L, narg, &ctypeid);
-	if (ctypeid != (uint32_t) CTID_CONST_STRUCT_ERROR_REF)
-		return NULL;
-
-	struct error *e = *(struct error **) data;
-	assert(e->refs);
-	return e;
-}
-
-static struct error *
-luaL_checkerror(struct lua_State *L, int narg)
-{
-	struct error *error = luaL_iserror(L, narg);
-	if (error == NULL)  {
-		luaL_error(L, "Invalid argument #%d (error expected, got %s)",
-		   narg, lua_typename(L, lua_type(L, narg)));
-	}
-	return error;
-}
-
-static int
-luaL_error_gc(struct lua_State *L)
-{
-	struct error *error = luaL_checkerror(L, 1);
-	error_unref(error);
-	return 0;
-}
-
-void
-luaT_pusherror(struct lua_State *L, struct error *e)
-{
-	assert(CTID_CONST_STRUCT_ERROR_REF != 0);
-	struct error **ptr = (struct error **) luaL_pushcdata(L,
-		CTID_CONST_STRUCT_ERROR_REF);
-	*ptr = e;
-	/* The order is important - first reference the error, then set gc */
-	error_ref(e);
-	lua_pushcfunction(L, luaL_error_gc);
-	luaL_setcdatagc(L, -2);
-}
-
-int
-luaT_error(lua_State *L)
-{
-	struct error *e = diag_last_error(&fiber()->diag);
-	assert(e != NULL);
-	/*
-	 * gh-1955 luaT_pusherror allocates Lua objects, thus it may trigger
-	 * GC. GC may invoke finalizers which are arbitrary Lua code,
-	 * potentially invalidating last error object, hence error_ref
-	 * below.
-	 */
-	error_ref(e);
-	luaT_pusherror(L, e);
-	error_unref(e);
-	lua_error(L);
-	unreachable();
-	return 0;
-}
 
 static inline int
 lbox_catch(lua_State *L)
@@ -954,13 +887,6 @@ tarantool_lua_utils_init(struct lua_State *L)
 		{NULL, NULL},
 	};
 
-	/* Get CTypeID for `struct error *' */
-	int rc = luaL_cdef(L, "struct error;");
-	assert(rc == 0);
-	(void) rc;
-	CTID_CONST_STRUCT_ERROR_REF = luaL_ctypeid(L, "const struct error &");
-	assert(CTID_CONST_STRUCT_ERROR_REF != 0);
-
 	luaL_register_type(L, LUAL_SERIALIZER, serializermeta);
 	/* Create NULL constant */
 	*(void **) luaL_pushcdata(L, CTID_P_VOID) = NULL;
@@ -984,4 +910,3 @@ tarantool_lua_utils_init(struct lua_State *L)
 
 	return 0;
 }
-
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 77c2204..02ce93b 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -55,7 +55,6 @@ extern "C" {
 
 struct lua_State;
 struct ibuf;
-struct error;
 
 /**
  * Single global lua_State shared by core and modules.
@@ -403,14 +402,6 @@ LUA_API int64_t
 luaL_toint64(struct lua_State *L, int idx);
 
 /**
- * Re-throws the last Tarantool error as a Lua object.
- * \sa lua_error()
- * \sa box_error_last()
- */
-LUA_API int
-luaT_error(lua_State *L);
-
-/**
  * Like lua_call(), but with the proper support of Tarantool errors.
  * \sa lua_call()
  */
@@ -432,11 +423,6 @@ luaT_state(void);
 
 /** \endcond public */
 
-void
-luaT_pusherror(struct lua_State *L, struct error *e);
-
-struct error *
-luaL_iserror(struct lua_State *L, int narg);
 
 /**
  * Push Lua Table with __serialize = 'map' hint onto the stack.
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] [error 2/3] error: Add lua traceback
  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
  2018-05-04 14:07   ` [tarantool-patches] [error 3/3] error: Add C frames in error.traceback Ilya Markov
  2 siblings, 0 replies; 4+ messages in thread
From: Ilya Markov @ 2018-05-04 14:07 UTC (permalink / raw)
  To: georgy; +Cc: tarantool-patches

* 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [tarantool-patches] [error 3/3] error: Add C frames in error.traceback
  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   ` [tarantool-patches] [error 2/3] error: Add lua traceback Ilya Markov
@ 2018-05-04 14:07   ` Ilya Markov
  2 siblings, 0 replies; 4+ messages in thread
From: Ilya Markov @ 2018-05-04 14:07 UTC (permalink / raw)
  To: georgy; +Cc: tarantool-patches

* Add backtrace generation in exception contructor.
* Add merge of lua and C traces.
* Modify C backtrace with support of current fiber stack.

Relates #677
---
 src/backtrace.cc                  | 10 ++++-
 src/exception.cc                  | 49 +++++++++++++++++++++
 src/lua/error.c                   | 92 +++++++++++++++++++++++++++++++++++++--
 test/app/traceback.result         | 45 ++++++++++++++++---
 test/app/traceback.test.lua       | 14 +++++-
 test/box/misc.result              | 26 ++++++++---
 test/replication/wal_off.result   |  2 +-
 test/replication/wal_off.test.lua |  2 +-
 8 files changed, 220 insertions(+), 20 deletions(-)

diff --git a/src/backtrace.cc b/src/backtrace.cc
index a86255e..c0c778e 100644
--- a/src/backtrace.cc
+++ b/src/backtrace.cc
@@ -368,7 +368,15 @@ backtrace_foreach(backtrace_cb cb, coro_context *coro_ctx, void *cb_ctx)
 {
 	unw_cursor_t unw_cur;
 	unw_context_t unw_ctx;
-	coro_unwcontext(&unw_ctx, coro_ctx);
+	if (coro_ctx == &fiber()->ctx) {
+		/*
+		 * We don't have to move over stack as we are inspecting
+		 * current fiber.
+		 */
+		unw_getcontext(&unw_ctx);
+	} else {
+		coro_unwcontext(&unw_ctx, coro_ctx);
+	}
 	unw_init_local(&unw_cur, &unw_ctx);
 	int frame_no = 0;
 	unw_word_t sp = 0, old_sp = 0, ip, offset;
diff --git a/src/exception.cc b/src/exception.cc
index 56077f7..d6b36ab 100644
--- a/src/exception.cc
+++ b/src/exception.cc
@@ -36,6 +36,7 @@
 
 #include "fiber.h"
 #include "reflection.h"
+#include "backtrace.h"
 
 extern "C" {
 
@@ -93,6 +94,7 @@ static const struct method_info exception_methods[] = {
 const struct type_info type_Exception = make_type("Exception", NULL,
 	exception_methods);
 
+
 void *
 Exception::operator new(size_t size)
 {
@@ -114,6 +116,44 @@ Exception::~Exception()
 	if (this != &out_of_memory) {
 		assert(refs == 0);
 	}
+	if (this->frames_count > 0) {
+		struct diag_frame *frame, *next;
+		/* Cleanup lua trace. */
+		for (frame = rlist_first_entry(&this->frames, typeof(*frame),
+					       link);
+		     &frame->link != &this->frames;) {
+			next = rlist_next_entry(frame, link);
+			free(frame);
+			frame = next;
+		}
+	}
+}
+
+static int
+error_backtrace_cb(int frameno, void *frameret, const char *func,
+		   size_t offset, void *cb_ctx)
+{
+	(void) frameno;
+	(void) frameret;
+	(void) offset;
+	Exception *e = (Exception *) cb_ctx;
+
+	struct diag_frame * frame =
+		(struct diag_frame *) malloc(sizeof(*frame));
+	if (frame == NULL) {
+		diag_set(OutOfMemory, sizeof(struct diag_frame),
+			 "malloc", "struct diag_frame");
+		return -1;
+	}
+	if (func)
+		snprintf(frame->func_name, sizeof(frame->func_name), "%s", func);
+	else
+		frame->func_name[0] = 0;
+	frame->filename[0] = 0;
+	frame->line = 0;
+	rlist_add_tail_entry(&e->frames, frame, link);
+	e->frames_count++;
+	return 0;
 }
 
 Exception::Exception(const struct type_info *type_arg, const char *file,
@@ -121,6 +161,15 @@ Exception::Exception(const struct type_info *type_arg, const char *file,
 {
 	error_create(this, exception_destroy, exception_raise,
 		     exception_log, type_arg, file, line);
+	int old_errno = errno;
+	if (cord()) {
+		backtrace_foreach(error_backtrace_cb, &fiber()->ctx, this);
+		if (this->frames_count > 0) {
+			rlist_shift_tail(&this->frames);
+			this->frames_count--;
+		}
+	}
+	errno = old_errno;
 }
 
 void
diff --git a/src/lua/error.c b/src/lua/error.c
index e6ccd32..3e667a5 100644
--- a/src/lua/error.c
+++ b/src/lua/error.c
@@ -31,6 +31,7 @@
 
 #include <diag.h>
 #include <fiber.h>
+#include <backtrace.h>
 #include "utils.h"
 #include "error.h"
 
@@ -104,15 +105,36 @@ luaT_pusherror(struct lua_State *L, struct error *e)
 	luaL_setcdatagc(L, -2);
 }
 
+static inline void
+copy_frame(struct diag_frame *dest, struct diag_frame *src)
+{
+	dest->line = src->line;
+	strcpy(dest->func_name, src->func_name);
+	strcpy(dest->filename, src->filename);
+}
+
 static int
 traceback_error(struct lua_State *L, struct error *e)
 {
 	lua_Debug ar;
 	int level = 0;
+	struct rlist lua_frames;
+	struct rlist *lua_framesp;
+	struct diag_frame *frame, *next;
+	if (e->frames_count <= 0) {
+		lua_framesp = &e->frames;
+	} else {
+		lua_framesp = &lua_frames;
+		rlist_create(lua_framesp);
+	}
+	/*
+	 * At this moment error object was created from exception so
+	 * traceback list is already created and filled with C trace.
+	 * Now we need to create lua trace and merge it with the existing one.
+	 */
 	while (lua_getstack(L, level++, &ar) > 0) {
 		lua_getinfo(L, "Sln", &ar);
-		struct diag_frame *frame =
-				(struct diag_frame *) malloc(sizeof(*frame));
+		frame = (struct diag_frame *) malloc(sizeof(*frame));
 		if (frame == NULL) {
 			luaT_pusherror(L, e);
 			return 1;
@@ -136,7 +158,67 @@ traceback_error(struct lua_State *L, struct error *e)
 			frame->line = 0;
 			e->frames_count++;
 		}
-		rlist_add_entry(&e->frames, frame, link);
+		rlist_add_entry(lua_framesp, frame, link);
+	}
+	if (e->frames_count > 0) {
+		struct diag_frame *lua_frame = rlist_first_entry(lua_framesp,
+							 typeof(*lua_frame),
+							 link);
+		rlist_foreach_entry(frame, &e->frames, link) {
+			/* We insert trace of lua user code in c trace,
+			 * where C calls lua.*/
+			if (strncmp(frame->func_name, "lj_BC_FUNCC",
+				    sizeof("lj_BC_FUNCC") - 1) == 0 &&
+			    frame->line != -1) {
+				/* We have to bypass internal error calls. */
+				next = rlist_next_entry(frame, link);
+				if (strncmp(next->func_name, "lj_err_run",
+					    sizeof("lj_err_run") - 1) == 0)
+					continue;
+				e->frames_count--;
+				for (;&lua_frame->link != lua_framesp;
+				      lua_frame = rlist_next_entry(lua_frame,
+								    link)) {
+					/* Skip empty frames. */
+					if (strncmp(lua_frame->filename,
+						    "[C]",
+						    sizeof("[C]") - 1) == 0 &&
+					    (*lua_frame->func_name) == '?')
+						continue;
+					break;
+				}
+
+				for (; &lua_frame->link != lua_framesp;
+				       lua_frame = rlist_next_entry(lua_frame,
+								    link)) {
+					if (lua_frame->filename[0]== 0 &&
+					    lua_frame->func_name[0] == 0)
+						break;
+					struct diag_frame *frame_copy =
+						(struct diag_frame *)
+						    malloc(sizeof(*frame_copy));
+					copy_frame(frame_copy, lua_frame);
+					rlist_add_entry(&frame->link,
+							frame_copy, link);
+					e->frames_count++;
+					/*
+					 * Mark the C frame that it was replaced
+					 * with lua.
+					 */
+					frame->line = -1;
+				}
+			}
+		}
+	}
+	if (&e->frames != lua_framesp) {
+		/* Cleanup lua trace. */
+		for (frame = rlist_first_entry(lua_framesp, typeof(*frame),
+					       link);
+		     &frame->link != lua_framesp;) {
+			next = rlist_next_entry(frame, link);
+			free(frame);
+			frame = next;
+		}
 	}
 	luaT_pusherror(L, e);
 	return 1;
@@ -174,6 +256,10 @@ lua_error_gettraceback(struct lua_State *L)
 	rlist_foreach_entry(frame, &e->frames, link) {
 		if (frame->func_name[0] != 0 || frame->line > 0 ||
 		    frame->filename[0] != 0) {
+			if (strncmp(frame->func_name, "lj_BC_FUNCC",
+				    sizeof("lj_BC_FUNCC") - 1) == 0 &&
+			    frame->line == -1)
+				continue;
 			/* push index */
 			lua_pushnumber(L, index++);
 			/* push value - table of filename and line */
diff --git a/test/app/traceback.result b/test/app/traceback.result
index bb54573..85576bf 100644
--- a/test/app/traceback.result
+++ b/test/app/traceback.result
@@ -5,17 +5,17 @@ err.type
 ---
 - ClientError
 ...
-err.trace[1].file
+err.trace[12].file
 ---
 - builtin/socket.lua
 ...
-err.trace[1].line
+err.trace[12].line
 ---
 - 997
 ...
 #err.trace
 ---
-- 8
+- 30
 ...
 s, err = pcall(error, "oh no" )
 ---
@@ -33,7 +33,7 @@ err.message
 ...
 #err.trace
 ---
-- 8
+- 33
 ...
 nil_var=nil
 ---
@@ -61,7 +61,7 @@ err.message
 ...
 #err.trace
 ---
-- 10
+- 36
 ...
 box.begin()
 ---
@@ -82,7 +82,38 @@ err.message
 ...
 #err.trace
 ---
-- 10
+- 39
+...
+s = box.schema.space.create("space")
+---
+...
+_ = s:create_index("prim")
+---
+...
+_ =s:on_replace(fail)
+---
+...
+st,err = pcall(s.insert, s, {1})
+---
+...
+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
+---
+- 45
+...
+s:drop()
+---
 ...
 errinj = box.error.injection
 ---
@@ -117,7 +148,7 @@ err.message
 ...
 #err.trace
 ---
-- 8
+- 32
 ...
 space:drop()
 ---
diff --git a/test/app/traceback.test.lua b/test/app/traceback.test.lua
index 51af677..1034049 100644
--- a/test/app/traceback.test.lua
+++ b/test/app/traceback.test.lua
@@ -1,7 +1,7 @@
 s, err = pcall(box.error, 1, "err")
 err.type
-err.trace[1].file
-err.trace[1].line
+err.trace[12].file
+err.trace[12].line
 #err.trace
 
 s, err = pcall(error, "oh no" )
@@ -27,6 +27,16 @@ err.type
 err.message
 #err.trace
 
+s = box.schema.space.create("space")
+_ = s:create_index("prim")
+_ =s:on_replace(fail)
+st,err = pcall(s.insert, s, {1})
+err= err:unpack()
+err.type
+err.message
+#err.trace
+s:drop()
+
 errinj = box.error.injection
 space = box.schema.space.create('tweedledum')
 index = space:create_index('primary', { type = 'hash' })
diff --git a/test/box/misc.result b/test/box/misc.result
index a5cad07..0a3c445 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -130,42 +130,58 @@ e:unpack()
   code: 1
   message: Illegal parameters, bla bla
   trace:
-  - file: builtin/socket.lua
-    line: <number>
+  - function: ClientError::ClientError(char const*, unsigned int, unsigned int, ...)
+  - function: BuildClientError
+  - function: box_error_set
+  - function: luaT_error_raise(lua_State*)
   - function: pcall
   - file: builtin/box/console.lua
+    function: eval
     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: lua_pcall
+  - function: luaT_call
+  - function: luaB_pcall
   - function: pcall
   - file: builtin/box/console.lua
+    function: eval
     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: lua_pcall
+  - function: luaT_call
+  - function: luaB_pcall
   - function: pcall
   - file: builtin/box/console.lua
+    function: eval
     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: lua_pcall
+  - function: luaT_call
+  - function: lua_fiber_run_f
+  - function: fiber_cxx_invoke(int (*)(__va_list_tag*), __va_list_tag*)
+  - function: fiber_loop
+  - function: coro_init
 ...
 e.type
 ---
diff --git a/test/replication/wal_off.result b/test/replication/wal_off.result
index f50bfc2..451c8de 100644
--- a/test/replication/wal_off.result
+++ b/test/replication/wal_off.result
@@ -84,7 +84,7 @@ box.cfg { replication = wal_off_uri }
 check = "Read access to universe"
 ---
 ...
-while string.find(box.info.replication[wal_off_id].upstream.message, check) == nil do fiber.sleep(0.01) end
+while box.info.replication[wal_off_id].upstream.message == nil or string.find(box.info.replication[wal_off_id].upstream.message, check) == nil do fiber.sleep(0.01) end
 ---
 ...
 box.cfg { replication = "" }
diff --git a/test/replication/wal_off.test.lua b/test/replication/wal_off.test.lua
index 43cd0ae..9a5e02d 100644
--- a/test/replication/wal_off.test.lua
+++ b/test/replication/wal_off.test.lua
@@ -29,7 +29,7 @@ test_run:cmd('switch default')
 
 box.cfg { replication = wal_off_uri }
 check = "Read access to universe"
-while string.find(box.info.replication[wal_off_id].upstream.message, check) == nil do fiber.sleep(0.01) end
+while box.info.replication[wal_off_id].upstream.message == nil or string.find(box.info.replication[wal_off_id].upstream.message, check) == nil do fiber.sleep(0.01) end
 box.cfg { replication = "" }
 
 test_run:cmd("stop server wal_off")
-- 
2.7.4

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-05-04 14:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [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   ` [tarantool-patches] [error 2/3] error: Add lua traceback Ilya Markov
2018-05-04 14:07   ` [tarantool-patches] [error 3/3] error: Add C frames in error.traceback Ilya Markov

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