Skip to content

Commit

Permalink
Explicitly raise RequestTimedOut on timed out requests:
Browse files Browse the repository at this point in the history
- Introduce ``RequestTimedOut`` and raise this for requests with
  timeout messaging.
- This allows us to be more specific on flaky tests that fail with
  node timeout errors. Specify ``RequestTimedOut`` as the expected
  exception in relevant tests marked with ``@pytest.mark.xfail``.
  • Loading branch information
fselmo committed Jul 25, 2024
1 parent 899a909 commit 8965da1
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 9 deletions.
1 change: 1 addition & 0 deletions newsfragments/3440.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement a ``RequestTimedOut`` exception, extending from ``Web3RPCError``, for when requests to the node time out.
1 change: 1 addition & 0 deletions newsfragments/3440.internal.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Mitigate inconsistently failing tests for CI runs with appropriate ``flaky`` or ``pytest.mark.xfail()`` decorators.
10 changes: 10 additions & 0 deletions tests/core/manager/test_response_formatters.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
BlockNotFound,
ContractLogicError,
MethodUnavailable,
RequestTimedOut,
TransactionNotFound,
Web3RPCError,
)
Expand Down Expand Up @@ -77,6 +78,10 @@
VALID_ERROR_RESPONSE,
{"error": {"code": -32601, "message": (METHOD_UNAVAILABLE_MSG)}},
)
ERROR_RESPONSE_REQUEST_TIMED_OUT = merge(
VALID_ERROR_RESPONSE,
{"error": {"code": -32002, "message": "Request timed out."}},
)
ERROR_RESPONSE_INVALID_ID = merge(VALID_ERROR_RESPONSE, {"id": b"invalid"})

ERROR_RESPONSE_INVALID_CODE = merge(VALID_ERROR_RESPONSE, {"error": {"code": "-32601"}})
Expand Down Expand Up @@ -190,6 +195,11 @@ def test_formatted_response_invalid_response_object(w3, response, error, error_m
MethodUnavailable,
METHOD_UNAVAILABLE_MSG,
),
(
ERROR_RESPONSE_REQUEST_TIMED_OUT,
RequestTimedOut,
f'{ERROR_RESPONSE_REQUEST_TIMED_OUT["error"]}',
),
),
)
def test_formatted_response_valid_error_object(response, w3, error, error_message):
Expand Down
13 changes: 10 additions & 3 deletions web3/_utils/module_testing/eth_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
MultipleFailedRequests,
NameNotFound,
OffchainLookup,
RequestTimedOut,
TimeExhausted,
TooManyRequests,
TransactionNotFound,
Expand Down Expand Up @@ -2359,7 +2360,9 @@ async def test_async_eth_replace_transaction_gas_price_too_low(
await async_w3.eth.replace_transaction(txn_hash, txn_params)

@pytest.mark.xfail(
reason="Very flaky on CI runs, hard to reproduce locally", strict=False
reason="Very flaky on CI runs, hard to reproduce locally",
strict=False,
raises=(RequestTimedOut, asyncio.TimeoutError, Web3ValueError),
)
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_gas_price_defaulting_minimum(
Expand All @@ -2385,7 +2388,9 @@ async def test_async_eth_replace_transaction_gas_price_defaulting_minimum(
) # minimum gas price

@pytest.mark.xfail(
reason="Very flaky on CI runs, hard to reproduce locally", strict=False
reason="Very flaky on CI runs, hard to reproduce locally",
strict=False,
raises=(RequestTimedOut, asyncio.TimeoutError, Web3ValueError),
)
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_gas_price_defaulting_strategy_higher(
Expand Down Expand Up @@ -2416,7 +2421,9 @@ def higher_gas_price_strategy(async_w3: "AsyncWeb3", txn: TxParams) -> Wei:
async_w3.eth.set_gas_price_strategy(None) # reset strategy

@pytest.mark.xfail(
reason="Very flaky on CI runs, hard to reproduce locally", strict=False
reason="Very flaky on CI runs, hard to reproduce locally",
strict=False,
raises=(RequestTimedOut, asyncio.TimeoutError, Web3ValueError),
)
@pytest.mark.asyncio
async def test_async_eth_replace_transaction_gas_price_defaulting_strategy_lower(
Expand Down
6 changes: 6 additions & 0 deletions web3/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,12 @@ class MethodUnavailable(Web3RPCError):
"""


class RequestTimedOut(Web3RPCError):
"""
Raised when a request to the node times out.
"""


class TransactionNotFound(Web3RPCError):
"""
Raised when a tx hash used to look up a tx in a jsonrpc call cannot be found.
Expand Down
35 changes: 29 additions & 6 deletions web3/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
BadResponseFormat,
MethodUnavailable,
ProviderConnectionError,
RequestTimedOut,
TaskNotRunning,
Web3RPCError,
Web3TypeError,
Expand Down Expand Up @@ -89,6 +90,13 @@


NULL_RESPONSES = [None, HexBytes("0x"), "0x"]
KNOWN_REQUEST_TIMEOUT_MESSAGING = {
# Note: It's important to be very explicit here and not too broad. We don't want
# to accidentally catch a message that is not for a request timeout. In the worst
# case, we raise something more generic like `Web3RPCError`. JSON-RPC unfortunately
# has not standardized error codes for request timeouts.
"request timed out", # go-ethereum
}
METHOD_NOT_FOUND = -32601


Expand Down Expand Up @@ -185,6 +193,7 @@ def _validate_response(
response, 'Response must include either "error" or "result".'
)
elif "error" in response:
web3_rpc_error: Optional[Web3RPCError] = None
error = response["error"]

# raise the error when the value is a string
Expand All @@ -202,7 +211,7 @@ def _validate_response(
response, 'error["code"] is required and must be an integer value.'
)
elif code == METHOD_NOT_FOUND:
exception = MethodUnavailable(
web3_rpc_error = MethodUnavailable(
repr(error),
rpc_response=response,
user_message=(
Expand All @@ -211,9 +220,6 @@ def _validate_response(
"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")
Expand All @@ -222,9 +228,26 @@ def _validate_response(
response, 'error["message"] is required and must be a string value.'
)

apply_error_formatters(error_formatters, response)
if any(
# parse specific timeout messages
timeout_str in error_message.lower()
for timeout_str in KNOWN_REQUEST_TIMEOUT_MESSAGING
):
web3_rpc_error = RequestTimedOut(
repr(error),
rpc_response=response,
user_message=(
"The request timed out. Check the connection to your node and "
"try again."
),
)

if web3_rpc_error is None:
# if no condition was met above, raise a more generic `Web3RPCError`
web3_rpc_error = Web3RPCError(repr(error), rpc_response=response)

response = apply_error_formatters(error_formatters, 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
Expand Down

0 comments on commit 8965da1

Please sign in to comment.