From 805957096f244d99f46e5703fa5ecb166be2ba10 Mon Sep 17 00:00:00 2001 From: fselmo Date: Mon, 22 Apr 2024 12:00:45 -0600 Subject: [PATCH 1/8] Validate RPC responses more strictly. Raise ``Web3RPCError`` over ``Web3ValueError``. - Better organize the formatted_response tests --- .../caching-utils/test_request_caching.py | 12 +- .../contracts/test_contract_call_interface.py | 6 +- .../test_contract_caller_interface.py | 10 +- .../contracts/test_contract_util_functions.py | 6 +- .../test_middleware_can_be_stateful.py | 2 +- .../manager/test_provider_request_wrapping.py | 2 + .../core/manager/test_response_formatters.py | 507 +++++++++--------- .../middleware/test_formatting_middleware.py | 4 +- web3/_utils/contracts.py | 10 +- web3/_utils/module_testing/eth_module.py | 34 +- web3/exceptions.py | 52 +- web3/manager.py | 175 +++--- web3/types.py | 2 +- 13 files changed, 473 insertions(+), 349 deletions(-) diff --git a/tests/core/caching-utils/test_request_caching.py b/tests/core/caching-utils/test_request_caching.py index 2f390e48f1..567ff84384 100644 --- a/tests/core/caching-utils/test_request_caching.py +++ b/tests/core/caching-utils/test_request_caching.py @@ -13,7 +13,7 @@ generate_cache_key, ) from web3.exceptions import ( - Web3ValueError, + Web3RPCError, ) from web3.providers import ( AsyncBaseProvider, @@ -31,7 +31,7 @@ def simple_cache_return_value_a(): _cache = SimpleCache() _cache.cache( generate_cache_key(f"{threading.get_ident()}:{('fake_endpoint', [1])}"), - {"result": "value-a"}, + {"jsonrpc": "2.0", "id": 0, "result": "value-a"}, ) return _cache @@ -92,9 +92,9 @@ def test_request_caching_does_not_cache_error_responses(request_mocker): with request_mocker( w3, mock_errors={"fake_endpoint": lambda *_: {"message": f"msg-{uuid.uuid4()}"}} ): - with pytest.raises(Web3ValueError) as err_a: + with pytest.raises(Web3RPCError) as err_a: w3.manager.request_blocking("fake_endpoint", []) - with pytest.raises(Web3ValueError) as err_b: + with pytest.raises(Web3RPCError) as err_b: w3.manager.request_blocking("fake_endpoint", []) assert str(err_a) != str(err_b) @@ -197,9 +197,9 @@ async def test_async_request_caching_does_not_cache_error_responses(request_mock async_w3, mock_errors={"fake_endpoint": lambda *_: {"message": f"msg-{uuid.uuid4()}"}}, ): - with pytest.raises(Web3ValueError) as err_a: + with pytest.raises(Web3RPCError) as err_a: await async_w3.manager.coro_request("fake_endpoint", []) - with pytest.raises(Web3ValueError) as err_b: + with pytest.raises(Web3RPCError) as err_b: await async_w3.manager.coro_request("fake_endpoint", []) assert str(err_a) != str(err_b) diff --git a/tests/core/contracts/test_contract_call_interface.py b/tests/core/contracts/test_contract_call_interface.py index 2a0a17b783..94e0f3e945 100644 --- a/tests/core/contracts/test_contract_call_interface.py +++ b/tests/core/contracts/test_contract_call_interface.py @@ -38,7 +38,7 @@ ) from web3.exceptions import ( BadFunctionCallOutput, - BlockNumberOutofRange, + BlockNumberOutOfRange, FallbackNotFound, InvalidAddress, MismatchedABI, @@ -538,7 +538,7 @@ def test_call_nonexistent_receive_function(fallback_function_contract): def test_throws_error_if_block_out_of_range(w3, math_contract): w3.provider.make_request(method="evm_mine", params=[20]) - with pytest.raises(BlockNumberOutofRange): + with pytest.raises(BlockNumberOutOfRange): math_contract.functions.counter().call(block_identifier=-50) @@ -1726,7 +1726,7 @@ async def test_async_call_nonexistent_receive_function( @pytest.mark.asyncio async def test_async_throws_error_if_block_out_of_range(async_w3, async_math_contract): await async_w3.provider.make_request(method="evm_mine", params=[20]) - with pytest.raises(BlockNumberOutofRange): + with pytest.raises(BlockNumberOutOfRange): await async_math_contract.functions.counter().call(block_identifier=-50) diff --git a/tests/core/contracts/test_contract_caller_interface.py b/tests/core/contracts/test_contract_caller_interface.py index a25ef4bfc5..0c3df9ce90 100644 --- a/tests/core/contracts/test_contract_caller_interface.py +++ b/tests/core/contracts/test_contract_caller_interface.py @@ -1,7 +1,7 @@ import pytest from web3.exceptions import ( - BlockNumberOutofRange, + BlockNumberOutOfRange, MismatchedABI, NoABIFound, NoABIFunctionsFound, @@ -139,12 +139,12 @@ def test_caller_with_invalid_block_identifier(w3, math_contract): # Invalid `block_identifier` should not raise when passed to `caller` directly. # The function call itself parses the value. caller = math_contract.caller(block_identifier="abc") - with pytest.raises(BlockNumberOutofRange): + with pytest.raises(BlockNumberOutOfRange): caller.counter() # Calling the function with an invalid `block_identifier` will raise. default_caller = math_contract.caller() - with pytest.raises(BlockNumberOutofRange): + with pytest.raises(BlockNumberOutOfRange): default_caller.counter(block_identifier="abc") @@ -347,12 +347,12 @@ async def test_async_caller_with_invalid_block_identifier( # Invalid `block_identifier` should not raise when passed to `caller` directly. # The function call itself parses the value. caller = async_math_contract.caller(block_identifier="abc") - with pytest.raises(BlockNumberOutofRange): + with pytest.raises(BlockNumberOutOfRange): await caller.counter() # Calling the function with an invalid `block_identifier` will raise. default_caller = async_math_contract.caller() - with pytest.raises(BlockNumberOutofRange): + with pytest.raises(BlockNumberOutOfRange): await default_caller.counter(block_identifier="abc") diff --git a/tests/core/contracts/test_contract_util_functions.py b/tests/core/contracts/test_contract_util_functions.py index 4009ddfd8f..82a132f59a 100644 --- a/tests/core/contracts/test_contract_util_functions.py +++ b/tests/core/contracts/test_contract_util_functions.py @@ -8,7 +8,7 @@ validate_payable, ) from web3.exceptions import ( - BlockNumberOutofRange, + BlockNumberOutOfRange, ) @@ -50,7 +50,7 @@ def test_parse_block_identifier_bytes_and_hex(w3): ), ) def test_parse_block_identifier_error(w3, block_identifier): - with pytest.raises(BlockNumberOutofRange): + with pytest.raises(BlockNumberOutOfRange): parse_block_identifier(w3, block_identifier) @@ -117,7 +117,7 @@ async def test_async_parse_block_identifier_bytes_and_hex(async_w3): ), ) async def test_async_parse_block_identifier_error(async_w3, block_identifier): - with pytest.raises(BlockNumberOutofRange): + with pytest.raises(BlockNumberOutOfRange): await async_parse_block_identifier(async_w3, block_identifier) diff --git a/tests/core/manager/test_middleware_can_be_stateful.py b/tests/core/manager/test_middleware_can_be_stateful.py index 231e0edc0b..69ab8921ad 100644 --- a/tests/core/manager/test_middleware_can_be_stateful.py +++ b/tests/core/manager/test_middleware_can_be_stateful.py @@ -15,7 +15,7 @@ class StatefulMiddleware(Web3Middleware): def wrap_make_request(self, make_request): def middleware(method, params): self.state.append((method, params)) - return {"result": self.state} + return {"jsonrpc": "2.0", "id": 1, "result": self.state} return middleware diff --git a/tests/core/manager/test_provider_request_wrapping.py b/tests/core/manager/test_provider_request_wrapping.py index 97d66b2277..3ea6565bb2 100644 --- a/tests/core/manager/test_provider_request_wrapping.py +++ b/tests/core/manager/test_provider_request_wrapping.py @@ -9,6 +9,8 @@ class DummyProvider(BaseProvider): def make_request(self, method, params): return { + "jsonrpc": "2.0", + "id": 1, "result": { "method": method, "params": params, diff --git a/tests/core/manager/test_response_formatters.py b/tests/core/manager/test_response_formatters.py index 0b7c1a3de3..17ea575563 100644 --- a/tests/core/manager/test_response_formatters.py +++ b/tests/core/manager/test_response_formatters.py @@ -4,6 +4,9 @@ from eth_utils.toolz import ( identity, ) +from toolz import ( + merge, +) from web3._utils.method_formatters import ( raise_block_not_found, @@ -16,328 +19,354 @@ ContractLogicError, MethodUnavailable, TransactionNotFound, - Web3ValueError, + Web3RPCError, +) + +DEFAULT_BAD_FORMAT_MSG = ( + "The response was in an unexpected format and unable to be parsed." +) +INVALID_JSONRPC_MSG = 'The "jsonrpc" field must be present with a value of "2.0".' +ERROR_OBJ_MSG = ( + 'response["error"] must be a valid object as defined by the JSON-RPC ' + "2.0 specification." ) +INVALID_ERROR_CODE_MSG = 'error["code"] is required and must be an integer value.' +INVALID_ERROR_MESSAGE_MSG = 'error["message"] is required and must be a string value.' +METHOD_UNAVAILABLE_MSG = ( + "the method eth_getTransactionByHash does not exist/is not available." +) +DEFAULT_USER_MSG = ( + "An RPC error was returned by the node. Check the message provided in the error " + "and any available logs for more information." +) + -ERROR_RESPONSE = { +VALID_RESULT_OBJ_RESPONSE = {"jsonrpc": "2.0", "id": 1, "result": {"foo": "bar"}} +VALID_RESPONSE_EXTRA_FIELDS = merge(VALID_RESULT_OBJ_RESPONSE, {"unexpected": "field"}) +VALID_RESPONSE_STRING_ID = merge(VALID_RESULT_OBJ_RESPONSE, {"id": "1"}) + +# response fields validation: id, jsonrpc, missing result / error +INVALID_RESPONSE_INVALID_ID = merge(VALID_RESULT_OBJ_RESPONSE, {"id": None}) +INVALID_RESPONSE_MISSING_ID = {"jsonrpc": "2.0", "result": {"foo": "bar"}} + +INVALID_RESPONSE_INVALID_JSONRPC = merge(VALID_RESULT_OBJ_RESPONSE, {"jsonrpc": "1.0"}) +INVALID_RESPONSE_MISSING_JSONRPC = {"id": 1, "result": {"foo": "bar"}} + +INVALID_RESPONSE_MISSING_RESULT_OR_ERROR = {"jsonrpc": "2.0", "id": 1} + + +# valid subscription response +VALID_SUBSCRIPTION_RESPONSE = { "jsonrpc": "2.0", - "error": { - "code": -32000, - "message": "Requested block number is in a range that is not available yet, " - "because the ancient block sync is still in progress.", + "method": "eth_subscription", + "params": { + "subscription": "0xf13f7073ddef66a8c1b0c9c9f0e543c3", + "result": {"foo": "bar"}, }, } -ERROR_RESPONSE_WITH_NONE_ID = { - "id": None, + +# error object validation +VALID_ERROR_RESPONSE = { "jsonrpc": "2.0", - "error": { - "code": -32000, - "message": "Requested block number is in a range that is not available yet, " - "because the ancient block sync is still in progress.", - }, -} -ERROR_RESPONSE_WITH_VALID_ID = { "id": 1, - "jsonrpc": "2.0", - "error": { - "code": -32000, - "message": "Requested block number is in a range that is not available yet, " - "because the ancient block sync is still in progress.", - }, -} -NONE_RESPONSE = {"jsonrpc": "2.0", "id": 1, "result": None} -ZERO_X_RESPONSE = {"jsonrpc": "2.0", "id": 1, "result": "0x"} -INVALID_JSONRPC_RESP_FORMAT = { - "jsonrpc": "999", - "error": { - "code": -32000, - "message": "Requested block number is in a range that is not available yet, " - "because the ancient block sync is still in progress.", - }, -} -UNEXPECTED_RESPONSE_FORMAT = {"jsonrpc": "2.0", "id": 1} -UNEXPECTED_RESPONSE_FORMAT_NONE_ID = {"jsonrpc": "2.0", "id": None} -ANOTHER_UNEXPECTED_RESP_FORMAT = { - "name": "LimitError", - "message": "You cannot query logs for more than 10000 blocks at once.", - "method": "eth_getLogs", -} -METHOD_NOT_FOUND_RESP_FORMAT = { - "jsonrpc": "2.0", - "error": { - "code": -32601, - "message": "the method eth_getTransactionByHash does not exist/is not " - "available", - }, -} -INVALID_CODE_RESP_FORMAT = { - "jsonrpc": "2.0", - "error": { - "code": "-32601", - "message": "the method eth_getTransactionByHash does not exist/is not " - "available", - }, -} -MISSING_CODE_RESP_FORMAT = { - "jsonrpc": "2.0", - "error": { - "message": "the method eth_getTransactionByHash does not exist/is not " - "available", - }, -} -INVALID_MESSAGE_RESP_FORMAT = { - "jsonrpc": "2.0", "error": { "code": -32000, - "message": {}, + "message": ( + "Requested block number is in a range that is not available yet, because " + "the ancient block sync is still in progress." + ), }, } -ETH_TESTER_METHOD_NOT_FOUND_RESP_FORMAT = { - "error": "the method eth_getTransactionByHash does not exist/is not available", -} +ERROR_RESPONSE_VALID_ID_STRING = merge(VALID_ERROR_RESPONSE, {"id": "1"}) +ERROR_RESPONSE_VALID_ID_NONE = merge(VALID_ERROR_RESPONSE, {"id": None}) +ERROR_RESPONSE_VALID_METHOD_UNAVAILABLE = merge( + VALID_ERROR_RESPONSE, + {"error": {"code": -32601, "message": (METHOD_UNAVAILABLE_MSG)}}, +) +ERROR_RESPONSE_INVALID_ID = merge(VALID_ERROR_RESPONSE, {"id": b"invalid"}) +ERROR_RESPONSE_INVALID_ERROR_OBJECT = merge( + VALID_ERROR_RESPONSE, {"error": METHOD_UNAVAILABLE_MSG} +) +ERROR_RESPONSE_INVALID_CODE = merge(VALID_ERROR_RESPONSE, {"error": {"code": "-32601"}}) +ERROR_RESPONSE_INVALID_MESSAGE = merge( + VALID_ERROR_RESPONSE, {"error": {"code": -32000, "message": {}}} +) +ERROR_RESPONSE_INVALID_MISSING_CODE = merge( + VALID_ERROR_RESPONSE, {"error": {"message": "msg"}} +) +ERROR_RESPONSE_INVALID_MISSING_MESSAGE = merge( + VALID_ERROR_RESPONSE, {"error": {"code": -32000}} +) + +# result object validation +VALID_RESPONSE_NULL_RESULT = merge(VALID_RESULT_OBJ_RESPONSE, {"result": None}) +VALID_RESPONSE_0x_RESULT = merge(VALID_RESULT_OBJ_RESPONSE, {"result": "0x"}) -def raise_contract_logic_error(response): - raise ContractLogicError +def raise_contract_logic_error(_response): + raise ContractLogicError("Test contract logic error.") @pytest.mark.parametrize( - "response,params,error_formatters,null_result_formatters,error", + "response,expected", + ( + (VALID_RESULT_OBJ_RESPONSE, VALID_RESULT_OBJ_RESPONSE["result"]), + (VALID_RESPONSE_STRING_ID, VALID_RESPONSE_STRING_ID["result"]), + # extra fields are ignored + (VALID_RESPONSE_EXTRA_FIELDS, VALID_RESPONSE_EXTRA_FIELDS["result"]), + # Response with null result doesn't raise w/o null result formatters + (VALID_RESPONSE_NULL_RESULT, VALID_RESPONSE_NULL_RESULT["result"]), + # Response with a result of '0x' doesn't raise w/o null result formatters + (VALID_RESPONSE_0x_RESULT, VALID_RESPONSE_0x_RESULT["result"]), + # Subscription response + (VALID_SUBSCRIPTION_RESPONSE, VALID_SUBSCRIPTION_RESPONSE["params"]), + ), +) +def test_formatted_response_valid_response_object(w3, response, expected): + formatted_resp = w3.manager.formatted_response(response, (), identity, identity) + assert formatted_resp == expected + + +@pytest.mark.parametrize( + "response,error,error_message", [ ( - # Error response with no result formatters raises a ValueError - ERROR_RESPONSE, - (), - identity, - identity, - Web3ValueError, - ), - ( - # Error response with error formatters raises error in formatter - ERROR_RESPONSE, - (), - raise_contract_logic_error, - identity, - ContractLogicError, + INVALID_RESPONSE_INVALID_JSONRPC, + BadResponseFormat, + f"{DEFAULT_BAD_FORMAT_MSG} {INVALID_JSONRPC_MSG} " + f"The raw response is: {INVALID_RESPONSE_INVALID_JSONRPC}", ), ( - # Error response with no error formatters raises ValueError - ERROR_RESPONSE, - (), - identity, - raise_block_not_found, - Web3ValueError, + INVALID_RESPONSE_MISSING_JSONRPC, + BadResponseFormat, + f"{DEFAULT_BAD_FORMAT_MSG} {INVALID_JSONRPC_MSG} " + f"The raw response is: {INVALID_RESPONSE_MISSING_JSONRPC}", ), ( - # Error response with no result formatters raises a ValueError - ERROR_RESPONSE_WITH_NONE_ID, - (), - identity, - raise_block_not_found, - Web3ValueError, + INVALID_RESPONSE_INVALID_ID, + BadResponseFormat, + f'{DEFAULT_BAD_FORMAT_MSG} "id" must be an integer or a string ' + "representation of an integer. The raw response is: " + f"{INVALID_RESPONSE_INVALID_ID}", ), ( - # Error response with no result formatters raises a ValueError - ERROR_RESPONSE_WITH_VALID_ID, - (), - identity, - raise_block_not_found, - Web3ValueError, + INVALID_RESPONSE_MISSING_ID, + BadResponseFormat, + f'{DEFAULT_BAD_FORMAT_MSG} Response must include an "id" field or be ' + "formatted as an `eth_subscription` response. The raw response is: " + f"{INVALID_RESPONSE_MISSING_ID}", ), ( - # None result raises error if there is a null_result_formatter - NONE_RESPONSE, - (), - identity, - raise_block_not_found, - BlockNotFound, + INVALID_RESPONSE_MISSING_RESULT_OR_ERROR, + BadResponseFormat, + f'{DEFAULT_BAD_FORMAT_MSG} Response must include either "error" or ' + '"result". The raw response is: ' + f"{INVALID_RESPONSE_MISSING_RESULT_OR_ERROR}", ), + ], +) +def test_formatted_response_invalid_response_object(w3, response, error, error_message): + with pytest.raises(error, match=re.escape(error_message)): + w3.manager.formatted_response(response, (), identity, identity) + + +@pytest.mark.parametrize( + "response,error,error_message", + ( ( - # Params are handled with a None result - NONE_RESPONSE, - ("0x03",), - identity, - raise_block_not_found, - BlockNotFound, + VALID_ERROR_RESPONSE, + Web3RPCError, + f'{VALID_ERROR_RESPONSE["error"]}\nUser message: {DEFAULT_USER_MSG}', ), ( - # Multiple params are handled with a None result - NONE_RESPONSE, - ("0x03", "0x01"), - identity, - raise_block_not_found_for_uncle_at_index, - BlockNotFound, + ERROR_RESPONSE_VALID_ID_STRING, + Web3RPCError, + f'{ERROR_RESPONSE_VALID_ID_STRING["error"]}\n' + f"User message: {DEFAULT_USER_MSG}", ), ( - # Raise function handles missing param - NONE_RESPONSE, - ("0x01",), - identity, - raise_block_not_found_for_uncle_at_index, - BlockNotFound, + ERROR_RESPONSE_VALID_ID_NONE, + Web3RPCError, + f'{ERROR_RESPONSE_VALID_ID_NONE["error"]}\n' + f"User message: {DEFAULT_USER_MSG}", ), ( - # 0x response gets handled the same as a None response - ZERO_X_RESPONSE, - ("0x03"), - identity, - raise_transaction_not_found, - TransactionNotFound, - ), - ( - METHOD_NOT_FOUND_RESP_FORMAT, - (), - identity, - identity, + ERROR_RESPONSE_VALID_METHOD_UNAVAILABLE, MethodUnavailable, + METHOD_UNAVAILABLE_MSG, ), - ( - ETH_TESTER_METHOD_NOT_FOUND_RESP_FORMAT, - (), - identity, - identity, - Web3ValueError, - ), - ], + ), ) -def test_formatted_response_raises_errors( - w3, response, params, error_formatters, null_result_formatters, error -): - with pytest.raises(error): - w3.manager.formatted_response( - response, params, error_formatters, null_result_formatters - ) +def test_formatted_response_valid_error_object(response, w3, error, error_message): + with pytest.raises(error, match=re.escape(error_message)): + w3.manager.formatted_response(response, (), identity, identity) @pytest.mark.parametrize( - "response,params,error_formatters,null_result_formatters,error,error_message", + "response,null_result_formatters,error,error_message", [ ( - NONE_RESPONSE, - ("0x01",), + ERROR_RESPONSE_INVALID_CODE, identity, - raise_block_not_found_for_uncle_at_index, - BlockNotFound, - "Unknown block identifier or uncle index", + BadResponseFormat, + f"{DEFAULT_BAD_FORMAT_MSG} {INVALID_ERROR_CODE_MSG} " + f"The raw response is: {ERROR_RESPONSE_INVALID_CODE}", ), ( - NONE_RESPONSE, - ("0x01", "0x00"), + ERROR_RESPONSE_INVALID_MISSING_CODE, identity, - raise_block_not_found_for_uncle_at_index, - BlockNotFound, - "Uncle at index: 0 of block with id: '0x01' not found.", + BadResponseFormat, + f"{DEFAULT_BAD_FORMAT_MSG} {INVALID_ERROR_CODE_MSG} " + f"The raw response is: {ERROR_RESPONSE_INVALID_MISSING_CODE}", ), ( - ZERO_X_RESPONSE, - ("0x01",), + ERROR_RESPONSE_INVALID_MESSAGE, identity, - raise_transaction_not_found, - TransactionNotFound, - "Transaction with hash: '0x01' not found.", + BadResponseFormat, + f"{DEFAULT_BAD_FORMAT_MSG} {INVALID_ERROR_MESSAGE_MSG} " + f"The raw response is: {ERROR_RESPONSE_INVALID_MESSAGE}", ), ( - INVALID_JSONRPC_RESP_FORMAT, - (), - identity, + ERROR_RESPONSE_INVALID_MISSING_MESSAGE, identity, BadResponseFormat, - f"The response was in an unexpected format and unable to be parsed. " - f'The "jsonrpc" field must be present with a value of "2.0". ' - f"The raw response is: {INVALID_JSONRPC_RESP_FORMAT}", + f"{DEFAULT_BAD_FORMAT_MSG} {INVALID_ERROR_MESSAGE_MSG} " + f"The raw response is: {ERROR_RESPONSE_INVALID_MISSING_MESSAGE}", ), ( - UNEXPECTED_RESPONSE_FORMAT, - (), - identity, + ERROR_RESPONSE_INVALID_ID, identity, BadResponseFormat, - f"The response was in an unexpected format and unable to be parsed. " - f"The raw response is: {UNEXPECTED_RESPONSE_FORMAT}", + f'{DEFAULT_BAD_FORMAT_MSG} "id" must be an integer or a string ' + "representation of an integer. " + f"The raw response is: {ERROR_RESPONSE_INVALID_ID}", ), ( - ANOTHER_UNEXPECTED_RESP_FORMAT, - (), - identity, + ERROR_RESPONSE_INVALID_ERROR_OBJECT, identity, BadResponseFormat, - f"The response was in an unexpected format and unable to be parsed. " - f"The raw response is: {ANOTHER_UNEXPECTED_RESP_FORMAT}", + f"{DEFAULT_BAD_FORMAT_MSG} {ERROR_OBJ_MSG} The raw response is: " + f"{ERROR_RESPONSE_INVALID_ERROR_OBJECT}", ), ( - INVALID_CODE_RESP_FORMAT, - (), - identity, - identity, + # Invalid error response w/ null result formatters raises on invalid format + ERROR_RESPONSE_INVALID_ERROR_OBJECT, # raises on invalid error object + raise_block_not_found, # does not raise `BlockNotFound` BadResponseFormat, - re.escape( - f"The response was in an unexpected format and unable to be parsed. " - f"error['code'] must be an integer. " - f"The raw response is: {INVALID_CODE_RESP_FORMAT}" - ), + f"{DEFAULT_BAD_FORMAT_MSG} {ERROR_OBJ_MSG} The raw response is: " + f"{ERROR_RESPONSE_INVALID_ERROR_OBJECT}", ), + ], +) +def test_formatted_response_invalid_error_object( + response, w3, null_result_formatters, error, error_message +): + with pytest.raises(error, match=re.escape(error_message)): + w3.manager.formatted_response(response, (), identity, null_result_formatters) + + +@pytest.mark.parametrize( + "error_formatters,null_result_formatters,error,error_message", + ( ( - MISSING_CODE_RESP_FORMAT, - (), + # Valid error response with error formatters raises error in formatter + raise_contract_logic_error, identity, + ContractLogicError, + "Test contract logic error.", + ), + ( + # Non-null valid error response with no error formatters and null result + # formatters raises generic `Web3RPCError`, not `BlockNotFound` identity, - BadResponseFormat, - re.escape( - f"The response was in an unexpected format and unable to be parsed. " - f"error['code'] must be an integer. " - f"The raw response is: {MISSING_CODE_RESP_FORMAT}" - ), + raise_block_not_found, + Web3RPCError, + f'{VALID_ERROR_RESPONSE["error"]}\nUser message: {DEFAULT_USER_MSG}', ), ( - INVALID_MESSAGE_RESP_FORMAT, - (), + # Valid error response with no null result formatters raises `Web3RPCError` identity, identity, - BadResponseFormat, - re.escape( - f"The response was in an unexpected format and unable to be parsed. " - f"error['message'] must be a string. " - f"The raw response is: {INVALID_MESSAGE_RESP_FORMAT}" - ), + Web3RPCError, + f'{VALID_ERROR_RESPONSE["error"]}\nUser message: {DEFAULT_USER_MSG}', ), - ], + ), ) -def test_formatted_response_raises_correct_error_message( - response, w3, params, error_formatters, null_result_formatters, error, error_message +def test_formatted_response_error_responses_with_formatters_raise_expected_exceptions( + w3, error_formatters, null_result_formatters, error, error_message ): - with pytest.raises(error, match=error_message): + with pytest.raises(error, match=re.escape(error_message)): w3.manager.formatted_response( - response, params, error_formatters, null_result_formatters + VALID_ERROR_RESPONSE, (), error_formatters, null_result_formatters ) @pytest.mark.parametrize( - "response,params,error_formatters,null_result_formatters,expected", - [ + "params,null_result_formatters,error,error_message", + ( + # raise via null_result_formatters ( - # Response with a result of None doesn't raise - # if there is no null result formatter - NONE_RESPONSE, - ("0x03"), - identity, - identity, - NONE_RESPONSE["result"], + (), + # test raise_block_not_found + raise_block_not_found, + BlockNotFound, + "Unknown block identifier", ), ( - # Response with a result of 0x doesn't raise - # if there is no null result formatter - ZERO_X_RESPONSE, - ("0x03"), - identity, - identity, - ZERO_X_RESPONSE["result"], + ("0x03",), # test with param + # test raise_block_not_found + raise_block_not_found, + BlockNotFound, + "Block with id: '0x03' not found.", ), - ], + ( + (), + # test raise_block_not_found_for_uncle_at_index + raise_block_not_found_for_uncle_at_index, + BlockNotFound, + "Unknown block identifier or uncle index", + ), + ( + ("0x01",), # test handles missing param + # test raise_block_not_found_for_uncle_at_index + raise_block_not_found_for_uncle_at_index, + BlockNotFound, + "Unknown block identifier or uncle index", + ), + ( + ("0x01", "0x00"), # both params + # test raise_block_not_found_for_uncle_at_index + raise_block_not_found_for_uncle_at_index, + BlockNotFound, + "Uncle at index: 0 of block with id: '0x01' not found.", + ), + ( + (), + # test raise_transaction_not_found + raise_transaction_not_found, + TransactionNotFound, + "Unknown transaction hash", + ), + ( + ("0x01",), + # test raise_transaction_not_found + raise_transaction_not_found, + TransactionNotFound, + "Transaction with hash: '0x01' not found.", + ), + ), ) -def test_formatted_response( - response, w3, params, error_formatters, null_result_formatters, expected +def test_formatted_response_null_and_0x_results_with_formatters( + w3, params, null_result_formatters, error, error_message ): - formatted_resp = w3.manager.formatted_response( - response, params, error_formatters, null_result_formatters - ) - assert formatted_resp == expected + with pytest.raises(error, match=re.escape(error_message)): + # test null result response + w3.manager.formatted_response( + VALID_RESPONSE_NULL_RESULT, params, identity, null_result_formatters + ) + + with pytest.raises(error, match=re.escape(error_message)): + # test 0x result response + w3.manager.formatted_response( + VALID_RESPONSE_0x_RESULT, params, identity, null_result_formatters + ) diff --git a/tests/core/middleware/test_formatting_middleware.py b/tests/core/middleware/test_formatting_middleware.py index c82efd34f1..4a9b7a2b99 100644 --- a/tests/core/middleware/test_formatting_middleware.py +++ b/tests/core/middleware/test_formatting_middleware.py @@ -7,7 +7,7 @@ Web3, ) from web3.exceptions import ( - Web3ValueError, + Web3RPCError, ) from web3.middleware import ( FormattingMiddlewareBuilder, @@ -98,6 +98,6 @@ def test_formatting_middleware_error_formatters(w3, request_mocker): expected = "error" with request_mocker(w3, mock_errors={"test_endpoint": {"message": "error"}}): - with pytest.raises(Web3ValueError) as err: + with pytest.raises(Web3RPCError) as err: w3.manager.request_blocking("test_endpoint", []) assert str(err.value) == expected diff --git a/web3/_utils/contracts.py b/web3/_utils/contracts.py index b914836040..6fdbe02f11 100644 --- a/web3/_utils/contracts.py +++ b/web3/_utils/contracts.py @@ -75,7 +75,7 @@ abi_string_to_text, ) from web3.exceptions import ( - BlockNumberOutofRange, + BlockNumberOutOfRange, Web3TypeError, Web3ValidationError, Web3ValueError, @@ -424,7 +424,7 @@ def parse_block_identifier( ): return w3.eth.get_block(block_identifier)["number"] else: - raise BlockNumberOutofRange + raise BlockNumberOutOfRange def parse_block_identifier_int(w3: "Web3", block_identifier_int: int) -> BlockNumber: @@ -434,7 +434,7 @@ def parse_block_identifier_int(w3: "Web3", block_identifier_int: int) -> BlockNu last_block = w3.eth.get_block("latest")["number"] block_num = last_block + block_identifier_int + 1 if block_num < 0: - raise BlockNumberOutofRange + raise BlockNumberOutOfRange return BlockNumber(block_num) @@ -453,7 +453,7 @@ async def async_parse_block_identifier( requested_block = await async_w3.eth.get_block(block_identifier) return requested_block["number"] else: - raise BlockNumberOutofRange + raise BlockNumberOutOfRange async def async_parse_block_identifier_int( @@ -466,5 +466,5 @@ async def async_parse_block_identifier_int( last_block_num = last_block["number"] block_num = last_block_num + block_identifier_int + 1 if block_num < 0: - raise BlockNumberOutofRange + raise BlockNumberOutOfRange return BlockNumber(block_num) diff --git a/web3/_utils/module_testing/eth_module.py b/web3/_utils/module_testing/eth_module.py index 6d16ba30a5..558df11956 100644 --- a/web3/_utils/module_testing/eth_module.py +++ b/web3/_utils/module_testing/eth_module.py @@ -83,7 +83,9 @@ TooManyRequests, TransactionNotFound, TransactionTypeMismatch, + Web3RPCError, Web3ValidationError, + Web3ValueError, ) from web3.middleware import ( ExtraDataToPOAMiddleware, @@ -399,7 +401,7 @@ async def test_invalid_eth_sign_typed_data( } """ with pytest.raises( - ValueError, + Web3ValueError, match=r".*Expected 2 items for array type Person\[2\], got 1 items.*", ): await async_w3.eth.sign_typed_data( @@ -1137,7 +1139,7 @@ async def test_eth_get_raw_transaction_by_block_raises_error_block_identifier( ) -> None: unknown_identifier = "unknown" with pytest.raises( - ValueError, + Web3ValueError, match=( "Value did not match any of the recognized block identifiers: " f"{unknown_identifier}" @@ -1537,7 +1539,7 @@ async def test_eth_call_offchain_lookup_raises_if_max_redirects_is_less_than_4( default_max_redirects = async_w3.provider.ccip_read_max_redirects async_w3.provider.ccip_read_max_redirects = max_redirects - with pytest.raises(ValueError, match="at least 4"): + with pytest.raises(Web3ValueError, match="at least 4"): await async_offchain_lookup_contract.caller().testOffchainLookup( OFFCHAIN_LOOKUP_TEST_DATA ) @@ -1836,7 +1838,7 @@ async def test_async_eth_get_logs_without_logs( "fromBlock": async_block_with_txn_with_log["number"], "toBlock": BlockNumber(async_block_with_txn_with_log["number"] - 1), } - with pytest.raises(ValueError): + with pytest.raises(Web3RPCError): result = await async_w3.eth.get_logs(filter_params) # Test with `address` @@ -2273,7 +2275,7 @@ async def test_async_eth_replace_transaction_underpriced( txn_params["maxFeePerGas"] = one_gwei_in_wei txn_params["maxPriorityFeePerGas"] = one_gwei_in_wei - with pytest.raises(ValueError, match="replacement transaction underpriced"): + with pytest.raises(Web3RPCError, match="replacement transaction underpriced"): await async_w3.eth.replace_transaction(txn_hash, txn_params) @flaky_geth_dev_mining @@ -2319,7 +2321,7 @@ async def test_async_eth_replace_transaction_already_mined( txn_params["maxFeePerGas"] = async_w3.to_wei(3, "gwei") txn_params["maxPriorityFeePerGas"] = async_w3.to_wei(2, "gwei") - with pytest.raises(ValueError, match="Supplied transaction with hash"): + with pytest.raises(Web3ValueError, match="Supplied transaction with hash"): await async_w3.eth.replace_transaction(txn_hash, txn_params) @flaky_geth_dev_mining @@ -2341,7 +2343,7 @@ async def test_async_eth_replace_transaction_incorrect_nonce( txn_params["maxFeePerGas"] = async_w3.to_wei(3, "gwei") txn_params["maxPriorityFeePerGas"] = async_w3.to_wei(2, "gwei") txn_params["nonce"] = Nonce(txn["nonce"] + 1) - with pytest.raises(ValueError): + with pytest.raises(Web3ValueError): await async_w3.eth.replace_transaction(txn_hash, txn_params) @flaky_geth_dev_mining @@ -2361,7 +2363,7 @@ async def test_async_eth_replace_transaction_gas_price_too_low( txn_hash = await async_w3.eth.send_transaction(txn_params) txn_params["gasPrice"] = async_w3.to_wei(1, "gwei") - with pytest.raises(ValueError): + with pytest.raises(Web3ValueError): await async_w3.eth.replace_transaction(txn_hash, txn_params) @flaky_geth_dev_mining @@ -2965,7 +2967,7 @@ def test_invalid_eth_sign_typed_data( } """ with pytest.raises( - ValueError, + Web3ValueError, match=r".*Expected 2 items for array type Person\[2\], got 1 items.*", ): w3.eth.sign_typed_data( @@ -3481,7 +3483,7 @@ def test_eth_replace_transaction_underpriced( txn_params["maxFeePerGas"] = one_gwei_in_wei txn_params["maxPriorityFeePerGas"] = one_gwei_in_wei - with pytest.raises(ValueError, match="replacement transaction underpriced"): + with pytest.raises(Web3RPCError, match="replacement transaction underpriced"): w3.eth.replace_transaction(txn_hash, txn_params) @flaky_geth_dev_mining @@ -3521,7 +3523,7 @@ def test_eth_replace_transaction_already_mined( txn_params["maxFeePerGas"] = w3.to_wei(3, "gwei") txn_params["maxPriorityFeePerGas"] = w3.to_wei(2, "gwei") - with pytest.raises(ValueError, match="Supplied transaction with hash"): + with pytest.raises(Web3ValueError, match="Supplied transaction with hash"): w3.eth.replace_transaction(txn_hash, txn_params) @flaky_geth_dev_mining @@ -3542,7 +3544,7 @@ def test_eth_replace_transaction_incorrect_nonce( txn_params["maxFeePerGas"] = w3.to_wei(3, "gwei") txn_params["maxPriorityFeePerGas"] = w3.to_wei(2, "gwei") txn_params["nonce"] = Nonce(txn["nonce"] + 1) - with pytest.raises(ValueError): + with pytest.raises(Web3ValueError): w3.eth.replace_transaction(txn_hash, txn_params) @flaky_geth_dev_mining @@ -3559,7 +3561,7 @@ def test_eth_replace_transaction_gas_price_too_low( txn_hash = w3.eth.send_transaction(txn_params) txn_params["gasPrice"] = w3.to_wei(1, "gwei") - with pytest.raises(ValueError): + with pytest.raises(Web3ValueError): w3.eth.replace_transaction(txn_hash, txn_params) @flaky_geth_dev_mining @@ -4003,7 +4005,7 @@ def test_eth_call_offchain_lookup_raises_if_max_redirects_is_less_than_4( default_max_redirects = w3.provider.ccip_read_max_redirects w3.provider.ccip_read_max_redirects = max_redirects - with pytest.raises(ValueError, match="at least 4"): + with pytest.raises(Web3ValueError, match="at least 4"): offchain_lookup_contract.functions.testOffchainLookup( OFFCHAIN_LOOKUP_TEST_DATA ).call() @@ -4549,7 +4551,7 @@ def test_eth_get_logs_without_logs( "fromBlock": block_with_txn_with_log["number"], "toBlock": BlockNumber(block_with_txn_with_log["number"] - 1), } - with pytest.raises(ValueError): + with pytest.raises(Web3RPCError): w3.eth.get_logs(filter_params) # Test with `address` @@ -4768,7 +4770,7 @@ def test_eth_get_raw_transaction_by_block_raises_error_block_identifier( ) -> None: unknown_identifier = "unknown" with pytest.raises( - ValueError, + Web3ValueError, match=( "Value did not match any of the recognized block identifiers: " f"{unknown_identifier}" diff --git a/web3/exceptions.py b/web3/exceptions.py index ff51366967..f810cdf453 100644 --- a/web3/exceptions.py +++ b/web3/exceptions.py @@ -13,6 +13,7 @@ from web3.types import ( BlockData, + RPCResponse, ) @@ -30,6 +31,8 @@ class Web3Exception(Exception): # deal with other exceptions """ + user_message: Optional[str] = None + def __init__( self, *args: Any, @@ -40,6 +43,12 @@ def __init__( # Assign properties of Web3Exception self.user_message = user_message + def __str__(self) -> str: + # append a clarifying user message if one is provided + return super().__str__() + ( + f"\nUser message: {self.user_message}" if self.user_message else "" + ) + class Web3AssertionError(Web3Exception, AssertionError): """ @@ -77,7 +86,7 @@ class BadFunctionCallOutput(Web3Exception): """ -class BlockNumberOutofRange(Web3Exception): +class BlockNumberOutOfRange(Web3Exception): """ block_identifier passed does not match known block. """ @@ -218,7 +227,7 @@ class TimeExhausted(Web3Exception): class TransactionNotFound(Web3Exception): """ - Raised when a tx hash used to lookup a tx in a jsonrpc call cannot be found. + Raised when a tx hash used to look up a tx in a jsonrpc call cannot be found. """ @@ -317,7 +326,44 @@ class BadResponseFormat(Web3Exception): """ -class MethodUnavailable(Web3Exception): +class Web3RPCError(Web3Exception): + """ + Raised when a JSON-RPC response contains an error field. + """ + + def __init__( + self, + message: str, + rpc_response: Optional[RPCResponse] = None, + user_message: Optional[str] = None, + ) -> None: + if user_message is None: + user_message = ( + "An RPC error was returned by the node. Check the message provided in " + "the error and any available logs for more information." + ) + + super().__init__( + message, + user_message=user_message, + ) + self.message = message + self.rpc_response = rpc_response + + +class MethodUnavailable(Web3RPCError): """ Raised when the method is not available on the node """ + + def __init__( + self, + message: str, + rpc_response: Optional[RPCResponse] = None, + user_message: Optional[str] = "This method is not available.", + ) -> None: + super().__init__( + message, + rpc_response=rpc_response, + user_message=user_message, + ) diff --git a/web3/manager.py b/web3/manager.py index 37ada37d14..b689981e1a 100644 --- a/web3/manager.py +++ b/web3/manager.py @@ -35,8 +35,8 @@ BadResponseFormat, MethodUnavailable, ProviderConnectionError, + Web3RPCError, Web3TypeError, - Web3ValueError, ) from web3.middleware import ( AttributeDictMiddleware, @@ -87,6 +87,7 @@ def _raise_bad_response_format(response: RPCResponse, error: str = "") -> None: raw_response = f"The raw response is: {response}" if error is not None and error != "": + error = error[:-1] if error.endswith(".") else error message = f"{message} {error}. {raw_response}" else: message = f"{message} {raw_response}" @@ -117,6 +118,99 @@ def apply_null_result_formatters( return response +def _validate_subscription_fields(response: RPCResponse) -> None: + params = response["params"] + subscription = params["subscription"] + if not isinstance(subscription, str) and not len(subscription) == 34: + _raise_bad_response_format( + response, "eth_subscription 'params' must include a 'subscription' field." + ) + + +def _validate_response( + response: RPCResponse, + error_formatters: Optional[Callable[..., Any]], + is_subscription_response: bool = False, +) -> None: + if "jsonrpc" not in response or response["jsonrpc"] != "2.0": + _raise_bad_response_format( + response, 'The "jsonrpc" field must be present with a value of "2.0".' + ) + + response_id = response.get("id") + if "id" in response: + int_error_msg = ( + '"id" must be an integer or a string representation of an integer.' + ) + if response_id is None and "error" in response: + # errors can sometimes have null `id`, according to the JSON-RPC spec + pass + elif not isinstance(response_id, (str, int)): + _raise_bad_response_format(response, int_error_msg) + elif isinstance(response_id, str): + try: + int(response_id) + except ValueError: + _raise_bad_response_format(response, int_error_msg) + elif is_subscription_response: + # if `id` is not present, this must be a subscription response + _validate_subscription_fields(response) + else: + _raise_bad_response_format( + response, + 'Response must include an "id" field or be formatted as an ' + "`eth_subscription` response.", + ) + + if all(key in response for key in {"error", "result"}): + _raise_bad_response_format( + response, 'Response cannot include both "error" and "result".' + ) + elif ( + not any(key in response for key in {"error", "result"}) + and not is_subscription_response + ): + _raise_bad_response_format( + response, 'Response must include either "error" or "result".' + ) + elif "error" in response: + error = response["error"] + + # raise the error when the value is a string + if error is None or not isinstance(error, dict): + _raise_bad_response_format( + response, + 'response["error"] must be a valid object as defined by the ' + "JSON-RPC 2.0 specification.", + ) + + # errors must include an integer code + code = error.get("code") + if not isinstance(code, int): + _raise_bad_response_format( + response, 'error["code"] is required and must be an integer value.' + ) + elif code == METHOD_NOT_FOUND: + raise MethodUnavailable( + repr(error), + user_message="Check your node provider or your client's API docs to " + "see what methods are supported and / or currently enabled.", + ) + + # errors must include a message + error_message = error.get("message") + if not isinstance(error_message, str): + _raise_bad_response_format( + response, 'error["message"] is required and must be a string value.' + ) + + apply_error_formatters(error_formatters, response) + raise Web3RPCError(repr(error), rpc_response=response) + + elif "result" not in response and not is_subscription_response: + _raise_bad_response_format(response) + + class RequestManager: logger = logging.getLogger("web3.manager.RequestManager") @@ -210,83 +304,34 @@ def formatted_response( error_formatters: Optional[Callable[..., Any]] = None, null_result_formatters: Optional[Callable[..., Any]] = None, ) -> Any: - # jsonrpc is not enforced (as per the spec) but if present, it must be 2.0 - if "jsonrpc" in response and response["jsonrpc"] != "2.0": - _raise_bad_response_format( - response, 'The "jsonrpc" field must be present with a value of "2.0"' - ) + is_subscription_response = ( + response.get("method") == "eth_subscription" + and response.get("params") is not None + and response["params"].get("subscription") is not None + and response["params"].get("result") is not None + ) - # id is not enforced (as per the spec) but if present, it must be a - # string or integer - # TODO: v7 - enforce id per the spec - if "id" in response: - response_id = response["id"] - # id is always None for errors - if response_id is None and "error" not in response: - _raise_bad_response_format( - response, '"id" must be None when an error is present' - ) - elif not isinstance(response_id, (str, int, type(None))): - _raise_bad_response_format(response, '"id" must be a string or integer') - - # Response may not include both "error" and "result" - if "error" in response and "result" in response: - _raise_bad_response_format( - response, 'Response cannot include both "error" and "result"' - ) + _validate_response( + response, + error_formatters, + is_subscription_response=is_subscription_response, + ) - # Format and validate errors - elif "error" in response: - error = response.get("error") - # Raise the error when the value is a string - if error is None or isinstance(error, str): - raise Web3ValueError(error) - - # Errors must include an integer code - code = error.get("code") - if not isinstance(code, int): - _raise_bad_response_format(response, "error['code'] must be an integer") - elif code == METHOD_NOT_FOUND: - raise MethodUnavailable( - error, - user_message="Check your node provider's API docs to see what " - "methods are supported", - ) - - # Errors must include a message - if not isinstance(error.get("message"), str): - _raise_bad_response_format( - response, "error['message'] must be a string" - ) - - apply_error_formatters(error_formatters, response) - - raise Web3ValueError(error) - - # Format and validate results - elif "result" in response: + # format results + if "result" in response: # Null values for result should apply null_result_formatters # Skip when result not present in the response (fallback to False) if response.get("result", False) in NULL_RESPONSES: apply_null_result_formatters(null_result_formatters, response, params) return response.get("result") - # Response from eth_subscription includes response["params"]["result"] - elif ( - response.get("method") == "eth_subscription" - and response.get("params") is not None - and response["params"].get("subscription") is not None - and response["params"].get("result") is not None - ): + # response from eth_subscription includes response["params"]["result"] + elif is_subscription_response: return { "subscription": response["params"]["subscription"], "result": response["params"]["result"], } - # Any other response type raises BadResponseFormat - else: - _raise_bad_response_format(response) - def request_blocking( self, method: Union[RPCEndpoint, Callable[..., RPCEndpoint]], diff --git a/web3/types.py b/web3/types.py index 90c35e755d..44160fbab7 100644 --- a/web3/types.py +++ b/web3/types.py @@ -295,7 +295,7 @@ class GethSyncingSubscriptionResponse(SubscriptionResponse): class RPCResponse(TypedDict, total=False): - error: Union[RPCError, str] + error: RPCError id: RPCId jsonrpc: Literal["2.0"] result: Any From b1477dc9e824ec1f14d0abfdc331f5430a0038a1 Mon Sep 17 00:00:00 2001 From: fselmo Date: Mon, 22 Apr 2024 15:50:23 -0600 Subject: [PATCH 2/8] Use request ``id`` concept for ``EthereumTesterProvider`` --- web3/providers/eth_tester/main.py | 47 +++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/web3/providers/eth_tester/main.py b/web3/providers/eth_tester/main.py index 7f23ce8ae5..48fdd260fb 100644 --- a/web3/providers/eth_tester/main.py +++ b/web3/providers/eth_tester/main.py @@ -7,6 +7,7 @@ Literal, Optional, Union, + cast, ) from eth_abi import ( @@ -59,6 +60,7 @@ class AsyncEthereumTesterProvider(AsyncBaseProvider): + _current_request_id = 0 _middleware = ( default_transaction_fields_middleware, ethereum_tester_middleware, @@ -99,13 +101,22 @@ async def request_func( return self._request_func_cache[-1] async def make_request(self, method: RPCEndpoint, params: Any) -> RPCResponse: - return _make_request(method, params, self.api_endpoints, self.ethereum_tester) + response = _make_request( + method, + params, + self.api_endpoints, + self.ethereum_tester, + repr(self._current_request_id), + ) + self._current_request_id += 1 + return response async def is_connected(self, show_traceback: bool = False) -> Literal[True]: return True class EthereumTesterProvider(BaseProvider): + _current_request_id = 0 _middleware = ( default_transaction_fields_middleware, ethereum_tester_middleware, @@ -173,23 +184,32 @@ def request_func( return self._request_func_cache[-1] def make_request(self, method: RPCEndpoint, params: Any) -> RPCResponse: - return _make_request(method, params, self.api_endpoints, self.ethereum_tester) + response = _make_request( + method, + params, + self.api_endpoints, + self.ethereum_tester, + repr(self._current_request_id), + ) + self._current_request_id += 1 + return response def is_connected(self, show_traceback: bool = False) -> Literal[True]: return True -def _make_response(result: Any, message: str = "") -> RPCResponse: +def _make_response(result: Any, response_id: str, message: str = "") -> RPCResponse: if isinstance(result, Exception): - return RPCResponse( + return cast( + RPCResponse, { - "id": 1, + "id": response_id, "jsonrpc": "2.0", - "error": RPCError({"code": -32601, "message": message}), - } + "error": cast(RPCError, {"code": -32601, "message": message}), + }, ) - return RPCResponse({"id": 1, "jsonrpc": "2.0", "result": result}) + return cast(RPCResponse, {"id": response_id, "jsonrpc": "2.0", "result": result}) def _make_request( @@ -197,6 +217,7 @@ def _make_request( params: Any, api_endpoints: Dict[str, Dict[str, Any]], ethereum_tester_instance: "EthereumTester", + request_id: str, ) -> RPCResponse: # do not import eth_tester derivatives until runtime, # it is not a default dependency @@ -209,11 +230,15 @@ def _make_request( try: delegator = api_endpoints[namespace][endpoint] except KeyError as e: - return _make_response(e, f"Unknown RPC Endpoint: {method}") + return _make_response(e, request_id, message=f"Unknown RPC Endpoint: {method}") try: response = delegator(ethereum_tester_instance, params) except NotImplementedError as e: - return _make_response(e, f"RPC Endpoint has not been implemented: {method}") + return _make_response( + e, + request_id, + message=f"RPC Endpoint has not been implemented: {method}", + ) except TransactionFailed as e: first_arg = e.args[0] try: @@ -230,4 +255,4 @@ def _make_request( reason = first_arg raise TransactionFailed(f"execution reverted: {reason}") else: - return _make_response(response) + return _make_response(response, request_id) From e1f1ea37f0afd313e74764eed9f29ab16de45d47 Mon Sep 17 00:00:00 2001 From: fselmo Date: Mon, 22 Apr 2024 16:40:17 -0600 Subject: [PATCH 3/8] Distinguish between method unavailable and method not supported --- web3/eth/async_eth.py | 9 ++++---- web3/eth/eth.py | 4 ++-- web3/exceptions.py | 54 +++++++++++++++++++------------------------ web3/manager.py | 1 + 4 files changed, 32 insertions(+), 36 deletions(-) diff --git a/web3/eth/async_eth.py b/web3/eth/async_eth.py index 3cab64c192..21f66a4943 100644 --- a/web3/eth/async_eth.py +++ b/web3/eth/async_eth.py @@ -57,12 +57,13 @@ BaseEth, ) from web3.exceptions import ( - MethodUnavailable, + MethodNotSupported, OffchainLookup, TimeExhausted, TooManyRequests, TransactionIndexingInProgress, TransactionNotFound, + Web3RPCError, Web3ValueError, ) from web3.method import ( @@ -194,7 +195,7 @@ async def max_priority_fee(self) -> Wei: """ try: return await self._max_priority_fee() - except (ValueError, MethodUnavailable): + except Web3RPCError: warnings.warn( "There was an issue with the method eth_maxPriorityFeePerGas. " "Calculating using eth_feeHistory.", @@ -736,7 +737,7 @@ async def subscribe( ] = None, ) -> HexStr: if not isinstance(self.w3.provider, PersistentConnectionProvider): - raise MethodUnavailable( + raise MethodNotSupported( "eth_subscribe is only supported with providers that support " "persistent connections." ) @@ -753,7 +754,7 @@ async def subscribe( async def unsubscribe(self, subscription_id: HexStr) -> bool: if not isinstance(self.w3.provider, PersistentConnectionProvider): - raise MethodUnavailable( + raise MethodNotSupported( "eth_unsubscribe is only supported with providers that support " "persistent connections." ) diff --git a/web3/eth/eth.py b/web3/eth/eth.py index efe9b7bcc3..5eb65ea66b 100644 --- a/web3/eth/eth.py +++ b/web3/eth/eth.py @@ -57,12 +57,12 @@ BaseEth, ) from web3.exceptions import ( - MethodUnavailable, OffchainLookup, TimeExhausted, TooManyRequests, TransactionIndexingInProgress, TransactionNotFound, + Web3RPCError, Web3ValueError, ) from web3.method import ( @@ -187,7 +187,7 @@ def max_priority_fee(self) -> Wei: """ try: return self._max_priority_fee() - except (ValueError, MethodUnavailable): + except Web3RPCError: warnings.warn( "There was an issue with the method eth_maxPriorityFeePerGas. " "Calculating using eth_feeHistory.", diff --git a/web3/exceptions.py b/web3/exceptions.py index f810cdf453..8b289567ca 100644 --- a/web3/exceptions.py +++ b/web3/exceptions.py @@ -78,6 +78,12 @@ class Web3TypeError(Web3Exception, TypeError): """ +class MethodNotSupported(Web3Exception): + """ + Raised when a method is not supported by the provider. + """ + + class BadFunctionCallOutput(Web3Exception): """ We failed to decode ABI output. @@ -225,25 +231,6 @@ class TimeExhausted(Web3Exception): """ -class TransactionNotFound(Web3Exception): - """ - Raised when a tx hash used to look up a tx in a jsonrpc call cannot be found. - """ - - -class TransactionIndexingInProgress(Web3Exception): - """ - Raised when a transaction receipt is not yet available due to transaction indexing - still being in progress. - """ - - -class BlockNotFound(Web3Exception): - """ - Raised when the block id used to lookup a block in a jsonrpc call cannot be found. - """ - - class InfuraProjectIdNotFound(Web3Exception): """ Raised when there is no Infura Project Id set. @@ -356,14 +343,21 @@ class MethodUnavailable(Web3RPCError): Raised when the method is not available on the node """ - def __init__( - self, - message: str, - rpc_response: Optional[RPCResponse] = None, - user_message: Optional[str] = "This method is not available.", - ) -> None: - super().__init__( - message, - rpc_response=rpc_response, - user_message=user_message, - ) + +class TransactionNotFound(Web3RPCError): + """ + Raised when a tx hash used to look up a tx in a jsonrpc call cannot be found. + """ + + +class TransactionIndexingInProgress(Web3RPCError): + """ + Raised when a transaction receipt is not yet available due to transaction indexing + still being in progress. + """ + + +class BlockNotFound(Web3RPCError): + """ + Raised when the block id used to look up a block in a jsonrpc call cannot be found. + """ diff --git a/web3/manager.py b/web3/manager.py index b689981e1a..75bba44b1d 100644 --- a/web3/manager.py +++ b/web3/manager.py @@ -193,6 +193,7 @@ def _validate_response( elif code == METHOD_NOT_FOUND: raise MethodUnavailable( repr(error), + rpc_response=response, user_message="Check your node provider or your client's API docs to " "see what methods are supported and / or currently enabled.", ) From 36c2e2e5e8b2be3ba34e1b232ebc3f6b7ac3fb35 Mon Sep 17 00:00:00 2001 From: fselmo Date: Tue, 23 Apr 2024 15:08:37 -0600 Subject: [PATCH 4/8] Fix filter middleware to mimic proper RPC response --- web3/middleware/filter.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/web3/middleware/filter.py b/web3/middleware/filter.py index 6824229935..97ea8efc4c 100644 --- a/web3/middleware/filter.py +++ b/web3/middleware/filter.py @@ -589,6 +589,10 @@ async def async_block_hashes_in_range( AsyncFilter = Union[AsyncRequestLogs, AsyncRequestBlocks] +def _simulate_rpc_response_with_result(filter_id: str) -> "RPCResponse": + return {"jsonrpc": "2.0", "id": -1, "result": filter_id} + + class LocalFilterMiddleware(Web3Middleware): def __init__(self, w3: Union["Web3", "AsyncWeb3"]): self.filters: Dict[str, SyncFilter] = {} @@ -615,7 +619,7 @@ def middleware(method: "RPCEndpoint", params: Any) -> "RPCResponse": raise NotImplementedError(method) self.filters[filter_id] = _filter - return {"result": filter_id} + return _simulate_rpc_response_with_result(filter_id) elif method in FILTER_CHANGES_METHODS: _filter_id = params[0] @@ -626,12 +630,16 @@ def middleware(method: "RPCEndpoint", params: Any) -> "RPCResponse": _filter = self.filters[_filter_id] if method == RPC.eth_getFilterChanges: - return {"result": next(_filter.filter_changes)} + return _simulate_rpc_response_with_result( + next(_filter.filter_changes) # type: ignore + ) elif method == RPC.eth_getFilterLogs: # type ignored b/c logic prevents RequestBlocks which # doesn't implement get_logs - return {"result": _filter.get_logs()} # type: ignore + return _simulate_rpc_response_with_result( + _filter.get_logs() # type: ignore + ) else: raise NotImplementedError(method) else: @@ -663,7 +671,7 @@ async def middleware(method: "RPCEndpoint", params: Any) -> "RPCResponse": raise NotImplementedError(method) self.async_filters[filter_id] = _filter - return {"result": filter_id} + return _simulate_rpc_response_with_result(filter_id) elif method in FILTER_CHANGES_METHODS: _filter_id = params[0] @@ -674,12 +682,16 @@ async def middleware(method: "RPCEndpoint", params: Any) -> "RPCResponse": _filter = self.async_filters[_filter_id] if method == RPC.eth_getFilterChanges: - return {"result": await _filter.filter_changes.__anext__()} + return _simulate_rpc_response_with_result( + await _filter.filter_changes.__anext__() # type: ignore + ) elif method == RPC.eth_getFilterLogs: # type ignored b/c logic prevents RequestBlocks which # doesn't implement get_logs - return {"result": await _filter.get_logs()} # type: ignore + return _simulate_rpc_response_with_result( + await _filter.get_logs() # type: ignore + ) else: raise NotImplementedError(method) else: From dc44990ea9175fdae6e6517e8e2a139a4e303bc2 Mon Sep 17 00:00:00 2001 From: fselmo Date: Tue, 23 Apr 2024 15:48:44 -0600 Subject: [PATCH 5/8] relevant newsfragments for #3359; update migration guide --- docs/v7_migration.rst | 37 +++++++++++++++++++++++++++++++++ newsfragments/3359.breaking.rst | 1 + newsfragments/3359.feature.rst | 1 + 3 files changed, 39 insertions(+) create mode 100644 newsfragments/3359.breaking.rst create mode 100644 newsfragments/3359.feature.rst diff --git a/docs/v7_migration.rst b/docs/v7_migration.rst index 1e1cc34d8e..6f3bd99d01 100644 --- a/docs/v7_migration.rst +++ b/docs/v7_migration.rst @@ -189,6 +189,41 @@ keys in the dictionary should be camelCase. This is because the dictionary is pa directly to the JSON-RPC request, where the keys are expected to be in camelCase. +Changes to Exception Handling +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +All Python standard library exceptions that were raised from within web3.py have +been replaced with custom ``Web3Exception`` classes. This change allows for better +control over exception handling, being able to distinguish between exceptions raised +by web3.py and those raised from elsewhere in a codebase. The following exceptions +have been replaced: + +- ``AssertionError`` -> ``Web3AssertionError`` +- ``ValueError`` -> ``Web3ValueError`` +- ``TypeError`` -> ``Web3TypeError`` +- ``AttributeError`` -> ``Web3AttributeError`` + +A new ``MethodNotSupported`` exception is now raised when a method is not supported by +web3.py. This allows a user to distinguish between when a method is not available on +the current provider, ``MethodUnavailable``, and when a method is not supported by +web3.py under certain conditions, ``MethodNotSupported``. + + +JSON-RPC Error Handling +``````````````````````` + +Rather than a ``ValueError`` being replaced with a ``Web3ValueError`` when a JSON-RPC +response comes back with an ``error`` object, a new ``Web3RPCError`` exception is +now raised to provide more distinction for JSON-RPC error responses. Some previously +existing exceptions now extend from this class since they too are related to JSON-RPC +errors: + +- ``MethodUnavailable`` +- ``BlockNotFound`` +- ``TransactionNotFound`` +- ``TransactionIndexingInProgress`` + + Miscellaneous Changes ~~~~~~~~~~~~~~~~~~~~~ @@ -207,3 +242,5 @@ Miscellaneous Changes without checking if the ``geth.ipc`` file exists. - ``Web3.is_address()`` returns ``True`` for non-checksummed addresses. - ``Contract.encodeABI()`` has been renamed to ``Contract.encode_abi()``. +- JSON-RPC responses are now more strictly validated against the JSON-RPC 2.0 + specification while providing more informative error messages for invalid responses. diff --git a/newsfragments/3359.breaking.rst b/newsfragments/3359.breaking.rst new file mode 100644 index 0000000000..19d180cce4 --- /dev/null +++ b/newsfragments/3359.breaking.rst @@ -0,0 +1 @@ +Validate JSON-RPC responses more strictly against the JSON-RPC 2.0 specifications. ``BlockNumberOutofRange`` -> ``BlockNumberOutOfRange``. diff --git a/newsfragments/3359.feature.rst b/newsfragments/3359.feature.rst new file mode 100644 index 0000000000..1360ef5420 --- /dev/null +++ b/newsfragments/3359.feature.rst @@ -0,0 +1 @@ +Raise ``Web3RPCError`` on JSON-RPC errors rather than ``Web3ValueError``. Raise ``MethodNotSupported`` exception when a method is not supported within *web3.py*; keep ``MethodUnavailable`` for when a method is not available on the current provider (JSON-RPC error). From 755129a6b1dd724b258d66589ddc36e173699c1d Mon Sep 17 00:00:00 2001 From: fselmo Date: Wed, 24 Apr 2024 09:23:28 -0600 Subject: [PATCH 6/8] Clean up duplicate test case; further organize response formatters test --- .../core/manager/test_response_formatters.py | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/tests/core/manager/test_response_formatters.py b/tests/core/manager/test_response_formatters.py index 17ea575563..952499e585 100644 --- a/tests/core/manager/test_response_formatters.py +++ b/tests/core/manager/test_response_formatters.py @@ -84,20 +84,23 @@ {"error": {"code": -32601, "message": (METHOD_UNAVAILABLE_MSG)}}, ) ERROR_RESPONSE_INVALID_ID = merge(VALID_ERROR_RESPONSE, {"id": b"invalid"}) -ERROR_RESPONSE_INVALID_ERROR_OBJECT = merge( - VALID_ERROR_RESPONSE, {"error": METHOD_UNAVAILABLE_MSG} -) + ERROR_RESPONSE_INVALID_CODE = merge(VALID_ERROR_RESPONSE, {"error": {"code": "-32601"}}) -ERROR_RESPONSE_INVALID_MESSAGE = merge( - VALID_ERROR_RESPONSE, {"error": {"code": -32000, "message": {}}} -) ERROR_RESPONSE_INVALID_MISSING_CODE = merge( VALID_ERROR_RESPONSE, {"error": {"message": "msg"}} ) + +ERROR_RESPONSE_INVALID_MESSAGE = merge( + VALID_ERROR_RESPONSE, {"error": {"code": -32000, "message": {}}} +) ERROR_RESPONSE_INVALID_MISSING_MESSAGE = merge( VALID_ERROR_RESPONSE, {"error": {"code": -32000}} ) +ERROR_RESPONSE_INVALID_ERROR_OBJECT = merge( + VALID_ERROR_RESPONSE, {"error": METHOD_UNAVAILABLE_MSG} +) + # result object validation VALID_RESPONSE_NULL_RESULT = merge(VALID_RESULT_OBJ_RESPONSE, {"result": None}) VALID_RESPONSE_0x_RESULT = merge(VALID_RESULT_OBJ_RESPONSE, {"result": "0x"}) @@ -283,13 +286,6 @@ def test_formatted_response_invalid_error_object( Web3RPCError, f'{VALID_ERROR_RESPONSE["error"]}\nUser message: {DEFAULT_USER_MSG}', ), - ( - # Valid error response with no null result formatters raises `Web3RPCError` - identity, - identity, - Web3RPCError, - f'{VALID_ERROR_RESPONSE["error"]}\nUser message: {DEFAULT_USER_MSG}', - ), ), ) def test_formatted_response_error_responses_with_formatters_raise_expected_exceptions( From 02f7dfb58788990cd5b106c2d0a3071a7c1e2e4a Mon Sep 17 00:00:00 2001 From: fselmo Date: Wed, 24 Apr 2024 10:34:14 -0600 Subject: [PATCH 7/8] Opt for logging user_message and response on RPC errors. --- .../core/manager/test_response_formatters.py | 14 ++++-------- web3/exceptions.py | 6 ----- web3/manager.py | 22 ++++++++++++++----- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/core/manager/test_response_formatters.py b/tests/core/manager/test_response_formatters.py index 952499e585..7d9d3123a7 100644 --- a/tests/core/manager/test_response_formatters.py +++ b/tests/core/manager/test_response_formatters.py @@ -35,10 +35,6 @@ METHOD_UNAVAILABLE_MSG = ( "the method eth_getTransactionByHash does not exist/is not available." ) -DEFAULT_USER_MSG = ( - "An RPC error was returned by the node. Check the message provided in the error " - "and any available logs for more information." -) VALID_RESULT_OBJ_RESPONSE = {"jsonrpc": "2.0", "id": 1, "result": {"foo": "bar"}} @@ -179,19 +175,17 @@ def test_formatted_response_invalid_response_object(w3, response, error, error_m ( VALID_ERROR_RESPONSE, Web3RPCError, - f'{VALID_ERROR_RESPONSE["error"]}\nUser message: {DEFAULT_USER_MSG}', + f'{VALID_ERROR_RESPONSE["error"]}', ), ( ERROR_RESPONSE_VALID_ID_STRING, Web3RPCError, - f'{ERROR_RESPONSE_VALID_ID_STRING["error"]}\n' - f"User message: {DEFAULT_USER_MSG}", + f'{ERROR_RESPONSE_VALID_ID_STRING["error"]}', ), ( ERROR_RESPONSE_VALID_ID_NONE, Web3RPCError, - f'{ERROR_RESPONSE_VALID_ID_NONE["error"]}\n' - f"User message: {DEFAULT_USER_MSG}", + f'{ERROR_RESPONSE_VALID_ID_NONE["error"]}', ), ( ERROR_RESPONSE_VALID_METHOD_UNAVAILABLE, @@ -284,7 +278,7 @@ def test_formatted_response_invalid_error_object( identity, raise_block_not_found, Web3RPCError, - f'{VALID_ERROR_RESPONSE["error"]}\nUser message: {DEFAULT_USER_MSG}', + f'{VALID_ERROR_RESPONSE["error"]}', ), ), ) diff --git a/web3/exceptions.py b/web3/exceptions.py index 8b289567ca..a51c324ad1 100644 --- a/web3/exceptions.py +++ b/web3/exceptions.py @@ -43,12 +43,6 @@ def __init__( # Assign properties of Web3Exception self.user_message = user_message - def __str__(self) -> str: - # append a clarifying user message if one is provided - return super().__str__() + ( - f"\nUser message: {self.user_message}" if self.user_message else "" - ) - class Web3AssertionError(Web3Exception, AssertionError): """ diff --git a/web3/manager.py b/web3/manager.py index 75bba44b1d..b1870624cc 100644 --- a/web3/manager.py +++ b/web3/manager.py @@ -131,6 +131,7 @@ def _validate_response( response: RPCResponse, error_formatters: Optional[Callable[..., Any]], is_subscription_response: bool = False, + logger: Optional[logging.Logger] = None, ) -> None: if "jsonrpc" not in response or response["jsonrpc"] != "2.0": _raise_bad_response_format( @@ -191,12 +192,18 @@ def _validate_response( response, 'error["code"] is required and must be an integer value.' ) elif code == METHOD_NOT_FOUND: - raise MethodUnavailable( + exception = MethodUnavailable( repr(error), rpc_response=response, - user_message="Check your node provider or your client's API docs to " - "see what methods are supported and / or currently enabled.", + user_message=( + "This method is not available. Check your node provider or your " + "client's API docs to see what methods are supported and / or " + "currently enabled." + ), ) + logger.error(exception.user_message) + logger.debug(f"RPC error response: {response}") + raise exception # errors must include a message error_message = error.get("message") @@ -206,7 +213,11 @@ def _validate_response( ) apply_error_formatters(error_formatters, response) - raise Web3RPCError(repr(error), rpc_response=response) + + web3_rpc_error = Web3RPCError(repr(error), rpc_response=response) + logger.error(web3_rpc_error.user_message) + logger.debug(f"RPC error response: {response}") + raise web3_rpc_error elif "result" not in response and not is_subscription_response: _raise_bad_response_format(response) @@ -298,8 +309,8 @@ async def _coro_make_request( # # See also: https://www.jsonrpc.org/specification # - @staticmethod def formatted_response( + self, response: RPCResponse, params: Any, error_formatters: Optional[Callable[..., Any]] = None, @@ -316,6 +327,7 @@ def formatted_response( response, error_formatters, is_subscription_response=is_subscription_response, + logger=self.logger, ) # format results From a51eca2248b523c0ebc50b5a3fa9a081755fd03e Mon Sep 17 00:00:00 2001 From: fselmo Date: Wed, 24 Apr 2024 12:24:00 -0600 Subject: [PATCH 8/8] changes from comments on PR #3359 --- tests/core/manager/test_response_formatters.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/core/manager/test_response_formatters.py b/tests/core/manager/test_response_formatters.py index 7d9d3123a7..e906e5170a 100644 --- a/tests/core/manager/test_response_formatters.py +++ b/tests/core/manager/test_response_formatters.py @@ -3,8 +3,6 @@ from eth_utils.toolz import ( identity, -) -from toolz import ( merge, )