Skip to content

better handling of API errors#110

Merged
ryanmcgrath merged 2 commits intoryanmcgrath:masterfrom
jvanasco:master
Aug 25, 2012
Merged

better handling of API errors#110
ryanmcgrath merged 2 commits intoryanmcgrath:masterfrom
jvanasco:master

Conversation

@jvanasco
Copy link
Copy Markdown
Contributor

@jvanasco jvanasco commented Aug 1, 2012

I kept getting Twitter errors that said 'Response was not valid JSON, unable to decode.'

After a bit of debugging, I found out why -- my app uses an environment.ini file to change between dev and production modes -- with corresponding API keys. I was getting an Unauthorized error when i used the wrong ini , but the current codebase doesn't display that.

The current code will immediately raise an error on a failed decode -- however Twitter provides the 'unauthorized' info in the response header ( a bunch of other codes would fall in there too ).

This patch is pretty simple:

1_ it adds a 'json_error' variable, which is set to True on a failed load -- and defers raising an error for a bit...
2_ the current response.status_code check is left in tact
3_ it adds a block 'if json_error' to raise an error after the other error processing occurs

This could probably be cleaner, but then I'd have to rewrite a lot of stuff. this just works, right, and was fast.

…I errors.

this could likely be done a bit cleaner -- but this works.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some prints in here :P

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha ha. thanks. fixing.

ryanmcgrath pushed a commit that referenced this pull request Aug 25, 2012
better handling of API errors
@ryanmcgrath ryanmcgrath merged commit a489a47 into ryanmcgrath:master Aug 25, 2012
@ryanmcgrath
Copy link
Copy Markdown
Owner

Mmm, this has been sitting for a bit now. Merged, will be pushed out in next release. Thanks!

@jvanasco
Copy link
Copy Markdown
Contributor Author

wonderful! thanks!

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

Successfully merging this pull request may close these issues.

3 participants