Skip to content

Commit

Permalink
fix: improve the error message for exceptions during fetching contrac…
Browse files Browse the repository at this point in the history
…t types from explorer (ApeWorX#2169)
  • Loading branch information
antazoey authored Jul 3, 2024
1 parent bd53c7a commit 9d79ecc
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 16 deletions.
23 changes: 18 additions & 5 deletions src/ape/managers/chain.py
Original file line number Diff line number Diff line change
Expand Up @@ -1044,8 +1044,7 @@ def get(
# In this case, it at least _looked_ like an address.
return None

contract_type = self._local_contract_types.get(address_key)
if contract_type:
if contract_type := self._local_contract_types.get(address_key):
if default and default != contract_type:
# Replacing contract type
self._local_contract_types[address_key] = default
Expand All @@ -1060,8 +1059,7 @@ def get(

return default

contract_type = self._get_contract_type_from_disk(address_key)
if not contract_type:
if not (contract_type := self._get_contract_type_from_disk(address_key)):
# Contract could be a minimal proxy
proxy_info = self._local_proxies.get(address_key) or self._get_proxy_info_from_disk(
address_key
Expand Down Expand Up @@ -1330,7 +1328,22 @@ def _get_contract_type_from_explorer(self, address: AddressType) -> Optional[Con
try:
contract_type = self._network.explorer.get_contract_type(address)
except Exception as err:
logger.error(f"Unable to fetch contract type at '{address}' from explorer.\n{err}")
explorer_name = self._network.explorer.name
if "rate limit" in str(err).lower():
# Don't show any additional error message during rate limit errors,
# if it can be helped, as it may scare users into thinking their
# contracts are not verified.
message = str(err)
else:
# Carefully word this message in a way that doesn't hint at
# any one specific reason, such as un-verified source code,
# which is potentially a scare for users.
message = (
f"Attempted to retrieve contract type from explorer '{explorer_name}' "
f"from address '{address}' but encountered an exception: {err}\n"
)

logger.error(message)
return None

if contract_type:
Expand Down
24 changes: 17 additions & 7 deletions tests/functional/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -717,18 +717,28 @@ def mock_compile(paths, project=None, settings=None):


@pytest.fixture
def mock_sepolia(ethereum, eth_tester_provider, vyper_contract_instance):
def mock_sepolia(create_mock_sepolia):
"""
Temporarily tricks Ape into thinking the local network
is Sepolia so we can test features that require a live
network.
"""
# Ensuring contract exists before hack.
# This allow the network to be past genesis which is more realistic.
_ = vyper_contract_instance
eth_tester_provider.network.name = "sepolia"
yield eth_tester_provider.network
eth_tester_provider.network.name = LOCAL_NETWORK_NAME
with create_mock_sepolia() as network:
yield network


@pytest.fixture
def create_mock_sepolia(ethereum, eth_tester_provider, vyper_contract_instance):
@contextmanager
def fn():
# Ensuring contract exists before hack.
# This allow the network to be past genesis which is more realistic.
_ = vyper_contract_instance
eth_tester_provider.network.name = "sepolia"
yield eth_tester_provider.network
eth_tester_provider.network.name = LOCAL_NETWORK_NAME

return fn


@pytest.fixture
Expand Down
106 changes: 102 additions & 4 deletions tests/functional/test_contracts_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,15 @@ def fn(addr, default=None):

return existing_fn(addr, default=default)

chain.contracts.get = mocker.MagicMock()
chain.contracts.get.side_effect = fn
original_get = chain.contracts.get
mock_get = mocker.MagicMock()
mock_get.side_effect = fn
chain.contracts.get = mock_get
try:
actual = chain.contracts.instance_at(new_address, contract_type=expected_contract_type)
finally:
chain.contracts.get = original_get

actual = chain.contracts.instance_at(new_address, contract_type=expected_contract_type)
ape_caplog.assert_last_log(expected_fail_message)
assert actual.contract_type == expected_contract_type

Expand Down Expand Up @@ -275,7 +280,7 @@ def test_get_multiple_no_addresses(chain, caplog):
assert "No addresses provided." in caplog.messages[-1]


def test_get_all_include_non_contract_address(vyper_contract_instance, chain, owner):
def test_get_multiple_include_non_contract_address(vyper_contract_instance, chain, owner):
actual = chain.contracts.get_multiple((vyper_contract_instance.address, owner.address))
assert len(actual) == 1
assert actual[vyper_contract_instance.address] == vyper_contract_instance.contract_type
Expand All @@ -298,6 +303,99 @@ def test_get_attempts_to_convert(chain):
chain.contracts.get("test.eth2")


@explorer_test
def test_get_attempts_explorer(
mock_explorer, create_mock_sepolia, chain, owner, vyper_fallback_container
):
contract = owner.deploy(vyper_fallback_container)

def get_contract_type(addr):
if addr == contract.address:
return contract.contract_type

raise ValueError("nope")

# Hack in a way to publish on this local network.
with create_mock_sepolia() as network:
mock_explorer.get_contract_type.side_effect = get_contract_type
network.__dict__["explorer"] = mock_explorer
del chain.contracts[contract.address]
try:
actual = chain.contracts.get(contract.address)
finally:
network.__dict__["explorer"] = None

assert actual == contract.contract_type
assert mock_explorer.get_contract_type.call_count > 0
mock_explorer.get_contract_type.reset_mock()


@explorer_test
def test_get_attempts_explorer_logs_errors_from_explorer(
mock_explorer, create_mock_sepolia, chain, owner, vyper_fallback_container, ape_caplog
):
contract = owner.deploy(vyper_fallback_container)
check_error_str = "__CHECK_FOR_THIS_ERROR__"

def get_contract_type(addr):
if addr == contract.address:
raise ValueError(check_error_str)

raise ValueError("nope")

with create_mock_sepolia() as network:
mock_explorer.get_contract_type.side_effect = get_contract_type
network.__dict__["explorer"] = mock_explorer
expected_log = (
f"Attempted to retrieve contract type from explorer 'mock' "
f"from address '{contract.address}' but encountered an "
f"exception: {check_error_str}"
)
del chain.contracts[contract.address]
try:
actual = chain.contracts.get(contract.address)
finally:
network.__dict__["explorer"] = None

assert expected_log in ape_caplog.head
assert actual is None
mock_explorer.get_contract_type.reset_mock()


@explorer_test
def test_get_attempts_explorer_logs_rate_limit_error_from_explorer(
mock_explorer, create_mock_sepolia, chain, owner, vyper_fallback_container, ape_caplog
):
contract = owner.deploy(vyper_fallback_container)

# Ensure is not cached locally.
del chain.contracts[contract.address]

check_error_str = "you have been rate limited"

def get_contract_type(addr):
if addr == contract.address:
raise ValueError(check_error_str)

raise ValueError("nope")

with create_mock_sepolia() as network:
mock_explorer.get_contract_type.side_effect = get_contract_type
network.__dict__["explorer"] = mock_explorer

# For rate limit errors, we don't show anything else,
# as it may be confusing.
expected_log = "you have been rate limited"
try:
actual = chain.contracts.get(contract.address)
finally:
network.__dict__["explorer"] = None

assert expected_log in ape_caplog.head
assert actual is None
mock_explorer.get_contract_type.reset_mock()


def test_cache_non_checksum_address(chain, vyper_contract_instance):
"""
When caching a non-checksum address, it should use its checksum
Expand Down

0 comments on commit 9d79ecc

Please sign in to comment.