From f6c03d7920351d2212f87fb5c278b01cf6dfe6b9 Mon Sep 17 00:00:00 2001 From: kamenkremen Date: Thu, 18 Dec 2025 12:50:10 +0300 Subject: [PATCH 1/2] storage: preserve original error on function fail In Tarantool versions prior to 3.0.0-beta1-18, `net_box.self.call()` cannot invoke C stored or Lua persistent functions. Therefore, if such a function is registered in `box.func`, vshard executes it via `func.call` instead. If such a function raises an error during an uncommitted transaction, 'Transaction is active...' error is going to be raised, which will mask the original error. This patch fixes that bug by checking the function call result. If the function returns an error and a transaction is still active, vshard now explicitly rolls back the transaction. Fixes #614 NO_DOC=bugfix --- test/storage-luatest/storage_1_test.lua | 26 +++++++++++++++++++++++++ vshard/storage/init.lua | 15 +++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/test/storage-luatest/storage_1_test.lua b/test/storage-luatest/storage_1_test.lua index d84f25f7..532ee2c7 100644 --- a/test/storage-luatest/storage_1_test.lua +++ b/test/storage-luatest/storage_1_test.lua @@ -674,3 +674,29 @@ test_group.test_info_disable_consistency = function(g) ilt.assert(res.is_enabled) end, {global_cfg}) end + +local function test_error_msg_is_preserved_template(add_to_schema, g) + g.replica_1_a:exec(function(add_to_schema) + rawset(_G, 'test_fail', function() + box.begin() + error('test_error') + end) + if add_to_schema then + box.schema.func.create('test_fail') + end + local status, err = ivshard.storage.call(1, 'read', 'test_fail', {}) + ilt.assert_not(status) + local err_msg = tostring(err) + ilt.assert_str_contains(err_msg, 'test_error') + ilt.assert_not_str_contains(err_msg, 'Transaction is active') + rawset(_G, 'test_fail', nil) + if add_to_schema then + box.schema.func.drop('test_fail') + end + end, {add_to_schema}) +end + +test_group.test_error_msg_is_preserved = function(g) + test_error_msg_is_preserved_template(false, g) + test_error_msg_is_preserved_template(true, g) +end diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index 30951e09..25e6d369 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -260,6 +260,14 @@ else end end +local function handle_results(status, ...) + if not status then + box.rollback() + return status, box.error.new(box.error.PROC_LUA, (...)) + end + return status, ... +end + -- -- Invoke a function on this instance. Arguments are unpacked into the function -- as arguments. @@ -284,7 +292,12 @@ local_call = function(func_name, args) if not func then return pcall(netbox_self_call, netbox_self, func_name, args) end - return pcall(func.call, func, args) + -- If the function is called directly, fails, and leaves an uncommitted + -- transaction, then in Tarantool versions before 3.0.0-beta1-18, + -- the original error is replaced by the "Transaction is active" error. + -- We manually check if the function returned an error. If so, we + -- rollback the transaction to reveal the original error. + return handle_results(pcall(func.call, func, args)) end end From 5ffabbc48baeb5c081e9aa72ae4c375081d942e9 Mon Sep 17 00:00:00 2001 From: Nikita Zheleztsov Date: Fri, 6 Feb 2026 17:56:53 +0300 Subject: [PATCH 2/2] storage: reimplement net.box.self.call() without msgpack encoding NO_DOC=refactoring NO_TEST=refactoring --- vshard/storage/init.lua | 52 ++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua index 25e6d369..740a32ff 100644 --- a/vshard/storage/init.lua +++ b/vshard/storage/init.lua @@ -2,15 +2,12 @@ local log = require('log') local luri = require('uri') local lfiber = require('fiber') local lmsgpack = require('msgpack') -local netbox = require('net.box') -- for net.box:self() local trigger = require('internal.trigger') local ffi = require('ffi') local json_encode = require('json').encode local yaml_encode = require('yaml').encode local fiber_clock = lfiber.clock local fiber_yield = lfiber.yield -local netbox_self = netbox.self -local netbox_self_call = netbox_self.call local MODULE_INTERNALS = '__module_vshard_storage' -- Reload requirements, in case this module is reloaded manually. @@ -269,37 +266,28 @@ local function handle_results(status, ...) end -- --- Invoke a function on this instance. Arguments are unpacked into the function --- as arguments. --- The function returns pcall() as is, because is used from places where --- exceptions are not allowed. +-- Reimplementation of the net.box.self.call(), but without encoding/decoding +-- of the arguments. Invoke a function on this instance. Arguments are unpacked +-- into the function as arguments. The function returns pcall() as is, because +-- is used from places where exceptions are not allowed. -- -local local_call - -if util.version_is_at_least(3, 0, 0, 'beta', 1, 18) then - -local_call = function(func_name, args) - return pcall(netbox_self_call, netbox_self, func_name, args) -end - -else -- < 3.0.0-beta1-18 - --- net_box.self.call() doesn't work with C stored and Lua persistent --- functions before 3.0.0-beta1-18, so we try to call it via func.call --- API prior to using net_box.self API. -local_call = function(func_name, args) - local func = box.func and box.func[func_name] - if not func then - return pcall(netbox_self_call, netbox_self, func_name, args) +local function local_call(name, args) + args = args or {} + name = tostring(name) + local status, func, obj + func = box.func and box.func[name] + if func then + return handle_results(pcall(func.call, func, args)) + end + status, func, obj = handle_results(pcall(box.internal.call_loadproc, name)) + if not status then + return status, func + end + if obj ~= nil then + return handle_results(pcall(func, obj, unpack(args))) + else + return handle_results(pcall(func, unpack(args))) end - -- If the function is called directly, fails, and leaves an uncommitted - -- transaction, then in Tarantool versions before 3.0.0-beta1-18, - -- the original error is replaced by the "Transaction is active" error. - -- We manually check if the function returned an error. If so, we - -- rollback the transaction to reveal the original error. - return handle_results(pcall(func.call, func, args)) -end - end local function master_call(replicaset, func, args, opts)