Proper RPC error responses (ethereum#3061)

* Refactor response/error handling

* Return RPC error or result

* Tester provider tests to handle rpc errors

* Update test error response to include data attr

* Remove data attribute from response

* Simplify response method

Author: Stuart Reed

newsfragments/3061.feature.rst

@@ -0,0 +1 @@
+Return structured JSON-RPC errors for missing or unimplemented eth-tester methods.

tests/core/providers/test_tester_provider.py

@@ -62,3 +62,29 @@ def test_eth_tester_provider_properly_handles_eth_tester_error_messages(
         TransactionFailed, match="execution reverted: The error message."
     ):
         provider.make_request(RPCEndpoint("eth_blockNumber"), [])
+
+
+def test_eth_tester_provider_properly_handles_eth_tester_key_error_messages():
+    provider = EthereumTesterProvider(api_endpoints={})
+    response = provider.make_request(RPCEndpoint("eth_blockNumber"), [])
+
+    assert response["error"]["code"] == -32601
+    assert response["error"]["message"] == "Unknown RPC Endpoint: eth_blockNumber"
+
+
+def test_eth_tester_provider_properly_handles_eth_tester_not_implmented_error_messages(
+    mocker,
+):
+    mocker.patch(
+        "eth_tester.main.EthereumTester.get_block_by_number",
+        side_effect=NotImplementedError("The error message."),
+    )
+
+    provider = EthereumTesterProvider()
+    response = provider.make_request(RPCEndpoint("eth_blockNumber"), [])
+
+    assert response["error"]["code"] == -32601
+    assert (
+        response["error"]["message"]
+        == "RPC Endpoint has not been implemented: eth_blockNumber"
+    )

tests/integration/test_ethereum_tester.py

@@ -31,6 +31,9 @@
     NetModuleTest,
     Web3ModuleTest,
 )
+from web3.exceptions import (
+    MethodUnavailable,
+)
 from web3.providers.eth_tester import (
     EthereumTesterProvider,
 )
@@ -264,7 +267,8 @@ def func_wrapper(self, eth_tester, *args, **kwargs):
 
 class TestEthereumTesterEthModule(EthModuleTest):
     test_eth_max_priority_fee_with_fee_history_calculation = not_implemented(
-        EthModuleTest.test_eth_max_priority_fee_with_fee_history_calculation, ValueError
+        EthModuleTest.test_eth_max_priority_fee_with_fee_history_calculation,
+        MethodUnavailable,
     )
     test_eth_max_priority_fee_with_fee_history_calculation_error_dict = not_implemented(
         EthModuleTest.test_eth_max_priority_fee_with_fee_history_calculation_error_dict,
@@ -290,16 +294,16 @@ class TestEthereumTesterEthModule(EthModuleTest):
         EthModuleTest.test_eth_sign_transaction_ens_names, ValueError
     )
     test_eth_submit_hashrate = not_implemented(
-        EthModuleTest.test_eth_submit_hashrate, ValueError
+        EthModuleTest.test_eth_submit_hashrate, MethodUnavailable
     )
     test_eth_submit_work = not_implemented(
-        EthModuleTest.test_eth_submit_work, ValueError
+        EthModuleTest.test_eth_submit_work, MethodUnavailable
     )
     test_eth_get_raw_transaction = not_implemented(
-        EthModuleTest.test_eth_get_raw_transaction, ValueError
+        EthModuleTest.test_eth_get_raw_transaction, MethodUnavailable
     )
     test_eth_get_raw_transaction_raises_error = not_implemented(
-        EthModuleTest.test_eth_get_raw_transaction, ValueError
+        EthModuleTest.test_eth_get_raw_transaction, MethodUnavailable
     )
     test_eth_get_raw_transaction_by_block = not_implemented(
         EthModuleTest.test_eth_get_raw_transaction_by_block, ValueError
@@ -588,7 +592,12 @@ class TestEthereumTesterNetModule(NetModuleTest):
 class TestEthereumTesterPersonalModule(GoEthereumPersonalModuleTest):
     test_personal_sign_and_ecrecover = not_implemented(
         GoEthereumPersonalModuleTest.test_personal_sign_and_ecrecover,
-        ValueError,
+        MethodUnavailable,
+    )
+
+    test_personal_list_wallets = not_implemented(
+        GoEthereumPersonalModuleTest.test_personal_list_wallets,
+        MethodUnavailable,
     )
 
     # Test overridden here since eth-tester returns False
@@ -598,9 +607,3 @@ def test_personal_unlock_account_failure(self, w3, unlockable_account_dual_type)
             unlockable_account_dual_type, "bad-password"
         )
         assert result is False
-
-    @pytest.mark.xfail(
-        raises=ValueError, reason="list_wallets not implemented in eth-tester"
-    )
-    def test_personal_list_wallets(self, w3: "Web3") -> None:
-        super().test_personal_list_wallets(w3)

web3/providers/eth_tester/main.py

@@ -35,6 +35,7 @@
 )
 from web3.types import (
     RPCEndpoint,
+    RPCError,
     RPCResponse,
 )
 
@@ -135,6 +136,19 @@ def is_connected(self, show_traceback: bool = False) -> Literal[True]:
         return True
 
 
+def _make_response(result: Any, message: str = "") -> RPCResponse:
+    if isinstance(result, Exception):
+        return RPCResponse(
+            {
+                "id": 1,
+                "jsonrpc": "2.0",
+                "error": RPCError({"code": -32601, "message": message, "data": None}),
+            }
+        )
+
+    return RPCResponse({"id": 1, "jsonrpc": "2.0", "result": result})
+
+
 def _make_request(
     method: RPCEndpoint,
     params: Any,
@@ -151,14 +165,12 @@ def _make_request(
 
     try:
         delegator = api_endpoints[namespace][endpoint]
-    except KeyError:
-        return RPCResponse({"error": f"Unknown RPC Endpoint: {method}"})
+    except KeyError as e:
+        return _make_response(e, f"Unknown RPC Endpoint: {method}")
     try:
         response = delegator(ethereum_tester_instance, params)
-    except NotImplementedError:
-        return RPCResponse(
-            {"error": f"RPC Endpoint has not been implemented: {method}"}
-        )
+    except NotImplementedError as e:
+        return _make_response(e, f"RPC Endpoint has not been implemented: {method}")
     except TransactionFailed as e:
         first_arg = e.args[0]
         try:
@@ -175,6 +187,4 @@ def _make_request(
             reason = first_arg
         raise TransactionFailed(f"execution reverted: {reason}")
     else:
-        return {
-            "result": response,
-        }
+        return _make_response(response)