diff --git a/changelogs/unreleased/gh-7288-txn-and-eval-error.md b/changelogs/unreleased/gh-7288-txn-and-eval-error.md new file mode 100644 index 0000000000000000000000000000000000000000..338c8c2f266b1e1216d35cfd2671e5674d10fa00 --- /dev/null +++ b/changelogs/unreleased/gh-7288-txn-and-eval-error.md @@ -0,0 +1,4 @@ +## bugfix/core + +* Fixed expression evaluation error overwritten by "Transaction is active at + return from function" error in case expression begins a transaction (gh-7288). diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua index 0f283c13e0ab56cdb27d541b1e15224ba975da67..b306f321864a7c845267322345fb5e80bf3c06a7 100644 --- a/src/box/lua/console.lua +++ b/src/box/lua/console.lua @@ -372,6 +372,13 @@ local function get_command(line) return nil end +-- +-- return args as table with 'n' set to args number +-- +local function table_pack(...) + return {n = select('#', ...), ...} +end + -- -- Evaluate command on local instance -- @@ -400,7 +407,22 @@ local function local_eval(storage, line) if not fun then return format(false, errmsg) end - return format(pcall(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 = table_pack(pcall(fun)) + -- + -- Rollback if transaction was began in the failed expression. + -- + -- In case of remote binary console if eval leaves an active transaction + -- then later a error is thrown overwriting error thrown by eval thus the + -- rollback. Also the check if we were already in transaction allows for + -- local and remote text console to preserve interactive transactions. + -- + if not res[1] and not in_txn_before and ffi.C.box_txn() then + box.rollback() + end + -- specify length to preserve trailing nils + return format(unpack(res, 1, res.n)) end local function eval(line) diff --git a/test/app-luatest/gh_7288_console_flavours_vs_txn_test.lua b/test/app-luatest/gh_7288_console_flavours_vs_txn_test.lua new file mode 100644 index 0000000000000000000000000000000000000000..2fb60e7d5c648b5c1b7efdd704afd69e4ab0f734 --- /dev/null +++ b/test/app-luatest/gh_7288_console_flavours_vs_txn_test.lua @@ -0,0 +1,184 @@ +local server = require('test.luatest_helpers.server') +local console = require('console') +local fiber = require('fiber') +local fio = require('fio') +local string = require('string') +local t = require('luatest') + +local function configure_box() + local workdir = fio.pathjoin(server.vardir, 'gh-7288') + fio.rmtree(workdir) + fio.mkdir(workdir) + box.cfg{ + memtx_use_mvcc_engine = true, + work_dir = workdir, + log = fio.pathjoin(workdir, 'self.log'), + } +end + +local remote_mt = { + start = function(self) + local server = server:new{ + alias = 'default', + box_cfg = {memtx_use_mvcc_engine = true}, + } + server:start() + if not self.bin then + self.uri = fio.pathjoin(server.vardir, 'gh-7288.sock') + server:eval(string.format('require("console").listen("%s")', self.uri)) + else + self.uri = server.net_box_uri + end + self.server = server + end, + + stop = function(self) + self.server:stop() + self.server = nil + self.uri = nil + end, + + connect = function(self, console) + console:send(string.format('require("console").connect("%s")', self.uri)) + end, + + disconnect = true, +} + +local flavours = { + alocal = {}, + remote_txt = setmetatable({bin = false}, {__index = remote_mt}), + remote_bin = setmetatable({bin = true}, {__index = remote_mt}), +} + +local g = t.group('gh-7288', { + {name = 'alocal'}, + {name = 'remote_txt'}, + {name = 'remote_bin'}, +}) + + +-- +-- Mock read/print methods to run a console. Without running console we can't +-- call console.connect to test remote consoles. +-- +local TestConsole = {} + +TestConsole.new = function(self, flavour) + local o = { flavour = flavour } + setmetatable(o, self) + return o +end + +TestConsole.__index = {} +TestConsole.__index.start = function(self) + if self.flavour.start then + self.flavour:start() + end +end + +TestConsole.__index.stop = function(self) + if self.flavour.stop then + self.flavour:stop() + end +end + +TestConsole.__index.connect = function(self) + self.ich = fiber.channel(10) + self.och = fiber.channel(10) + local on_start = fiber.channel() + console.on_start(function(console) + console.read = function(_) + return self.ich:get() + end + console.print = function(_, output) + self.och:put(output) + end + on_start:put(true) + end) + fiber.create(function() + console.start() + -- write console fiber exit message + self.och:put(true) + end) + assert(on_start:get(3)) + if self.flavour.connect then + self.flavour:connect(self) + end +end + +TestConsole.__index.send = function(self, input) + self.ich:put(input) + return assert(self.och:get(3)) +end + +TestConsole.__index.disconnect = function(self) + if self.flavour.disconnect then + self:send(nil) + end + self.ich:close() + -- read console fiber exit message + assert(self.och:get(3)) + self.och:close() + self.ich = nil + self.och = nil + console.on_start(nil) +end + +configure_box() + +g.before_all(function(cg) + cg.console = TestConsole:new(flavours[cg.params.name]) + cg.console:start() +end) + +g.after_all(function(cg) + cg.console:stop() + cg.console = nil +end) + +g.before_each(function(cg) + cg.console:connect() +end) + +g.after_each(function(cg) + cg.console:disconnect() +end) + +local true_output = [[ +--- +- true +... +]] + +local false_output = [[ +--- +- false +... +]] + +g.test_begin_in_expr_without_error = function(cg) + t.xfail_if(cg.params.name == "remote_bin") + local console = cg.console + console:send('box.begin()') + t.assert_equals(console:send('box.is_in_txn()'), true_output) +end + +g.test_begin_in_expr_with_error = function(cg) + local console = cg.console + local expected = [[ +--- +- error: '[string "box.begin() error("test error")"]:1: test error' +... +]] + t.assert_equals(console:send('box.begin() error("test error")'), expected) + t.assert_equals(console:send('box.is_in_txn()'), false_output) +end + +g.test_error_in_different_expr = function(cg) + t.xfail_if(cg.params.name == "remote_bin") + local console = cg.console + console:send('box.begin()') + console:send('error("test error")') + t.assert_equals(console:send('box.is_in_txn()'), true_output) +end