Closed mfield closed 5 years ago
For parameter validation, we should switch to ValueError
.
I agree that we need to do a better job signaling if the request should be retried. It might be better to have a field on ApiException
like retriable
or even just http_status_code
and sift_status_code
so that we're not as coupled to requests
for http, but I'm not opposed to exposing the response in the exception.
Looks like API3 errors may not be parsed correctly. I see this in the sift console
"{\"error\":\"too_many_requests\",\"description\":\"You've sent too many requests to this API recently. Try again later.\"}"
on my end it looks like the description
field is not understood as the error message, all I see is
ApiException: https://api3.siftscience.com/v3/accounts/5b2c18de4f0c58535cc39b7c/users/498742366996267018/decisions returned non-2XX http status code 400
I think that's just the exception message from requests
; we don't parse the response.
I do get more informative error messages from the events api. I'm not sure if it's api/api3 or labels/decisions that explains why.
Oh, there's missing handling in this block: https://github.com/SiftScience/sift-python/blob/e55afff4fd787d7d56effef013e59e61f79f8444/sift/client.py#L797-L802
There should be if 'description' in self.body:
(or something similar)
We changed the ApiException
class to include a lot more metadata.
When a request to Sift fails, I want to know if I should retry it. The only way for me to do that right now is to parse the string representation of a
sift.client.ApiException
based on error messages we've already seen.For example, this error is retryable:
But this one is not:
Lower level errors that may get raised from the Requests library are also converted to the same
sift.client.ApiException
, so I lose out on the whole taxonomy of errors in http://docs.python-requests.org/en/master/_modules/requests/exceptions/. I think in our case we'd lean toward an at least once approach rather than an at most once approach so maybe the difference between a ConnectTimeout and a ReadTimeout isn't that huge? It would be nice to have the information available to make a more deliberate decision though.The
sift-ruby
approach of putting the response as an attribute of the error seems pretty good. It would give me access to these error codes that are pretty well documented https://siftscience.com/developers/docs/python/apis-overview/core-topics/error-reference.