Skip to content

Commit

Permalink
Merge pull request #1268 from njgheorghita/raise-custom-exception
Browse files Browse the repository at this point in the history
Add custom exceptions for jsonrpc lookups that return none
  • Loading branch information
njgheorghita authored Mar 8, 2019
2 parents eef7c40 + bd899fc commit 652650d
Show file tree
Hide file tree
Showing 12 changed files with 74 additions and 48 deletions.
3 changes: 2 additions & 1 deletion tests/core/eth-module/test_transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from web3.exceptions import (
TimeExhausted,
TransactionNotFound,
ValidationError,
)
from web3.middleware.simulate_unmined_transaction import (
Expand Down Expand Up @@ -53,7 +54,7 @@ def test_unmined_transaction_wait_for_receipt(web3):
'to': '0xd3CdA913deB6f67967B99D67aCDFa1712C293601',
'value': 123457
})
with pytest.raises(ValueError):
with pytest.raises(TransactionNotFound):
web3.eth.getTransactionReceipt(txn_hash)

txn_receipt = web3.eth.waitForTransactionReceipt(txn_hash)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,8 +182,7 @@ def test_latest_block_based_cache_middleware_busts_cache(w3, mocker):
result = w3.manager.request_blocking('fake_endpoint', [])

assert w3.manager.request_blocking('fake_endpoint', []) == result
with pytest.raises(ValueError):
w3.testing.mine()
w3.testing.mine()

# should still be cached for at least 1 second. This also verifies that
# the middleware caches the latest block based on the block time.
Expand Down Expand Up @@ -212,10 +211,8 @@ def result_cb(method, params):
}))
w3.middleware_onion.add(latest_block_based_cache_middleware)

with pytest.raises(ValueError):
w3.manager.request_blocking('fake_endpoint', [])
with pytest.raises(ValueError):
w3.manager.request_blocking('fake_endpoint', [])
w3.manager.request_blocking('fake_endpoint', [])
w3.manager.request_blocking('fake_endpoint', [])

assert next(counter) == 2

Expand Down
6 changes: 2 additions & 4 deletions tests/core/middleware/test_simple_cache_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,8 @@ def result_cb(method, params):
rpc_whitelist={'fake_endpoint'},
))

with pytest.raises(ValueError):
w3.manager.request_blocking('fake_endpoint', [])
with pytest.raises(ValueError):
w3.manager.request_blocking('fake_endpoint', [])
w3.manager.request_blocking('fake_endpoint', [])
w3.manager.request_blocking('fake_endpoint', [])

assert next(counter) == 2

Expand Down
6 changes: 2 additions & 4 deletions tests/core/middleware/test_time_based_cache_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,8 @@ def mk_result(method, params):
w3.middleware_onion.add(construct_result_generator_middleware({'fake_endpoint': mk_result}))
w3.middleware_onion.add(time_cache_middleware)

with pytest.raises(ValueError):
w3.manager.request_blocking('fake_endpoint', [])
with pytest.raises(ValueError):
w3.manager.request_blocking('fake_endpoint', [])
w3.manager.request_blocking('fake_endpoint', [])
w3.manager.request_blocking('fake_endpoint', [])

assert next(counter) == 2

Expand Down
9 changes: 2 additions & 7 deletions tests/core/testing-module/test_testing_snapshot_and_revert.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
import pytest


def test_snapshot_revert_to_latest_snapshot(web3):
web3.testing.mine(5)

Expand All @@ -14,8 +11,7 @@ def test_snapshot_revert_to_latest_snapshot(web3):

block_after_mining = web3.eth.getBlock("latest")

with pytest.raises(ValueError):
web3.testing.revert()
web3.testing.revert()

block_after_revert = web3.eth.getBlock("latest")

Expand All @@ -42,8 +38,7 @@ def test_snapshot_revert_to_specific(web3):

block_after_mining = web3.eth.getBlock("latest")

with pytest.raises(ValueError):
web3.testing.revert(snapshot_idx)
web3.testing.revert(snapshot_idx)

block_after_revert = web3.eth.getBlock("latest")

Expand Down
6 changes: 1 addition & 5 deletions tests/core/testing-module/test_testing_timeTravel.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
import pytest


def test_time_traveling(web3):
current_block_time = web3.eth.getBlock("pending")['timestamp']

time_travel_to = current_block_time + 12345

with pytest.raises(ValueError):
web3.testing.timeTravel(time_travel_to)
web3.testing.timeTravel(time_travel_to)

latest_block_time = web3.eth.getBlock("pending")['timestamp']
assert latest_block_time >= time_travel_to
10 changes: 6 additions & 4 deletions web3/_utils/module_testing/eth_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
)

from web3.exceptions import (
BlockNotFound,
InvalidAddress,
TransactionNotFound,
)

UNKNOWN_ADDRESS = '0xdEADBEeF00000000000000000000000000000000'
Expand Down Expand Up @@ -278,7 +280,7 @@ def test_eth_replaceTransaction_non_existing_transaction(
'gas': 21000,
'gasPrice': web3.eth.gasPrice,
}
with pytest.raises(ValueError):
with pytest.raises(TransactionNotFound):
web3.eth.replaceTransaction(
'0x98e8cc09b311583c5079fa600f6c2a3bea8611af168c52e4b60b5b243a441997',
txn_params
Expand Down Expand Up @@ -485,7 +487,7 @@ def test_eth_getBlockByHash(self, web3, empty_block):
assert block['hash'] == empty_block['hash']

def test_eth_getBlockByHash_not_found(self, web3, empty_block):
with pytest.raises(ValueError):
with pytest.raises(BlockNotFound):
web3.eth.getBlock(UNKNOWN_HASH)

def test_eth_getBlockByNumber_with_integer(self, web3, empty_block):
Expand All @@ -498,7 +500,7 @@ def test_eth_getBlockByNumber_latest(self, web3, empty_block):
assert block['number'] == current_block_number

def test_eth_getBlockByNumber_not_found(self, web3, empty_block):
with pytest.raises(ValueError):
with pytest.raises(BlockNotFound):
web3.eth.getBlock(12345)

def test_eth_getBlockByNumber_pending(self, web3, empty_block):
Expand Down Expand Up @@ -565,7 +567,7 @@ def test_eth_getTransactionReceipt_unmined(self, web3, unlocked_account_dual_typ
'gas': 21000,
'gasPrice': web3.eth.gasPrice,
})
with pytest.raises(ValueError):
with pytest.raises(TransactionNotFound):
web3.eth.getTransactionReceipt(txn_hash)

def test_eth_getTransactionReceipt_with_log_entry(self,
Expand Down
4 changes: 2 additions & 2 deletions web3/_utils/module_testing/parity_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
class ParityModuleTest:

def test_list_storage_keys_no_support(self, web3, emitter_contract_address):
with pytest.raises(ValueError):
web3.parity.listStorageKeys(emitter_contract_address, 10, None)
keys = web3.parity.listStorageKeys(emitter_contract_address, 10, None)
assert keys is None

def test_trace_replay_transaction(self, web3, parity_fixture_data):
trace = web3.parity.traceReplayTransaction(parity_fixture_data['mined_txn_hash'])
Expand Down
5 changes: 4 additions & 1 deletion web3/_utils/transactions.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
curry,
merge,
)
from web3.exceptions import (
TransactionNotFound,
)

VALID_TRANSACTION_PARAMS = [
'from',
Expand Down Expand Up @@ -66,7 +69,7 @@ def wait_for_transaction_receipt(web3, txn_hash, timeout=120, poll_latency=0.1):
while True:
try:
txn_receipt = web3.eth.getTransactionReceipt(txn_hash)
except ValueError:
except TransactionNotFound:
txn_receipt = None
# FIXME: The check for a null `blockHash` is due to parity's
# non-standard implementation of the JSON-RPC API and should
Expand Down
47 changes: 36 additions & 11 deletions web3/eth.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@
Contract,
)
from web3.exceptions import (
BlockNotFound,
TimeExhausted,
TransactionNotFound,
)
from web3.iban import (
Iban,
Expand Down Expand Up @@ -142,10 +144,13 @@ def getBlock(self, block_identifier, full_transactions=False):
if_number='eth_getBlockByNumber',
)

return self.web3.manager.request_blocking(
result = self.web3.manager.request_blocking(
method,
[block_identifier, full_transactions],
)
if result is None:
raise BlockNotFound(f"Block with id: {block_identifier} not found.")
return result

def getBlockTransactionCount(self, block_identifier):
"""
Expand All @@ -158,10 +163,13 @@ def getBlockTransactionCount(self, block_identifier):
if_hash='eth_getBlockTransactionCountByHash',
if_number='eth_getBlockTransactionCountByNumber',
)
return self.web3.manager.request_blocking(
result = self.web3.manager.request_blocking(
method,
[block_identifier],
)
if result is None:
raise BlockNotFound(f"Block with id: {block_identifier} not found.")
return result

def getUncleCount(self, block_identifier):
"""
Expand All @@ -174,10 +182,13 @@ def getUncleCount(self, block_identifier):
if_hash='eth_getUncleCountByBlockHash',
if_number='eth_getUncleCountByBlockNumber',
)
return self.web3.manager.request_blocking(
result = self.web3.manager.request_blocking(
method,
[block_identifier],
)
if result is None:
raise BlockNotFound(f"Block with id: {block_identifier} not found.")
return result

def getUncleByBlock(self, block_identifier, uncle_index):
"""
Expand All @@ -190,16 +201,24 @@ def getUncleByBlock(self, block_identifier, uncle_index):
if_hash='eth_getUncleByBlockHashAndIndex',
if_number='eth_getUncleByBlockNumberAndIndex',
)
return self.web3.manager.request_blocking(
result = self.web3.manager.request_blocking(
method,
[block_identifier, uncle_index],
)
if result is None:
raise BlockNotFound(
f"Uncle at index: {uncle_index} of block with id: {block_identifier} not found."
)
return result

def getTransaction(self, transaction_hash):
return self.web3.manager.request_blocking(
result = self.web3.manager.request_blocking(
"eth_getTransactionByHash",
[transaction_hash],
)
if result is None:
raise TransactionNotFound(f"Transaction with hash: {transaction_hash} not found.")
return result

@deprecated_for("w3.eth.getTransactionByBlock")
def getTransactionFromBlock(self, block_identifier, transaction_index):
Expand All @@ -220,10 +239,16 @@ def getTransactionByBlock(self, block_identifier, transaction_index):
if_hash='eth_getTransactionByBlockHashAndIndex',
if_number='eth_getTransactionByBlockNumberAndIndex',
)
return self.web3.manager.request_blocking(
result = self.web3.manager.request_blocking(
method,
[block_identifier, transaction_index],
)
if result is None:
raise TransactionNotFound(
f"Transaction index: {transaction_index} "
f"on block id: {block_identifier} not found."
)
return result

def waitForTransactionReceipt(self, transaction_hash, timeout=120):
try:
Expand All @@ -237,20 +262,20 @@ def waitForTransactionReceipt(self, transaction_hash, timeout=120):
)

def getTransactionReceipt(self, transaction_hash):
return self.web3.manager.request_blocking(
result = self.web3.manager.request_blocking(
"eth_getTransactionReceipt",
[transaction_hash],
)
if result is None:
raise TransactionNotFound(f"Transaction with hash: {transaction_hash} not found.")
return result

def getTransactionCount(self, account, block_identifier=None):
if block_identifier is None:
block_identifier = self.defaultBlock
return self.web3.manager.request_blocking(
"eth_getTransactionCount",
[
account,
block_identifier,
],
[account, block_identifier],
)

def replaceTransaction(self, transaction_hash, new_transaction):
Expand Down
14 changes: 14 additions & 0 deletions web3/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,3 +128,17 @@ class ManifestValidationError(PMError):
Raised when a provided manifest cannot be published, since it's invalid.
"""
pass


class TransactionNotFound(Exception):
"""
Raised when a tx hash used to lookup a tx in a jsonrpc call cannot be found.
"""
pass


class BlockNotFound(Exception):
"""
Raised when the block id used to lookup a block in a jsonrpc call cannot be found.
"""
pass
3 changes: 0 additions & 3 deletions web3/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ def request_blocking(self, method, params):
if "error" in response:
raise ValueError(response["error"])

if response['result'] is None:
raise ValueError(f"The call to {method} did not return a value.")

return response['result']

async def coro_request(self, method, params):
Expand Down

0 comments on commit 652650d

Please sign in to comment.