[3.2 -> 4.0] Modify trace_api_plugin to report serialization errors to user

plugins/trace_api_plugin/test/test_data_handlers.cpp

@@ -135,6 +135,53 @@ BOOST_AUTO_TEST_SUITE(abi_data_handler_tests)
       BOOST_TEST(to_kv(expected) == to_kv(std::get<0>(actual)), boost::test_tools::per_element());
    }
 
+   BOOST_AUTO_TEST_CASE(abi_fail_yield)
+   {
+      auto action = action_trace_v1 {
+          { 0, "alice"_n, "alice"_n, "foo"_n, {}, {0x00, 0x01, 0x02, 0x03}},
+          {0x04, 0x05, 0x06, 0x07}
+      };
+
+      std::variant<action_trace_v0, action_trace_v1> action_trace_t = action;
+
+      auto abi = chain::abi_def ( {},
+                                   {
+                                    { "foo", "", { {"a", "varuint32"}, {"b", "varuint32"}, {"c", "varuint32"}, {"d", "varuint32"} } }
+                                   },
+                                   {
+                                    { "foo"_n, "foo", ""}
+                                   },
+                                   {}, {}, {}
+      );
+      abi.version = "eosio::abi/1.";
+
+      bool except_called = false;
+      auto except_handler = [&](const exception_with_context& e) {
+         except_called = true;
+         BOOST_CHECK(std::get<0>(e).operator bool());
+         BOOST_CHECK(std::get<2>(e) > 0);
+         if (std::get<0>(e)) { // rethrow so caller is notified of error
+            std::rethrow_exception(std::get<0>(e));
+         }
+      };
+
+      abi_data_handler handler(except_handler);
+      handler.add_abi("alice"_n, std::move(abi));
+
+      fc::variant expected = fc::mutable_variant_object()
+          ("a", 0)
+          ("b", 1)
+          ("c", 2)
+          ("d", 3);
+
+      uint32_t depth = 0;
+      BOOST_CHECK_EXCEPTION(handler.serialize_to_variant(action_trace_t, [&](){ ++depth; if (depth > 1) throw std::runtime_error("oops"); }), std::runtime_error,
+                            [](const std::runtime_error& e) {
+                               return std::string(e.what()).find("oops") != std::string::npos;
+                            });
+      BOOST_CHECK(except_called);
+   }
+
    BOOST_AUTO_TEST_CASE(basic_abi_wrong_type)
    {
       auto action = action_trace_v0 {

plugins/trace_api_plugin/test/test_responses.cpp

@@ -556,6 +556,65 @@ BOOST_AUTO_TEST_SUITE(trace_responses)
       BOOST_REQUIRE_THROW(get_block_trace( 1, yield ), yield_exception);
    }
 
+   BOOST_FIXTURE_TEST_CASE(yield_throws_from_data_handler, response_test_fixture)
+   {
+      auto action_trace = action_trace_v1 {
+         {
+            0,
+            "receiver"_n, "contract"_n, "action"_n,
+            {{ "alice"_n, "active"_n }},
+            { 0x00, 0x01, 0x02, 0x03 }
+         },
+         { 0x04, 0x05, 0x06, 0x07 }
+      };
+
+      auto transaction_trace = transaction_trace_v2 {
+         "0000000000000000000000000000000000000000000000000000000000000001"_h,
+         std::vector<action_trace_v1> {
+            action_trace
+         },
+         fc::enum_type<uint8_t, chain::transaction_receipt_header::status_enum>{chain::transaction_receipt_header::status_enum::executed},
+         10,
+         5,
+         std::vector<chain::signature_type>{ chain::signature_type() },
+         { chain::time_point(), 1, 0, 100, 50, 0 }
+      };
+
+      auto block_trace = block_trace_v2 {
+         "b000000000000000000000000000000000000000000000000000000000000001"_h,
+         1,
+         "0000000000000000000000000000000000000000000000000000000000000000"_h,
+         chain::block_timestamp_type(0),
+         "bp.one"_n,
+         "0000000000000000000000000000000000000000000000000000000000000000"_h,
+         "0000000000000000000000000000000000000000000000000000000000000000"_h,
+         0,
+         std::vector<transaction_trace_v2> {
+            transaction_trace
+         }
+      };
+
+      mock_get_block = [&block_trace]( uint32_t height, const yield_function& ) -> get_block_t {
+         BOOST_TEST(height == 1);
+         return std::make_tuple(data_log_entry(block_trace), false);
+      };
+
+      mock_get_block = [&block_trace]( uint32_t height, const yield_function& ) -> get_block_t {
+         BOOST_TEST(height == 1);
+         return std::make_tuple(data_log_entry(block_trace), true);
+      };
+
+      // simulate data_handler failing
+      mock_data_handler_v1 = [&](const action_trace_v1&, const yield_function&) -> std::tuple<fc::variant, std::optional<fc::variant>> {
+         throw std::runtime_error("mock error");
+      };
+
+      // no other yield calls will throw
+      yield_function yield = [&]() {};
+
+      BOOST_REQUIRE_THROW(get_block_trace( 1, yield ), std::runtime_error);
+   }
+
    BOOST_FIXTURE_TEST_CASE(old_version_block_response, response_test_fixture)
    {
       auto block_trace = block_trace_v0 {

plugins/trace_api_plugin/trace_api_plugin.cpp

@@ -202,6 +202,9 @@ struct trace_api_rpc_plugin_impl : public std::enable_shared_from_this<trace_api
       ilog("initializing trace api rpc plugin");
       std::shared_ptr<abi_data_handler> data_handler = std::make_shared<abi_data_handler>([](const exception_with_context& e){
          log_exception(e, fc::log_level::debug);
+         if (std::get<0>(e)) { // rethrow so caller is notified of error
+            std::rethrow_exception(std::get<0>(e));
+         }
       });
 
       if( options.count("trace-rpc-abi") ) {

tests/TestHarness/testUtils.py

@@ -216,7 +216,7 @@ def checkDelayedOutput(popen, cmd, ignoreError=False):
         Utils.checkOutputFileWrite(start, cmd, output, error)
         if popen.returncode != 0 and not ignoreError:
             raise subprocess.CalledProcessError(returncode=popen.returncode, cmd=cmd, output=output, stderr=error)
-        return output.decode("utf-8")
+        return output.decode("utf-8") if popen.returncode == 0 else error.decode("utf-8")
 
     @staticmethod
     def errorExit(msg="", raw=False, errorCode=1):
@@ -315,13 +315,13 @@ def runCmdArrReturnJson(cmdArr, trace=False, silentErrors=True):
         return Utils.toJson(retStr)
 
     @staticmethod
-    def runCmdReturnStr(cmd, trace=False):
+    def runCmdReturnStr(cmd, trace=False, ignoreError=False):
         cmdArr=shlex.split(cmd)
-        return Utils.runCmdArrReturnStr(cmdArr)
+        return Utils.runCmdArrReturnStr(cmdArr, ignoreError=ignoreError)
 
     @staticmethod
-    def runCmdArrReturnStr(cmdArr, trace=False):
-        retStr=Utils.checkOutput(cmdArr)
+    def runCmdArrReturnStr(cmdArr, trace=False, ignoreError=False):
+        retStr=Utils.checkOutput(cmdArr, ignoreError=ignoreError)
         if trace: Utils.Print ("RAW > %s" % (retStr))
         return retStr
 

tests/plugin_http_api_test.py

@@ -1362,6 +1362,22 @@ def test_TraceApi(self) :
         self.assertEqual(ret_json["code"], 404)
         self.assertEqual(ret_json["error"]["code"], 0)
 
+        # get_transaction_trace with empty parameter
+        command = "get_transaction_trace"
+        ret_json = self.nodeos.processUrllibRequest(resource, command)
+        self.assertEqual(ret_json["code"], 400)
+        # get_transaction_trace with empty content parameter
+        ret_json = self.nodeos.processUrllibRequest(resource, command, self.empty_content_dict)
+        self.assertEqual(ret_json["code"], 400)
+        # get_transaction_trace with invalid parameter
+        ret_json = self.nodeos.processUrllibRequest(resource, command, self.http_post_invalid_param)
+        self.assertEqual(ret_json["code"], 400)
+        # get_transaction_trace with valid parameter
+        payload = {"id":"f6e325a524e0d75c2275e7d9c2d9e065a38760c29b1d0471a75ccde650ef26d6"}
+        ret_json = self.nodeos.processUrllibRequest(resource, command, payload)
+        self.assertEqual(ret_json["code"], 404)
+        self.assertEqual(ret_json["error"]["code"], 0)
+
     # test all db_size api
     def test_DbSizeApi(self) :
         resource = "db_size"

tests/trace_plugin_test.py

@@ -5,6 +5,7 @@
 import time
 import unittest
 import os
+import signal
 
 from TestHarness import Cluster, Node, TestHelper, Utils, WalletMgr, CORE_SYMBOL
 
@@ -30,7 +31,7 @@ def cleanEnv(self, shouldCleanup: bool) :
     def startEnv(self) :
         account_names = ["alice", "bob", "charlie"]
         abs_path = os.path.abspath(os.getcwd() + '/unittests/contracts/eosio.token/eosio.token.abi')
-        traceNodeosArgs = " --trace-rpc-abi eosio.token=" + abs_path
+        traceNodeosArgs = " --verbose-http-errors --trace-rpc-abi eosio.token=" + abs_path
         self.cluster.launch(totalNodes=1, extraNodeosArgs=traceNodeosArgs)
         self.walletMgr.launch()
         testWalletName="testwallet"
@@ -106,6 +107,19 @@ def test_TraceApi(self) :
         global testSuccessful
         testSuccessful = True
 
+        # relaunch with no time allocated for http response & abi-serializer. Will fail because get_info fails.
+        node.kill(signal.SIGTERM)
+        Utils.Print("Ignore expected: ERROR: Node relaunch Failed")
+        isRelaunchSuccess = node.relaunch(timeout=10, addSwapFlags={"--http-max-response-time-ms": "0", "--abi-serializer-max-time-ms": "10"})
+
+        cmdDesc="get block_trace"
+        cmd=" --print-response %s %d" % (cmdDesc, blockNum)
+        cmd="%s %s %s" % (Utils.EosClientPath, node.eosClientArgs(), cmd)
+        result=Utils.runCmdReturnStr(cmd, ignoreError=True)
+
+        Utils.Print(f"{cmdDesc} returned: {result}")
+        self.assertIn("Internal Server Error", result)
+
     @classmethod
     def setUpClass(self):
         self.cleanEnv(self, shouldCleanup=True)