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

More Descriptive Error Message On Http 200 But No JSON #944

Merged
merged 3 commits into from Mar 17, 2016

Conversation

@Calvinp
Copy link
Contributor

Calvinp commented Mar 11, 2016

When a user's session expires, we send an http 302 redirect to the login page. However, the browser catches this and follows it without passing it off to the application. Then the application receives html when it was expecting JSON. This handles that problem by advising the user of the situation and that one possible cause is an expired session.

Calvinp added 2 commits Mar 11, 2016
… Instructs the user that a possible cause is an expired session, and to please log in again
Messenger().error
message: '''
<p>Expected JSON but received something else (possibly html). The response has been saved to your js console.</p>
<p>One possible cause is an http redirect to an html web page.</p>

This comment has been minimized.

Copy link
@ssalinas

ssalinas Mar 14, 2016

Member

I still think we can be a bit smarter about the messaging here. We can know if it is html by checking if the string starts with <!DOCTYPE html>. Then we can either say '...but received html' or '...but received something else' if we don't know.

Also, The one possible cause is very specific to our own use case. If there is no way for us to know that that might be the error, we shouldn't be mentioning possible causes.

…her than dumping the html on the screen as the error message
@ssalinas
Copy link
Member

ssalinas commented Mar 17, 2016

LGTM

ssalinas added a commit that referenced this pull request Mar 17, 2016
…00-no-json

More Descriptive Error Message On Http 200 But No JSON
@ssalinas ssalinas merged commit d7cc7f4 into master Mar 17, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ssalinas ssalinas deleted the more-descriptive-error-on-http-200-no-json branch Mar 17, 2016
@ssalinas ssalinas modified the milestone: 0.4.12 Apr 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.