Custom solana-py exceptions

[broper2]

Has it been considered to raise exceptions custom to solana-py library instead of generic HTTPError created in:

solana-py/src/solana/rpc/providers/core.py

Line 54 in f38f01a

raw_response.raise_for_status()

This may also be useful in timeout scenario. It would be a simple change to wrap provider code in decorator catching and converting into custom exception. Then clients and classes composed with solana library can more specifically catch exceptions.

Would be happy to create PR, let me know your thoughts.

[michaelhly]

@kevinheavey what do you think? Feel free to push up a PR to have us look :)

[kevinheavey]

@broper2 definitely interested to hear more about what we might catch with this. Most of the time when the RPC returns an error of some kind the status code is still 200 and the error information lives in the JSON itself.

As for the specific timeout question, requests and httpx both appear to raise a ConnectionError if the request times out (though the requests docs suggest otherwise). This happens whether or not we use .raise_for_status()

[broper2]

@kevinheavey @michaelhly Definitely have noticed the 200 status code and error json with invalid input to request. This behavior is clearly by design and I think it makes sense for solana-py client to pass that up to caller. I am more talking about flaky, unexpected issues that can arise with HTTP requests.

For specific example, I have seen timeout behavior raise a ReadTimeout exception (see below logs) in my application. My WIP idea is to put decorator around http provider function with POST call, catching RequestException type and converting to solana-py internal exception which captures same info. Then users of library can have a single exception type and interface to work with. I'll start working on PR unless any objections from the audience can save me some time. Thanks!

7:52:39 AM web.1 | File "/Users/brianroper/.virtualenvs/solana-tracker/lib/python3.8/site-packages/solana/rpc/providers/http.py", line 21, in make_request
7:52:39 AM web.1 | raw_response = requests.post(**request_kwargs, timeout=self.timeout)
7:52:39 AM web.1 | File "/Users/brianroper/.virtualenvs/solana-tracker/lib/python3.8/site-packages/requests/api.py", line 117, in post
7:52:39 AM web.1 | return request('post', url, data=data, json=json, **kwargs)
7:52:39 AM web.1 | File "/Users/brianroper/.virtualenvs/solana-tracker/lib/python3.8/site-packages/requests/api.py", line 61, in request
7:52:39 AM web.1 | return session.request(method=method, url=url, **kwargs)
7:52:39 AM web.1 | File "/Users/brianroper/.virtualenvs/solana-tracker/lib/python3.8/site-packages/requests/sessions.py", line 542, in request
7:52:39 AM web.1 | resp = self.send(prep, **send_kwargs)
7:52:39 AM web.1 | File "/Users/brianroper/.virtualenvs/solana-tracker/lib/python3.8/site-packages/requests/sessions.py", line 655, in send
7:52:39 AM web.1 | r = adapter.send(request, **kwargs)
7:52:39 AM web.1 | File "/Users/brianroper/.virtualenvs/solana-tracker/lib/python3.8/site-packages/requests/adapters.py", line 529, in send
7:52:39 AM web.1 | raise ReadTimeout(e, request=request)
7:52:39 AM web.1 | requests.exceptions.ReadTimeout: HTTPSConnectionPool(host='api.mainnet-beta.solana.com', port=443): Read timed out. (read timeout=10)

[kevinheavey]

@broper2 is the problem that it's hard to know what exception to catch under the current workflow? I don't get what you're looking for but it doesn't sound insane so maybe this will make more sense to me when your PR is done 😅

[broper2]

@kevinheavey @michaelhly np, raised PR #152. To summarize, the idea is to have general, custom exceptions raised from solana-py clients. The client RPC implementation should be black-box to callers, it is better if the callers do not need to dig thru client code to know what errors they should catch if they need to do that.

Consider if solana-py moved away from HTTP provider to another way to implement RPC. The exceptions raised would most likely no longer be of "requests" library. Then all code dependent on solana-py would need to be updated. Raising exceptions native to solana-py eliminates this coupling.

BTW, totally open to changes to namings and error messages :)