Skip to content

Commit

Permalink
server: fix assertion error serialization in exec()
Browse files Browse the repository at this point in the history
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 JSON encoding hack introduced by the
aforementioned commit. In order not to reintroduce issue #242 fixed by
that commit, we simply rollback the active transaction (if any) in case
the executed function failed.

Closes #376
  • Loading branch information
locker committed Oct 17, 2024
1 parent 3a78617 commit 3ad7fae
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 17 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
- Add support for declarative configuration to `server.lua` (gh-367).
- Make `assert_covers` recursive (gh-379).
- Add alias `--no-capture` for the option `-c` (gh-391).
- Fix reporting of an assertion failure in `Server:exec()` in case verbose
error serialization is enabled in Tarantool (gh-376).

## 1.0.1

Expand Down
25 changes: 8 additions & 17 deletions luatest/server.lua
Original file line number Diff line number Diff line change
Expand Up @@ -742,8 +742,7 @@ end

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

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

Expand Down
24 changes: 24 additions & 0 deletions test/server_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ local t = require('luatest')
local g = t.group()
local utils = require('luatest.utils')

local helper = require('test.helpers.general')

local Process = t.Process
local Server = t.Server

Expand All @@ -17,6 +19,9 @@ local server = Server:new({
command = command,
workdir = fio.pathjoin(datadir, 'common'),
env = {
LUA_PATH = root .. '/?.lua;' ..
root .. '/?/init.lua;' ..
root .. '/.rocks/share/tarantool/?.lua',
TARANTOOL_LOG = fio.pathjoin(datadir, 'server_test.log'),
custom_env = 'test_value',
},
Expand Down Expand Up @@ -563,3 +568,22 @@ g.test_grep_log = function()
server.net_box:close()
server.net_box = nil
end

g.before_test('test_assertion_failure', function()
-- The compat module option may be unavailable.
pcall(function()
local compat = require('compat')
compat.box_error_serialize_verbose = 'new'
end)
end)

g.after_test('test_assertion_failure', function()
pcall(function()
require('compat').box_error_serialize_verbose = 'default'
end)
end)

g.test_assertion_failure = function()
server:connect_net_box()
helper.assert_failure(server.exec, server, function() t.assert(false) end)
end

0 comments on commit 3ad7fae

Please sign in to comment.