Log and raise a better exception for Ethereum RPC errors

gnosis/eth/ethereum_client.py

@@ -154,6 +154,28 @@ def with_exception_handling(*args, **kwargs):
     return with_exception_handling
 
 
+def parse_rpc_result_or_raise(
+    result: Dict[str, Any], eth_fn: str, arguments: Any
+) -> Any:
+    """
+    Responses from RPC should return a dictionary with `result` key and they are always 200 OK
+    even if errored. If not, raise an error
+
+    :param result:
+    :param eth_fn: RPC function called (for more info if exception is raised)
+    :param arguments: Arguments for the RPC function (for more info if exception is raised)
+    :return: `result["result"]` if key exists
+    :raises: ValueError
+    """
+
+    if "result" not in result:
+        message = f"Problem calling `{eth_fn}` on {arguments}, result={result}"
+        logger.error(message)
+        raise ValueError(message)
+
+    return result["result"]
+
+
 class EthereumTxSent(NamedTuple):
     tx_hash: bytes
     tx: TxParams
@@ -1095,11 +1117,7 @@ def trace_blocks(
         results = sorted(response.json(), key=lambda x: x["id"])
         traces = []
         for block_identifier, result in zip(block_identifiers, results):
-            if "result" not in result:
-                message = f"Problem calling batch `trace_block` on block={block_identifier}, result={result}"
-                logger.error(message)
-                raise ValueError(message)
-            raw_tx = result["result"]
+            raw_tx = parse_rpc_result_or_raise(result, "trace_block", block_identifier)
             if raw_tx:
                 try:
                     decoded_traces = self._decode_traces(raw_tx)
@@ -1152,8 +1170,8 @@ def trace_transactions(
             raise ValueError(message)
         results = sorted(response.json(), key=lambda x: x["id"])
         traces = []
-        for result in results:
-            raw_tx = result["result"]
+        for tx_hash, result in zip(tx_hashes, results):
+            raw_tx = parse_rpc_result_or_raise(result, "trace_transaction", tx_hash)
             if raw_tx:
                 try:
                     decoded_traces = self._decode_traces(raw_tx)
@@ -1663,8 +1681,10 @@ def get_transactions(self, tx_hashes: List[EthereumHash]) -> List[Optional[TxDat
             self.ethereum_node_url, json=payload, timeout=self.slow_timeout
         ).json()
         txs = []
-        for result in sorted(results, key=lambda x: x["id"]):
-            raw_tx = result["result"]
+        for tx_hash, result in zip(tx_hashes, sorted(results, key=lambda x: x["id"])):
+            raw_tx = parse_rpc_result_or_raise(
+                result, "eth_getTransactionByHash", tx_hash
+            )
             if raw_tx:
                 txs.append(transaction_result_formatter(raw_tx))
             else:
@@ -1712,8 +1732,10 @@ def get_transaction_receipts(
             self.ethereum_node_url, json=payload, timeout=self.slow_timeout
         ).json()
         receipts = []
-        for result in sorted(results, key=lambda x: x["id"]):
-            tx_receipt = result["result"]
+        for tx_hash, result in zip(tx_hashes, sorted(results, key=lambda x: x["id"])):
+            tx_receipt = parse_rpc_result_or_raise(
+                result, "eth_getTransactionReceipt", tx_hash
+            )
             # Parity returns tx_receipt even is tx is still pending, so we check `blockNumber` is not None
             if tx_receipt and tx_receipt["blockNumber"] is not None:
                 receipts.append(receipt_formatter(tx_receipt))
@@ -1764,8 +1786,16 @@ def get_blocks(
             self.ethereum_node_url, json=payload, timeout=self.slow_timeout
         ).json()
         blocks = []
-        for result in sorted(results, key=lambda x: x["id"]):
-            raw_block = result["result"]
+        for block_identifier, result in zip(
+            block_identifiers, sorted(results, key=lambda x: x["id"])
+        ):
+            raw_block = parse_rpc_result_or_raise(
+                result,
+                "eth_getBlockByNumber"
+                if isinstance(block_identifier, int)
+                else "eth_getBlockByHash",
+                block_identifier,
+            )
             if raw_block:
                 if "extraData" in raw_block:
                     del raw_block[

gnosis/eth/tests/test_ethereum_client.py

@@ -21,6 +21,7 @@
     InvalidNonce,
     ParityManager,
     SenderAccountNotFoundInNode,
+    parse_rpc_result_or_raise,
 )
 from ..exceptions import BatchCallException, InvalidERC20Info
 from ..utils import fast_to_checksum_address, get_eth_address_with_key
@@ -776,6 +777,19 @@ def test_rinkeby_ethereum_network_name(self):
 
 
 class TestEthereumClient(EthereumTestCaseMixin, TestCase):
+    def test_parse_rpc_result_or_raise(self):
+        self.assertEqual(parse_rpc_result_or_raise({"result": "test"}, "", ""), "test")
+
+        with self.assertRaisesMessage(
+            ValueError,
+            "Problem calling `trace_transaction` on 0x230b7f018951818c2a4545654d43a086ed2a3ed7c5b7c03990f4ac22ffae3840, result={'error': 'Something bad happened'}",
+        ):
+            parse_rpc_result_or_raise(
+                {"error": "Something bad happened"},
+                "trace_transaction",
+                "0x230b7f018951818c2a4545654d43a086ed2a3ed7c5b7c03990f4ac22ffae3840",
+            )
+
     def test_ethereum_client_str(self):
         self.assertTrue(str(self.ethereum_client))