Suggestion: raise exception on error from server in request #893
Description
Summary
Change JsonRpcClient.request(...) to throw when the response from the server is not successful, for ease of error handling and consistency with xrpl.js.
Version
The behavior described below applies to xrpl-py version 4.3.1.
Background
Consider the following code:
from xrpl.clients import JsonRpcClient
from xrpl.models import AccountInfo
from xrpl.utils import drops_to_xrp
client = JsonRpcClient("https://s.altnet.rippletest.net:51234")
response = client.request(AccountInfo(
account="rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
ledger_index="validated",
))
xrp = drops_to_xrp(response.result['account_data']['Balance'])
print(f"Account has {xrp} XRP")Should the client.request(...) be in a try/except block? Well, yes, because the request can time out, or encounter other network errors that will throw. Just as a couple quick test cases that throw:
- Specify the wrong port and it'll throw
httpx.ConnectTimeout - Specify the wrong domain name and it'll throw
httpx.ConnectError - I think these or similar errors can occur when your internet connection legitimately cuts out.
- Specify a completely wrong data type, like
account=Noneand it'll throwxrpl.models.exceptions.XRPLModelException. - I'm not sure, but it might also throw if the server cuts you off for rate limiting.
But there's a second class of errors that don't throw because the server responds, but with an error. A few test cases that don't throw:
- The address (or whatever data) you specified/requested doesn't exist in the ledger. It'll return a
Responsewith an error such asnotFound. - The server doesn't have a validated ledger, or doesn't have the ledger you requested. It'll return a
Responsewith an error such asnoNetwork, orlgrNotFound. - You specified a parameter in an invalid format. It'll return a
Responsewith an error such asactMalformedorinvalidParams.
But actually, sometimes a failed response from the server does throw anyway. At least one example:
- Specify an invalid
api_version, and it'll throwxrpl.asyncio.clients.exceptions.XRPLRequestFailureException.
Proper Handling
As a result, robust handling of a simple API request means two-tiered handling of responses. First, you try/except to catch network errors, then you check response.is_successful() to see if the request did what you need, and look at error if it didn't. This seems more complicated than necessary, as it's easy to forget one tier or the other. But if you remember both, the code looks like this:
from xrpl.clients import JsonRpcClient
from xrpl.models import AccountInfo
from xrpl.utils import drops_to_xrp
client = JsonRpcClient("https://s.altnet.rippletest.net:51234")
try:
response = client.request(AccountInfo(
account="rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn",
ledger_index=None,
))
except Exception as e:
print(f"Request error: {e}")
exit(1)
if not response.is_successful():
print(f"Response error: {response.result['error']}")
exit(1)
xrp = drops_to_xrp(response.result['account_data']['Balance'])
print(f"Account has {xrp} XRP")If you're comparing with similar code from xrpl.js, you can easily forget to check is_successful() because xrpl.js handles this differently and throws if the response isn't successful. In xrpl-py, it'll silently slip through until you get an error much later when you try to do something with the unsuccessful response object, like access the data in it that should be there on a successful response.
Solution: Always throw if not successful
The whole is_successful() thing is, in my opinion, unnecessary. If the server doesn't respond successfully, you're going to have to handle the response differently regardless, which is something try/except is good for. You need a try/except anyway, since the request can throw for unpredictable (network) reasons or predictable (bad request) ones. You can put that to use and catch different types of errors by importing the appropriate exception type and using except XRPLRequestFailureException as e or whatever appropriately.
I'd much rather be able to work under the assumption that if request(...) returned without throwing an exception, then request.result contains the fields of a successful response.