python-tumblpy icon indicating copy to clipboard operation
python-tumblpy copied to clipboard

Tumblr can return error responses that aren't handled correctly

Open Syfaro opened this issue 7 years ago • 2 comments

It appears that when Tumblr is having issues, error responses can be different than expected.

I got the error: AttributeError: 'str' object has no attribute 'get'

Sentry recorded that content was a str instead of a dict. The status code was 504.

Relevant line: https://github.com/michaelhelmick/python-tumblpy/blob/master/tumblpy/api.py#L165

It seems like the easiest fix would be to check if the response was a str and if it was, use that for the error_message. Also, in looking at that code, it appears error_message gets written over for every error instead of being appended to. This seems like it could be fixed by ', '.join(content['errors']) instead of the loop.

If these seems like reasonable fixes, I'm happy to open a PR for them.

Syfaro avatar Jun 10 '18 19:06 Syfaro

That sounds good! Glad to accept a PR

michaelhelmick avatar Jun 11 '18 18:06 michaelhelmick

It would be nice if the structure of throwable exceptions were more like the base Python exceptions, where different types of errors convey a sense of how to respond to what went wrong. (As a class tree, E.G. Warning vs Exception https://docs.python.org/2/library/exceptions.html#exception-hierarchy )

With respect to error types such as, E.G. response.status_code == 429 (rate limit exceeded) it might be better if handling that behavior were generally abstracted within the tumblpy object. Though some applications might also prefer a non-blocking version. I'm unsure if a settable object property, or a per function property, or both is desirable for providing control over a default (which I argue should be to block, given that's what Tumblr's rate limit does).

mjevans avatar Dec 16 '18 07:12 mjevans