Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tumblr can return error responses that aren't handled correctly #51

Open
Syfaro opened this issue Jun 10, 2018 · 2 comments
Open

Tumblr can return error responses that aren't handled correctly #51

Syfaro opened this issue Jun 10, 2018 · 2 comments

Comments

@Syfaro
Copy link

Syfaro commented Jun 10, 2018

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.

@michaelhelmick
Copy link
Owner

That sounds good! Glad to accept a PR

@mjevans
Copy link

mjevans commented Dec 16, 2018

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants