search

westonplatter / fast_arrow

(no longer maintained) A simple yet robust (stock+options) API client for Robinhood
MIT License
127 stars 37 forks source link

Fix Requests Error Handling #93

Closed k3an3 closed 4 years ago

k3an3 commented 5 years ago

Version of "fast_arrow:" master What you did: Made a request with expired/invalid auth tokens What you expected to happen: A 4xx error from requests or a nice InvalidCredentials error of some sort What actually happened: The Client.get and Client.post methods absorb the exceptions and return None, causing an error in other parts of the library that process response data. Additional relevant information:

The Client.get and Client.post methods specifically catch 403 and try to reauthenticate using refresh tokens, but I think I'm seeing 401s instead these days (unrelated: I'm not sure if the refresh tokens are working at all). Either way, for any error status code in 4xx-5xx besides 400 and 429 (for post), or even the second time in a call where the result is 403, the method will silently fail and return None to whatever called it. It is not immediately apparent that a login failure occurred in this case, since the actual error will usually be a NoneType related issue in completely different code.

TL;DR: When there is a requests.exceptions.RequestException that the library cannot recover from, the exception should probably be raised so that calling code can deal with it. That, or create generic exceptions to distinguish between server errors, client errors, and authentication issues.

westonplatter commented 4 years ago

@k3an3 thanks for the comment. Can you give another try with the new 1.0.0.rc2 release? I moved authentication into another library, fast_arrow_auth. The examples have been updated to reflect the new authentication setup.

westonplatter commented 4 years ago

@k3an3 took another look at your comments, I think I didn't understand your suggestion the first time. Sorry about that.

I was able to reproduce the behavior you mention ... the Client didn't refresh the bearer token. Therefore, in #102, I removed the automatic retry and just raise an error for get and post via res.raise_for_status().

I think this addresses your original comments. I'm still not getting it, please reopen the issue! Thanks a ton for the help pointing out this bug!!