Skip to content

Commit c52aa69

Browse files
committed
server: fix assertion error serialization in exec()
Commit e6a2093 ("Add error handling at the server instance") broke assertion failure reporting in `server:exec()` in case verbose error serialization is enabled in Tarantool (`box_error_serialize_verbose` compatibility option is set to `new`). The problem is with the verbose error serialization, a raw string raised with `error()` is converted to an error object when marshalled through IPROTO while the `server:exec()` code expects it to remain a string because it encodes the original error in JSON. As a result, we get a mangled error like ``` {"status":"fail","class":"LuatestError","message":"...some_test.lua:42: expected: a value evaluating to true, actual: false"} {"code":32,"type":"LuajitError","trace":[{"file":"./src/lua/utils.c","line":687}]} ``` instead of ``` ...some_test.lua:42: expected: a value evaluating to true, actual: false ``` To fix this issue, we revert the aforementioned commit. In order not to reintroduce #242, we simply rollback the active transaction (if any) in case the executed function failed. Closes #376
1 parent 3a78617 commit c52aa69

File tree

3 files changed

+34
-17
lines changed

3 files changed

+34
-17
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
- Add support for declarative configuration to `server.lua` (gh-367).
1818
- Make `assert_covers` recursive (gh-379).
1919
- Add alias `--no-capture` for the option `-c` (gh-391).
20+
- Fix reporting of an assertion failure in `Server:exec()` in case verbose
21+
error serialization is enabled in Tarantool (gh-376).
2022

2123
## 1.0.1
2224

luatest/server.lua

+8-17
Original file line numberDiff line numberDiff line change
@@ -742,8 +742,7 @@ end
742742

743743
local function exec_tail(ok, ...)
744744
if not ok then
745-
local _ok, res = pcall(json.decode, tostring(...))
746-
error(_ok and res or ..., 0)
745+
error(..., 0)
747746
else
748747
return ...
749748
end
@@ -824,16 +823,11 @@ function Server:exec(fn, args, options)
824823
error(('bad argument #3 for exec at %s: an array is required'):format(utils.get_fn_location(fn)))
825824
end
826825

827-
-- The function `fn` can return multiple values and we cannot use the
828-
-- classical approach to work with the `pcall`:
829-
--
830-
-- local status, result = pcall(function() return 1, 2, 3 end)
831-
--
832-
-- `result` variable will contain only `1` value, not `1, 2, 3`.
833-
-- To solve this, we put everything from `pcall` in a table.
834-
-- Table must be unpacked with `unpack(result, i, table.maxn(result))`,
835-
-- otherwise nil return values won't be supported.
836-
return exec_tail(pcall(self.net_box.eval, self.net_box, [[
826+
-- If the function fails an assertion in an open transaction, Tarantool
827+
-- will raise the "Transaction is active at return from function" error,
828+
-- thus overwriting the original error raised by the assertion. To avoid
829+
-- that, let's rollback the active transaction on failure.
830+
return exec_tail(self.net_box:eval([[
837831
local dump, args, passthrough_ups = ...
838832
local fn = loadstring(dump)
839833
for i = 1, debug.getinfo(fn, 'u').nups do
@@ -849,12 +843,9 @@ function Server:exec(fn, args, options)
849843
result = {pcall(fn, unpack(args))}
850844
end
851845
if not result[1] then
852-
if type(result[2]) == 'table' then
853-
result[2] = require('json').encode(result[2])
854-
end
855-
error(result[2], 0)
846+
box.rollback()
856847
end
857-
return unpack(result, 2, table.maxn(result))
848+
return unpack(result)
858849
]], {string.dump(fn), args, passthrough_ups}, options))
859850
end
860851

test/server_test.lua

+24
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ local t = require('luatest')
66
local g = t.group()
77
local utils = require('luatest.utils')
88

9+
local helper = require('test.helpers.general')
10+
911
local Process = t.Process
1012
local Server = t.Server
1113

@@ -17,6 +19,9 @@ local server = Server:new({
1719
command = command,
1820
workdir = fio.pathjoin(datadir, 'common'),
1921
env = {
22+
LUA_PATH = root .. '/?.lua;' ..
23+
root .. '/?/init.lua;' ..
24+
root .. '/.rocks/share/tarantool/?.lua',
2025
TARANTOOL_LOG = fio.pathjoin(datadir, 'server_test.log'),
2126
custom_env = 'test_value',
2227
},
@@ -563,3 +568,22 @@ g.test_grep_log = function()
563568
server.net_box:close()
564569
server.net_box = nil
565570
end
571+
572+
g.before_test('test_assertion_failure', function()
573+
-- The compat module option may be unavailable.
574+
pcall(function()
575+
local compat = require('compat')
576+
compat.box_error_serialize_verbose = 'new'
577+
end)
578+
end)
579+
580+
g.after_test('test_assertion_failure', function()
581+
pcall(function()
582+
require('compat').box_error_serialize_verbose = 'default'
583+
end)
584+
end)
585+
586+
g.test_assertion_failure = function()
587+
server:connect_net_box()
588+
helper.assert_failure(server.exec, server, function() t.assert(false) end)
589+
end

0 commit comments

Comments
 (0)