No error handling for flickr endpoints return 5XX http errors

Scraft commented 5 years ago

I have a script which sorts users albums, it does this by getting a list of their albums, and then for each album gets a list of the photos, and then for each photo gets the 'taken' info key and uses that to construct the right order and finally set this as the albums sort order.

I am running this on an account with ~150 albums and ~16,000 photos. So we are talking about a lot of calls to REST endpoints - the nice thing about this is it highlights any areas which can potentially go wrong. At the moment, I have never had a run which doesn't go wrong - I have put a workaround in, but it would be nice if this could be done on your side.

Essentially what happens is method_call.py, in call_api, around lines 140-150:

try:
    resp = json.loads(resp)
except ValueError as e:
    logger.error("Could not parse response: %s", str(resp))

if resp["stat"] != "ok":
    raise FlickrAPIError(resp["code"], resp["message"])

resp = clean_content(resp)

return resp

Ultimately throws a type error from if resp["stat"] != "ok": as the response isn't some JSON that the flickr endpoint has generated, instead it is a 5XX error (typically 502). In my client code, all calls to your API, I have put inside a function which will attempt to call your function up to 5 times, stopping when successful and continuing when a TypeError is raised. After each failure, I wait (sleep) for an increased time (1 second, then 2 seconds, then 3 seconds, then 4 seconds).

The issue with the flickr REST api returning 5XX errors has been talked about for a long time, I don't know if it'll ever be fixed, but either way, it would be amazing if python-flickr-api could handle both this, and just in general handle the REST api not returning 200 and your expected response. I feel the wait and retry approach is the best option, you could expose the parameters for number of retries and how the delay should work, but it would be nice if you just defaulted it in such a way that people like me writing an application like this would find it just works without any extra steps.

mtrovo commented 5 years ago

Hi, @Scraft thanks for your bug report. Could you provide more info about the 5XX body content? If possible even just paste it here on the comments.

I'm willing to make a change to fix this but I just want to make sure if it's just a matter of reusing the same FlickrAPIError or if I need to create some new for it.

Scraft commented 5 years ago

Sure, I made the following addition to method_call.py around line 131:

if CACHE is None:
    resp = requests.post(request_url, args)
else:
    resp = CACHE.get(data) or requests(request_url, args)
    if data not in CACHE:
        CACHE.set(data, resp)
    #!!!
    if resp.status_code != 200:
        print "status_code: '" + str(resp.status_code) + "'"
        print "contents: '" + str(resp.content) + "'"
    #!!!
    resp = resp.content
    if raw:
        return resp

Which resulted in the following:

status_code: '502'
contents: '<html>
<head><title>502 Bad Gateway</title></head>
<body bgcolor="white">
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx/1.7.6</center>
</body>
</html>
'

Hopefully, that shows the issue clearly for you.

mtrovo commented 5 years ago

I released a new version with a fix for it, let me know if it doesn't solve your issue.

alexis-mignon / python-flickr-api

No error handling for flickr endpoints return 5XX http errors #107