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

Author: kamenkremen

test/storage-luatest/storage_1_test.lua

@@ -674,3 +674,57 @@ 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
+
+test_group.test_local_call_return_type_consistency = function(g)
+    g.replica_1_a:exec(function()
+        local test_function = (function()
+            return box.tuple.new({100, 200, 'tuple'})
+        end)
+        rawset(_G, 'test_return_tuple1', test_function)
+        rawset(_G, 'test_return_tuple2', test_function)
+        box.schema.func.create('test_return_tuple1')
+        local status1, result1 = ivshard.storage.call(1,
+                                                      'read',
+                                                      'test_return_tuple1',
+                                                      {})
+        local status2, result2 = ivshard.storage.call(1,
+                                                      'read',
+                                                      'test_return_tuple2',
+                                                      {})
+        ilt.assert(status1)
+        ilt.assert(status2)
+        ilt.assert_equals(type(result1), type(result2))
+        ilt.assert_equals(type(result1), 'table')
+        ilt.assert_equals(result1, result2)
+        ilt.assert_equals(result1, {100, 200, 'tuple'})
+        box.schema.func.drop('test_return_tuple1')
+        _G.test_return_tuple1 = nil
+        _G.test_return_tuple2 = nil
+    end)
+end

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