-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle proxy error #413
Handle proxy error #413
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,14 @@ | ||
from pathlib import Path | ||
from typing import Any, List | ||
|
||
from multiversx_sdk_network_providers import GenericError | ||
from multiversx_sdk_network_providers.proxy_network_provider import \ | ||
ProxyNetworkProvider | ||
|
||
from multiversx_sdk_cli import cli_shared, utils | ||
from multiversx_sdk_cli.cli_output import CLIOutputBuilder | ||
from multiversx_sdk_cli.cosign_transaction import cosign_transaction | ||
from multiversx_sdk_cli.errors import NoWalletProvided | ||
from multiversx_sdk_cli.errors import NoWalletProvided, ProxyError | ||
from multiversx_sdk_cli.transactions import (compute_relayed_v1_data, | ||
do_prepare_transaction, | ||
load_transaction_from_file) | ||
|
@@ -88,11 +89,17 @@ def send_transaction(args: Any): | |
|
||
tx = load_transaction_from_file(args.infile) | ||
output = CLIOutputBuilder() | ||
proxy = ProxyNetworkProvider(args.proxy) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Try / finally was good here - so that the user sees the emitted transaction even if the send fails with exception. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed. Added. |
||
try: | ||
proxy = ProxyNetworkProvider(args.proxy) | ||
tx_hash = proxy.send_transaction(tx) | ||
output.set_emitted_transaction_hash(tx_hash) | ||
except GenericError as ge: | ||
url = ge.url | ||
message = ge.data["error"] | ||
data = ge.data["data"] | ||
code = ge.data["code"] | ||
raise ProxyError(message, url, data, code) | ||
finally: | ||
output = output.set_emitted_transaction(tx).build() | ||
utils.dump_out_json(output, outfile=args.outfile) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -112,6 +112,7 @@ def wallet_new(args: Any): | |
|
||
mnemonic = Mnemonic.generate() | ||
print(f"Mnemonic: {mnemonic.get_text()}") | ||
print(f"Wallet address: {mnemonic.derive_key().generate_public_key().to_address(address_hrp).to_bech32()}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hopefully, not a breaking change for some users 🤞 People who rely on a more structured output should use the |
||
|
||
if format is None: | ||
return | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,3 +198,13 @@ def __init__(self, message: str): | |
class ArgumentsNotProvidedError(KnownError): | ||
def __init__(self, message: str): | ||
super().__init__(message) | ||
|
||
|
||
class ProxyError(KnownError): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
def __init__(self, message: str, url: str, data: str, code: str): | ||
inner = { | ||
"url": url, | ||
"data": data, | ||
"code": code | ||
} | ||
super().__init__(message, inner) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,14 +4,15 @@ | |
import time | ||
from typing import Any, Dict, Optional, Protocol, Sequence, TextIO, Tuple | ||
|
||
from multiversx_sdk_core import Address, Transaction, TransactionPayload | ||
Check failure on line 7 in multiversx_sdk_cli/transactions.py GitHub Actions / runner / mypy
|
||
from multiversx_sdk_network_providers import GenericError | ||
Check failure on line 8 in multiversx_sdk_cli/transactions.py GitHub Actions / runner / mypy
|
||
|
||
from multiversx_sdk_cli import errors | ||
from multiversx_sdk_cli.accounts import Account, LedgerAccount | ||
from multiversx_sdk_cli.cli_password import (load_guardian_password, | ||
load_password) | ||
from multiversx_sdk_cli.cosign_transaction import cosign_transaction | ||
from multiversx_sdk_cli.errors import NoWalletProvided | ||
from multiversx_sdk_cli.errors import NoWalletProvided, ProxyError | ||
from multiversx_sdk_cli.interfaces import ITransaction | ||
from multiversx_sdk_cli.ledger.ledger_functions import do_get_ledger_address | ||
|
||
|
@@ -30,7 +31,7 @@ | |
def send_transaction(self, transaction: ITransaction) -> str: | ||
... | ||
|
||
def send_transactions(self, transactions: Sequence[ITransaction]) -> Tuple[int, str]: | ||
def send_transactions(self, transactions: Sequence[ITransaction]) -> Tuple[int, Dict[str, str]]: | ||
... | ||
|
||
def get_transaction(self, tx_hash: str, with_process_status: Optional[bool] = False) -> ITransactionOnNetwork: | ||
|
@@ -132,7 +133,15 @@ | |
def _send_transaction_and_wait_for_result(proxy: INetworkProvider, payload: ITransaction, num_seconds_timeout: int = 100) -> ITransactionOnNetwork: | ||
AWAIT_TRANSACTION_PERIOD = 5 | ||
|
||
tx_hash = proxy.send_transaction(payload) | ||
try: | ||
tx_hash = proxy.send_transaction(payload) | ||
except GenericError as ge: | ||
url = ge.url | ||
message = ge.data["error"] | ||
data = ge.data["data"] | ||
code = ge.data["code"] | ||
raise ProxyError(message, url, data, code) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we have this in several places now, perhaps use the decorator pattern, and do a custom proxy provider e.g. Alternatively, catch There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created a |
||
|
||
num_periods_to_wait = int(num_seconds_timeout / AWAIT_TRANSACTION_PERIOD) | ||
|
||
for _ in range(0, num_periods_to_wait): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just in case, perhaps use
ge.get("...", "")
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done