Handle empty/ 404 responses

[mkai]

The Tumblr API returns and empty list (and not a dict) in some cases, e. g. if a blog was not found (404). In these case, calling content.get() raises an AttributeError. This commit fixes it.

Example for a 404 response:
http://api.tumblr.com/v2/blog/cocofuckedchanel.tumblr.com/posts/

[michaelhelmick]

Hi @mkai

Thank you for your pull request, one question though; if an attribute error is thrown, shouldn't we do a try/except around the block of code?

try:
    content = content.get('response', {})
except AttributeError:
    raise TumblpyError('Unable to parse content.')

[mkai]

Hi @michaelhelmick,

the AttributeError is thrown because content is an empty list after the statement in line 236 and .get() is called on it.

So if you prefer using a try/ except clause, we would need to do:

content = content.get('response', {})
try:
    content.get()
except AttributeError:
   raise TumblpyError('Unable to parse content.')

But I would prefer the other solution because the content could in fact be parsed, only that contains no useful response; the check for a success status code afterwards (<200 or >301) takes care of the rest nicely.

Up to you!

[michaelhelmick]

@mkai Right, but it would error out on

content = content.get('response', {})

so it would never get to your line of code that you changed

[michaelhelmick]

And we always want to check of 'response' so if it doesn't have it, we should throw the try/except around that.

[mkai]

I don't see where that line of code would ever error out/ raise an exception. If response is not a key in the dict, .get() returns None or the alternate value you gave (an empty dict).

But the problem is that the 404ing result does indeed have a response key set (the value is an empty Python list), and calling .get() on it raises the exception.

So after calling content = content.get('response', {}), content is an empty list; i. e. all we need to do is avoid calling .get() on that list.

[michaelhelmick]

I'm saying if the content is okay and doesn't return just a list, It'll look like this
{'response':...}
So, regardless, we want to do:

content = content.get('response', {})

We should do a try/except around that because if it IS a list, then an AttributeError is thrown (which we would then throw a TumblpyError but if it isn't a list then it will either grab the response key or it will set content to an empty dict and continue with the script.

If we just go with the code you changed, right now, it still has a chance of throwing an AttributeError

[mkai]

Hi Mike,

I'm leaving that up to you! Just wanted to report the bug. Thanks for your effort with the library.

[michaelhelmick]

Alright. I'll prob push a 0.6.2 release today. Thanks for your support.

Sent from my iPhone

On Oct 18, 2012, at 12:12 PM, Markus Kaiserswerth notifications@github.com wrote:

Hi Mike,

I'm leaving that up to you! Just wanted to report the bug. Thanks for your effort with the library.

—
Reply to this email directly or view it on GitHub.

[michaelhelmick]

Tumblpy 0.6.2 is available via pip

pip install -I python-tumblpy

[mkai]

I'm sorry, but the release doesn't fix the problem.

  File "virtualenv/lib/python2.7/site-packages/tumblpy.py", line 245, in request
    if content.get('errors') or content.get('error'):
AttributeError: 'list' object has no attribute 'get'

For a test case, please make an API request to http://api.tumblr.com/v2/blog/cocofuckedchanel.tumblr.com/posts/

[michaelhelmick]

@mkai I know when and how this error is thrown, but you are running an old version of Tumblpy.

Line 245 of 0.6.2 release is:

for error in content['errors']:

[michaelhelmick]

https://github.com/michaelhelmick/python-tumblpy/blob/master/tumblpy.py#L245

[mkai]

@michaelhelmick Yeah, the line number differed because I inserted two print statements for debugging.

Here's the output without modifications of version 0.6.2:

  File "virtualenv/lib/python2.7/site-packages/tumblpy.py", line 243, in request
    if content.get('errors') or content.get('error'):
AttributeError: 'list' object has no attribute 'get'

I can only repeat that your try/except block will never raise an AttributeError, but either return the value of the 'response' key or the alternate value, an empty dict. In the 404 case, the value of the value of the 'response' key is an empty list, on which you try to call .get(), which results in an AttributeError in line 243.

[michaelhelmick]

Alright, I'll try and argue my case (line 232):

content = json.loads(response.content)

That's fine, whether response.content is a list or dict or string or whatever.

If it does return a 404, then (in your case) content is a list so (for the all intents and purposes)

content = []

So if I run:

content = content.get('response', {})

If content is a dict, then it's going to look for the key (response), if it can't find response then it sets content = {}
But since (in your case) content is set to a list (content = []) and I try to do a .get() on it, it's going to throw an AttributeError at that point on line 238 and it should never make it past that.

[michaelhelmick]

Wow, my apologies. I see what you are saying. I didn't get a chance to look at the link you posted. I will post a fix for this.

[michaelhelmick]

@mkai Can you actually re-open your pull request?

[mkai]

:)
You re-opened the issue which also re-opended the pull request.

[michaelhelmick]

Tumblpy 0.6.3 is available via pip Thanks!