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.

Sign up for GitHub

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

Jump to bottom

Update error handling on APIHandlers #2853

Merged
merged 4 commits into from
Sep 25, 2017
Merged

Update error handling on APIHandlers #2853

merged 4 commits into from
Sep 25, 2017

Conversation

minrk
Copy link
Member

@minrk minrk commented Sep 21, 2017 •
edited
Loading

  • Raise 403 on unauthenticated API requests
    instead of redirecting to login page, which doesn't make sense for API handlers
    and can cause undesirable side effects when using custom login handlers as in JupyterHub
    (Redirect to login url on mismatched oauth state jupyterhub/jupyterhub#1438).
  • deprecate @json_errors decorator with write_error method
    on APIHandler, which is the natural and more robust place to implement custom error handling in Tornado.

@minrk
raise 403 on APIHandler failed login
9220922 
instead of redirecting to human login page, which can have side effects
@minrk
use .write_error on APIHandler instead of @json_errors for JSON err…
ba353e2 
…or messages

this is the standard tornado way to do it, and catches errors at the `prepare` stage, which method decorators do not
@minrk
stop using @json_handlers
962c5cc
@minrk
specify version for deprecation
4467dc9
@minrk minrk mentioned this pull request Sep 21, 2017
Closed
@takluyver takluyver added this to the 5.2 milestone Sep 21, 2017
takluyver
takluyver approved these changes Sep 21, 2017
View reviewed changes
Copy link
Member

@takluyver takluyver left a comment

Choose a reason for hiding this comment

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

This looks good to me. I'll leave it for a day or so in case others want to have a look.

@rgbkrk rgbkrk merged commit b2edf89 into jupyter:master Sep 25, 2017
@minrk minrk deleted the api-403 branch October 10, 2017 14:48
@minrk minrk mentioned this pull request Oct 10, 2017
Merged
@gnestor gnestor modified the milestone: 5.2 Oct 13, 2017
This was referenced Oct 16, 2017
Closed
Closed
Closed
@takluyver takluyver mentioned this pull request Oct 31, 2017
Merged
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@takluyver takluyver takluyver approved these changes

Assignees
No one assigned
Labels
status:resolved-locked
Projects
None yet
Milestone
5.2
Development

Successfully merging this pull request may close these issues.
4 participants
@minrk @takluyver @gnestor @rgbkrk