diff --git a/newsfragments/3053.feature.rst b/newsfragments/3053.feature.rst new file mode 100644 index 0000000000..8d23689a22 --- /dev/null +++ b/newsfragments/3053.feature.rst @@ -0,0 +1 @@ +Improved error messaging for exceptions from malformed JSON-RPC responses. \ No newline at end of file diff --git a/tests/core/manager/test_response_formatters.py b/tests/core/manager/test_response_formatters.py index 1daedd8db1..c6afb3954f 100644 --- a/tests/core/manager/test_response_formatters.py +++ b/tests/core/manager/test_response_formatters.py @@ -1,4 +1,5 @@ import pytest +import re from eth_utils.toolz import ( identity, @@ -25,9 +26,36 @@ "because the ancient block sync is still in progress.", }, } +ERROR_RESPONSE_WITH_NONE_ID = { + "id": None, + "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.", @@ -41,6 +69,28 @@ "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": {}, + }, +} ETH_TESTER_METHOD_NOT_FOUND_RESP_FORMAT = { "error": "the method eth_getTransactionByHash does not exist/is not available", } @@ -77,6 +127,22 @@ def raise_contract_logic_error(response): raise_block_not_found, ValueError, ), + ( + # Error response with no result formatters raises a ValueError + ERROR_RESPONSE_WITH_NONE_ID, + (), + identity, + raise_block_not_found, + ValueError, + ), + ( + # Error response with no result formatters raises a ValueError + ERROR_RESPONSE_WITH_VALID_ID, + (), + identity, + raise_block_not_found, + ValueError, + ), ( # None result raises error if there is a null_result_formatter NONE_RESPONSE, @@ -169,13 +235,24 @@ def test_formatted_response_raises_errors( TransactionNotFound, "Transaction with hash: '0x01' not found.", ), + ( + INVALID_JSONRPC_RESP_FORMAT, + (), + identity, + 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}", + ), ( UNEXPECTED_RESPONSE_FORMAT, (), identity, identity, BadResponseFormat, - f"The response was in an unexpected format and unable to be parsed. The raw response is: {UNEXPECTED_RESPONSE_FORMAT}", # noqa: E501 + f"The response was in an unexpected format and unable to be parsed. " + f"The raw response is: {UNEXPECTED_RESPONSE_FORMAT}", ), ( ANOTHER_UNEXPECTED_RESP_FORMAT, @@ -183,7 +260,44 @@ def test_formatted_response_raises_errors( identity, identity, BadResponseFormat, - f"The response was in an unexpected format and unable to be parsed. The raw response is: {ANOTHER_UNEXPECTED_RESP_FORMAT}", # noqa: E501 + f"The response was in an unexpected format and unable to be parsed. " + f"The raw response is: {ANOTHER_UNEXPECTED_RESP_FORMAT}", + ), + ( + INVALID_CODE_RESP_FORMAT, + (), + identity, + 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: {INVALID_CODE_RESP_FORMAT}" + ), + ), + ( + MISSING_CODE_RESP_FORMAT, + (), + identity, + 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}" + ), + ), + ( + INVALID_MESSAGE_RESP_FORMAT, + (), + 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}" + ), ), ], ) diff --git a/web3/_utils/contract_error_handling.py b/web3/_utils/contract_error_handling.py index ff5dbae1a8..de4922419a 100644 --- a/web3/_utils/contract_error_handling.py +++ b/web3/_utils/contract_error_handling.py @@ -49,96 +49,118 @@ "function type.", } +MISSING_DATA = "no data" -def raise_contract_logic_error_on_revert(response: RPCResponse) -> RPCResponse: - """ - Reverts contain a `data` attribute with the following layout: - "Reverted " - Function selector for Error(string): 08c379a (4 bytes) - Data offset: 32 (32 bytes) - String length (32 bytes) - Reason string (padded, use string length from above to get meaningful part) - See also https://solidity.readthedocs.io/en/v0.6.3/control-structures.html#revert +def _parse_error_with_reverted_prefix(data: str) -> str: """ - if not isinstance(response["error"], dict): - raise ValueError("Error expected to be a dict") - - data = response["error"].get("data", "") - message = response["error"].get("message", "") - - message_present = message is not None and message != "" + Parse errors from the data string which begin with the "Reverted" prefix. + "Reverted", function selector and offset are always the same for revert errors + """ + prefix = f"Reverted {SOLIDITY_ERROR_FUNC_SELECTOR}" + data_offset = ("00" * 31) + "20" # 0x0000...0020 (32 bytes) + revert_pattern = prefix + data_offset + error = data + + if data.startswith(revert_pattern): + # if common revert pattern + string_length = int(data[len(revert_pattern) : len(revert_pattern) + 64], 16) + error = data[ + len(revert_pattern) + 64 : len(revert_pattern) + 64 + string_length * 2 + ] + elif data.startswith("Reverted 0x"): + # Special case for this form: 'Reverted 0x...' + error = data.split(" ")[1][2:] + + try: + error = bytes.fromhex(error).decode("utf8") + except UnicodeDecodeError: + warnings.warn("Could not decode revert reason as UTF-8", RuntimeWarning) + raise ContractLogicError("execution reverted", data=data) - if data is None: - if message_present: - raise ContractLogicError(message) - elif not message_present: - raise ContractLogicError("execution reverted") - else: - raise Exception("Unreachable") + return error - # Ganache case: - if isinstance(data, dict) and message_present: - raise ContractLogicError(f"execution reverted: {message}", data=data) - # Parity/OpenEthereum case: - if data.startswith("Reverted "): - # "Reverted", function selector and offset are always the same for revert errors - prefix = f"Reverted {SOLIDITY_ERROR_FUNC_SELECTOR}" - data_offset = ("00" * 31) + "20" # 0x0000...0020 (32 bytes) - revert_pattern = prefix + data_offset - - if data.startswith(revert_pattern): - # if common revert pattern - string_length = int( - data[len(revert_pattern) : len(revert_pattern) + 64], 16 - ) - reason_as_hex = data[ - len(revert_pattern) + 64 : len(revert_pattern) + 64 + string_length * 2 - ] - elif data.startswith("Reverted 0x"): - # Special case for this form: 'Reverted 0x...' - reason_as_hex = data.split(" ")[1][2:] - else: - raise ContractLogicError("execution reverted", data=data) +def _raise_contract_error(response_error_data: str) -> None: + """ + Decode response error from data string and raise appropriate exception. - try: - reason_string = bytes.fromhex(reason_as_hex).decode("utf8") - raise ContractLogicError(f"execution reverted: {reason_string}", data=data) - except UnicodeDecodeError: - warnings.warn("Could not decode revert reason as UTF-8", RuntimeWarning) - raise ContractLogicError("execution reverted", data=data) + "Reverted " (prefix may be present in `data`) + Function selector for Error(string): 08c379a (4 bytes) + Data offset: 32 (32 bytes) + String length (32 bytes) + Reason string (padded, use string length from above to get meaningful part) + """ + if response_error_data.startswith("Reverted "): + reason_string = _parse_error_with_reverted_prefix(response_error_data) + raise ContractLogicError( + f"execution reverted: {reason_string}", data=response_error_data + ) - # --- EIP-3668 | CCIP Read --- # - if data[:10] == OFFCHAIN_LOOKUP_FUNC_SELECTOR: - parsed_data_as_bytes = to_bytes(hexstr=data[10:]) + elif response_error_data[:10] == OFFCHAIN_LOOKUP_FUNC_SELECTOR: + # --- EIP-3668 | CCIP read error --- # + parsed_data_as_bytes = to_bytes(hexstr=response_error_data[10:]) abi_decoded_data = abi.decode( list(OFFCHAIN_LOOKUP_FIELDS.values()), parsed_data_as_bytes ) offchain_lookup_payload = dict( zip(OFFCHAIN_LOOKUP_FIELDS.keys(), abi_decoded_data) ) - raise OffchainLookup(offchain_lookup_payload, data=data) + raise OffchainLookup(offchain_lookup_payload, data=response_error_data) - # --- Solidity Panic Error --- # - if data[:10] == PANIC_ERROR_FUNC_SELECTOR: - panic_error_code = data[-2:] - raise ContractPanicError(PANIC_ERROR_CODES[panic_error_code], data=data) + elif response_error_data[:10] == PANIC_ERROR_FUNC_SELECTOR: + # --- Solidity Panic Error --- # + panic_error_code = response_error_data[-2:] + raise ContractPanicError( + PANIC_ERROR_CODES[panic_error_code], data=response_error_data + ) # Solidity 0.8.4 introduced custom error messages that allow args to # be passed in (or not). See: # https://blog.soliditylang.org/2021/04/21/custom-errors/ - if len(data) >= 10 and not data[:10] == SOLIDITY_ERROR_FUNC_SELECTOR: + elif ( + len(response_error_data) >= 10 + and not response_error_data[:10] == SOLIDITY_ERROR_FUNC_SELECTOR + ): # Raise with data as both the message and the data for backwards # compatibility and so that data can be accessed via 'data' attribute # on the ContractCustomError exception - raise ContractCustomError(data, data=data) + raise ContractCustomError(response_error_data, data=response_error_data) - # Geth case: - if message_present and response["error"].get("code", "") == 3: - raise ContractLogicError(message, data=data) - # Geth Revert without error message case: - if message_present and "execution reverted" in message: - raise ContractLogicError("execution reverted", data=data) + +def raise_contract_logic_error_on_revert(response: RPCResponse) -> RPCResponse: + """ + Revert responses contain an error with the following optional attributes: + `code` - in this context, used for an unknown edge case when code = '3' + `message` - error message is passed to the raised exception + `data` - response error details (str, dict, None) + + See also https://solidity.readthedocs.io/en/v0.6.3/control-structures.html#revert + """ + error = response.get("error") + if error is None or isinstance(error, str): + raise ValueError(error) + + message = error.get("message") + message_present = message is not None and message != "" + data = error.get("data", MISSING_DATA) + + if data is None: + if message_present: + raise ContractLogicError(message, data=data) + elif not message_present: + raise ContractLogicError("execution reverted", data=data) + elif isinstance(data, dict) and message_present: + raise ContractLogicError(f"execution reverted: {message}", data=data) + elif isinstance(data, str): + _raise_contract_error(data) + + if message_present: + # Geth Revert with error message and code 3 case: + if error.get("code") == 3: + raise ContractLogicError(message, data=data) + # Geth Revert without error message case: + elif "execution reverted" in message: + raise ContractLogicError("execution reverted", data=data) return response diff --git a/web3/manager.py b/web3/manager.py index 9014402985..3e94f30d91 100644 --- a/web3/manager.py +++ b/web3/manager.py @@ -75,6 +75,19 @@ NULL_RESPONSES = [None, HexBytes("0x"), "0x"] +METHOD_NOT_FOUND = -32601 + + +def _raise_bad_response_format(response: RPCResponse, error: str = "") -> None: + message = "The response was in an unexpected format and unable to be parsed." + raw_response = f"The raw response is: {response}" + + if error is not None and error != "": + message = f"{message} {error}. {raw_response}" + else: + message = f"{message} {raw_response}" + + raise BadResponseFormat(message) def apply_error_formatters( @@ -198,6 +211,15 @@ async def _coro_make_request( self.logger.debug(f"Making request. Method: {method}") return await request_func(method, params) + # + # formatted_response parses and validates JSON-RPC responses for expected + # properties (result or an error) with the expected types. + # + # Required properties are not strictly enforced to further determine which + # exception to raise for specific cases. + # + # See also: https://www.jsonrpc.org/specification + # @staticmethod def formatted_response( response: RPCResponse, @@ -205,36 +227,73 @@ def formatted_response( error_formatters: Optional[Callable[..., Any]] = None, null_result_formatters: Optional[Callable[..., Any]] = None, ) -> Any: - if "error" in response: + # 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"' + ) + + # 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"' + ) + + # 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 ValueError(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) + + # 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) - # guard against eth-tester case - eth-tester returns a string - # with no code, so can't parse what the error is. - if isinstance(response["error"], dict): - resp_code = response["error"].get("code") - if resp_code == -32601: - raise MethodUnavailable(response["error"]) - raise ValueError(response["error"]) - # NULL_RESPONSES includes None, so return False here as the default - # so we don't apply the null_result_formatters if there is no 'result' key - elif response.get("result", False) in NULL_RESPONSES: - # null_result_formatters raise either a BlockNotFound - # or a TransactionNotFound error, depending on the method called - apply_null_result_formatters(null_result_formatters, response, params) - return response["result"] - elif response.get("result") is not None: - return response["result"] + raise ValueError(error) + + # Format and validate results + elif "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_subscribe includes response["params"]["result"] elif ( - # eth_subscribe case response.get("params") is not None and response["params"].get("result") is not None ): return response["params"]["result"] + + # Any other response type raises BadResponseFormat else: - raise BadResponseFormat( - "The response was in an unexpected format and unable to be parsed. " - f"The raw response is: {response}" - ) + _raise_bad_response_format(response) def request_blocking( self, diff --git a/web3/types.py b/web3/types.py index 67042b09b9..5e234434ff 100644 --- a/web3/types.py +++ b/web3/types.py @@ -283,7 +283,7 @@ class GethSyncingSubscriptionResponse(SubscriptionResponse): class RPCResponse(TypedDict, total=False): error: Union[RPCError, str] - id: int + id: Optional[Union[int, str]] jsonrpc: Literal["2.0"] result: Any