Updates per feedback, naming improvements

web3/_utils/contract_error_handling.py

@@ -52,9 +52,9 @@
 MISSING_DATA = "no data"
 
 
-def _parse_openethereum_hex_error(data: str) -> str:
+def _parse_error_with_reverted_prefix(data: str) -> str:
     """
-    Parse Parity/OpenEthereum error from the data.
+    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}"
@@ -72,64 +72,75 @@ def _parse_openethereum_hex_error(data: str) -> str:
         # 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)
+
     return error
 
 
-def _raise_error_from_decoded_revert_data(data: str) -> None:
-    """
-    Decode response error data and raise appropriate exception.
+def _raise_contract_error(response_error_data: str) -> None:
     """
-    if data.startswith("Reverted "):
-        try:
-            reason_string = bytes.fromhex(_parse_openethereum_hex_error(data)).decode(
-                "utf8"
-            )
-        except UnicodeDecodeError:
-            warnings.warn("Could not decode revert reason as UTF-8", RuntimeWarning)
-            raise ContractLogicError("execution reverted", data=data)
+    Decode response error from data string and raise appropriate exception.
 
-        raise ContractLogicError(f"execution reverted: {reason_string}", 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
+        )
 
-    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)
 
-    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)
 
 
 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)
+    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)
 
-    code = error.get("code")
     message = error.get("message")
     message_present = message is not None and message != ""
     data = error.get("data", MISSING_DATA)
@@ -141,12 +152,12 @@ def raise_contract_logic_error_on_revert(response: RPCResponse) -> RPCResponse:
             raise ContractLogicError("execution reverted", data=data)
     elif isinstance(data, dict) and message_present:
         raise ContractLogicError(f"execution reverted: {message}", data=data)
-
-    _raise_error_from_decoded_revert_data(data)
+    elif isinstance(data, str):
+        _raise_contract_error(data)
 
     if message_present:
         # Geth Revert with error message and code 3 case:
-        if code == 3:
+        if error.get("code") == 3:
             raise ContractLogicError(message, data=data)
         # Geth Revert without error message case:
         elif "execution reverted" in message:

web3/manager.py

@@ -79,12 +79,15 @@
 
 
 def _raise_bad_response_format(response: RPCResponse, error: str = "") -> None:
-    error_message = f"The error is: {error}. " if error else ""
-    raise BadResponseFormat(
-        "The response was in an unexpected format and unable to be parsed. "
-        f"{error_message}"
-        f"The raw response is: {response}"
-    )
+    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(
@@ -209,9 +212,13 @@ async def _coro_make_request(
         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.
-    # Responses are inconsistent so allow for missing required JSON-RPC properties
+    # 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(
@@ -220,31 +227,42 @@ 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, '"jsonrpc" must equal "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 and not isinstance(response["id"], (str, int, 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, '"id" must be a string, integer or null'
+                response, 'Response cannot include both "error" and "result"'
             )
 
         # Format and validate errors
         if "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 may include a code
-            # https://docs.alchemy.com/reference/error-reference#json-rpc-error-codes
+            # 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)
 
-            if not type(error.get("message")) in (str, int, None):
+            # Errors must include a message
+            if not isinstance(error.get("message"), str):
                 _raise_bad_response_format(
-                    response, "error['message'] must be a string, integer or null"
+                    response, "error['message'] must be a string"
                 )
 
             apply_error_formatters(error_formatters, response)
@@ -266,6 +284,7 @@ def formatted_response(
         ):
             return response["params"]["result"]
 
+        # Any other response type raises BadResponseFormat
         else:
             _raise_bad_response_format(response)
 

web3/types.py

@@ -283,7 +283,7 @@ class GethSyncingSubscriptionResponse(SubscriptionResponse):
 
 class RPCResponse(TypedDict, total=False):
     error: Union[RPCError, str]
-    id: int
+    id: Union[int, str]
     jsonrpc: Literal["2.0"]
     result: Any