diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua index f3b645c983335b1bd3a34d1551bf21c5b4f04aaa..ed5222f365cb8b83a4f9aeff9999ef72e8d7ac3b 100644 --- a/src/box/lua/console.lua +++ b/src/box/lua/console.lua @@ -29,6 +29,7 @@ local yaml = require('yaml') local net_box = require('net.box') local help = require('help').help local utils = require('internal.utils') +local tarantool = require('tarantool') local DEFAULT_CONNECT_TIMEOUT = 10 local PUSH_TAG_HANDLE = '!push!' @@ -414,6 +415,59 @@ local function get_command(line) return nil end +-- +-- Create wrapper for a function. +-- +-- Without a wrapper function will be called using xpcall and the box.error +-- trace frame will be at pcall implementation. Thus the trace test will be not +-- strict enough (it can be any pcall beneath console pcall). +-- +local function wrapped_call(fn) + return function() + -- With `return fn()` wrapper does not work due to tail call + -- optimization. + local res = utils.table_pack(fn()) + return unpack(res, 1, res.n) + end +end + +-- Calculates trace pointing to the fn() in wrapped_call. +local function get_wrapped_trace() + local fun = wrapped_call(function() + box.error(box.error.UNKNOWN, 2) + end) + local _, err = pcall(fun) + return err.trace[1] +end + +-- Calculates trace pointing to the fn() in wrapped_call. +local wrapped_trace = get_wrapped_trace() +assert(wrapped_trace.file == 'builtin/box/console.lua') + +-- +-- This function is intended to be used with xpcall and wrapped_call. +-- Returns {err, info} where `info` is debug info for frame calling function +-- wrapped in wrapped_call and `err` is just original error. +-- +-- `info` is used to find the module that raises an error. +-- +local function eval_error_handler(err) + local level = 2 + local info = debug.getinfo(level, 'Sl') + -- Find place of xpcall. + while info ~= nil do + if info.short_src == wrapped_trace.file and + info.currentline == wrapped_trace.line then + break + end + level = level + 1 + info = debug.getinfo(level, 'Sl') + end + -- Corresponds to the frame of function executed in console. + info = debug.getinfo(level - 1, 'S') + return {err = err, info = info} +end + local initial_env -- Get initial console environment. @@ -511,9 +565,11 @@ local function local_eval(storage, line) if not fun then return format(false, errmsg) end + -- Wrapper is required to check error trace. See below. + fun = wrapped_call(fun) -- box.is_in_txn() is stubbed to throw a error before call to box.cfg{} local in_txn_before = ffi.C.box_txn() - local res = utils.table_pack(pcall(fun)) + local res = utils.table_pack(xpcall(fun, eval_error_handler)) -- -- Rollback if transaction was began in the failed expression. -- @@ -525,6 +581,37 @@ local function local_eval(storage, line) if not res[1] and not in_txn_before and ffi.C.box_txn() then box.rollback() end + -- + -- In case of errors we set box.error trace frame to the place of box API + -- call by client (gh-9914). This should be tested. The idea is to use + -- existing diff tests that use console for test script execution. Let + -- check if trace frame is correct. + -- + -- It is hard to fix box.error trace for all API at once so we will fix it + -- per module basis. + -- + if tarantool.build.test_build and not res[1] and + tarantool._internal.trace_check_is_required(res[2].info.short_src) then + local err = res[2].err + if not box.error.is(err) then + return format(false, {err, 'Warning, box error expected'}) + end + local trace = err.trace[1] + if not table.equals(trace, wrapped_trace) then + return format(false, {err, 'Warning, unexpected trace', trace}) + end + end + if not res[1] then + local err = res[2].err + -- Unfortunately due to introduced wrapper (see above) if module + -- raises non box error then trace information apperead breaking + -- existing tests. Let's temporarily counteract by stripping + -- this information. + if type(err) == 'string' then + err = string.gsub(err, '^builtin/box/console.lua:%d+: ', '') + end + res[2] = err + end -- specify length to preserve trailing nils return format(unpack(res, 1, res.n)) end diff --git a/src/lua/init.lua b/src/lua/init.lua index f116046e318b450ebb36bf303910c6ae56a34990..70ddd75150e2fe82a01ccbc67c4c11091ca59e5c 100644 --- a/src/lua/init.lua +++ b/src/lua/init.lua @@ -194,6 +194,43 @@ local function run_preload() end end +-- +-- List of modules (by path) which are required to use box.error and +-- set trace properly. +-- +local trace_check_required_modules = { + ['builtin/box/schema.lua'] = true, + ['builtin/box/session.lua'] = true, + ['builtin/digest.lua'] = true, + ['builtin/error.lua'] = true, + ['builtin/tarantool.lua']= true, +} + +-- +-- Return true if module with given `path` is required to use box.error +-- to indicate error and error trace should be set the module function +-- invocation place. +-- +local function trace_check_is_required(path) + return trace_check_required_modules[path] ~= nil +end + +-- +-- Raise box error with trace not pointing to the place of this function +-- invocation. +-- +local function raise_incorrect_trace() + box.error(box.error.UNKNOWN, 1) +end + +-- +-- This module is expected to raise box errors only. Raise non box error +-- for testing purpuses. +-- +local function raise_non_box_error() + error('foo bar') +end + -- Extract all fields from a table except ones that start from -- the underscore. -- @@ -219,6 +256,9 @@ local tarantool_lua = { strip_cwd_from_path = strip_cwd_from_path, module_name_from_filename = module_name_from_filename, run_preload = run_preload, + trace_check_is_required = trace_check_is_required, + raise_incorrect_trace = raise_incorrect_trace, + raise_non_box_error = raise_non_box_error, }, } -- tarantool module is already registered by src/lua/init.c diff --git a/test/app-luatest/console_check_trace_test.lua b/test/app-luatest/console_check_trace_test.lua new file mode 100644 index 0000000000000000000000000000000000000000..a96b038ee970457e662c0b4bc1227aca7aa0a912 --- /dev/null +++ b/test/app-luatest/console_check_trace_test.lua @@ -0,0 +1,36 @@ +local console = require('console') +local yaml = require('yaml') +local tarantool = require('tarantool') + +local t = require('luatest') + +local g = t.group() + +g.test_error_check_trace = function() + t.skip_if(not tarantool.build.test_build, 'requires test build') + local err = yaml.decode(console.eval([[ + require('tarantool')._internal.raise_incorrect_trace() + ]])) + t.assert_type(err, 'table') + t.assert_type(err[1], 'table') + t.assert_type(err[1].error, 'table') + err = err[1].error + t.assert_type(err[3], 'table') + t.assert_type(err[3].line, 'number') + err[3].line = nil + t.assert_equals(err, { + 'Unknown error', + 'Warning, unexpected trace', + {file = 'builtin/tarantool.lua'}, + }) + + local err = yaml.decode(console.eval([[ + require('tarantool')._internal.raise_non_box_error() + ]])) + t.assert_type(err, 'table') + t.assert_type(err[1], 'table') + t.assert_type(err[1].error, 'table') + err = err[1].error + t.assert_str_matches(err[1], 'builtin/tarantool.lua:%d+: foo bar') + t.assert_equals(err[2], 'Warning, box error expected') +end diff --git a/test/box/error.result b/test/box/error.result index 369e79b297a93b3109dc111406d9c205a38de926..74549a41555aa56561ea1c4c2e6d18f26bce1173 100644 --- a/test/box/error.result +++ b/test/box/error.result @@ -32,7 +32,13 @@ e | --- | - foo | ... -e:unpack() +u = e:unpack() + | --- + | ... +u.trace[1].line = nil + | --- + | ... +u | --- | - name: ILLEGAL_PARAMS | code: 0 @@ -40,8 +46,7 @@ e:unpack() | type: IllegalParams | message: foo | trace: - | - file: '[C]' - | line: 4294967295 + | - file: builtin/box/console.lua | ... e.type | --- diff --git a/test/box/error.test.lua b/test/box/error.test.lua index 27064994a9e0beceb8111b5a2c12d85f94c1ea4c..2486fbcaf6bcddb05dd056ea69819ff5304fd906 100644 --- a/test/box/error.test.lua +++ b/test/box/error.test.lua @@ -9,7 +9,9 @@ box.error(box.error.ILLEGAL_PARAMS, "foo") box.error() e = box.error.last() e -e:unpack() +u = e:unpack() +u.trace[1].line = nil +u e.type e.code e.message