From 3f8216d1aeb48255a05072c2d4f498ae938b5aae Mon Sep 17 00:00:00 2001
From: kamenkremen <ivanz0610@gmail.com>
Date: Thu, 18 Dec 2025 12:50:10 +0300
Subject: [PATCH] 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.

Additionally, this patch fixes inconsistent return types. Previously,
functions registered in the schema returned raw cdata (such as tuples),
while other functions returned data converted via msgpack. This patch
ensures consistency by performing a msgpack conversion for registered
functions as well.

Fixes #614
Fixes #630

NO_DOC=bugfix
---
 test/storage-luatest/storage_1_test.lua | 52 +++++++++++++++++++++++++
 vshard/storage/init.lua                 | 25 +++++++++++-
 2 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/test/storage-luatest/storage_1_test.lua b/test/storage-luatest/storage_1_test.lua
index d84f25f7..c1b1506e 100644
--- a/test/storage-luatest/storage_1_test.lua
+++ b/test/storage-luatest/storage_1_test.lua
@@ -674,3 +674,55 @@ 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
+
+local function test_local_call_return_type_consistency_template(
+    add_to_schema, g)
+    g.replica_1_a:exec(function(add_to_schema)
+        rawset(_G, 'test_return_tuple', function()
+            return box.tuple.new({100, 200, 'tuple'})
+        end)
+        if add_to_schema then
+            box.schema.func.create('test_return_tuple')
+        end
+        local status, result = ivshard.storage.call(
+            1, 'read', 'test_return_tuple', {})
+        ilt.assert(status)
+        ilt.assert_equals(type(result), 'table')
+        ilt.assert_equals(result, {100, 200, 'tuple'})
+        if add_to_schema then
+            box.schema.func.drop('test_return_tuple')
+        end
+        _G.test_return_tuple = nil
+    end, {add_to_schema})
+end
+
+test_group.test_local_call_return_type_consistency = function(g)
+    test_local_call_return_type_consistency_template(true, g)
+    test_local_call_return_type_consistency_template(false, g)
+end
diff --git a/vshard/storage/init.lua b/vshard/storage/init.lua
index 30951e09..40889d18 100644
--- a/vshard/storage/init.lua
+++ b/vshard/storage/init.lua
@@ -7,6 +7,7 @@ local trigger = require('internal.trigger')
 local ffi = require('ffi')
 local json_encode = require('json').encode
 local yaml_encode = require('yaml').encode
+local msgpack = require('msgpack')
 local fiber_clock = lfiber.clock
 local fiber_yield = lfiber.yield
 local netbox_self = netbox.self
@@ -260,6 +261,23 @@ else
     end
 end
 
+--
+-- This is copy-paste from the net_box.lua handle_eval_result.
+--
+local function handle_results(status, ...)
+    if not status then
+        box.rollback()
+        return status, box.error.new(box.error.PROC_LUA, (...))
+    end
+    local results = {...}
+    for i = 1, select('#', ...) do
+        if type(results[i]) == 'cdata' then
+            results[i] = msgpack.decode(msgpack.encode(results[i]))
+        end
+    end
+    return status, unpack(results)
+end
+
 --
 -- Invoke a function on this instance. Arguments are unpacked into the function
 -- as arguments.
@@ -284,7 +302,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