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

Add 'reason' field to JSON error responses #2958

Merged
merged 2 commits into from Nov 1, 2017

Conversation

Projects
None yet
3 participants
@kevin-bates
Copy link
Member

kevin-bates commented Oct 19, 2017

During the deprecation/removal of the @json_errors decorator, the
reason field was not carried forward into the compatible replacement
method APIHandler.write_error. This broke some client (tests) that
relied on that field's presence.

Fixes #2957.

Add 'reason' field to JSON error responses
During the deprecation/removal of the `@json_errors` decorator, the
`reason` field was not carried forward into the compatible replacement
method `APIHandler.write_error`.  This broke some client (tests) that
relied on that field's presence.

Fixes #2957.

@takluyver takluyver requested a review from minrk Oct 20, 2017

@takluyver takluyver added this to the 5.3 milestone Oct 31, 2017

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Oct 31, 2017

@kevin-bates looking back at the old decorator code before #2853, it set reason to e.reason when e is an HTTPError, and None otherwise. I'd suggest that we replicate that.

@kevin-bates

This comment has been minimized.

Copy link
Member Author

kevin-bates commented Oct 31, 2017

@takluyver - yeah, that's probably the best approach. Change committed.

@takluyver

This comment has been minimized.

Copy link
Member

takluyver commented Oct 31, 2017

Thanks. I'll merge tomorrow if no-one says otherwise.

@takluyver takluyver merged commit 9a5c2c0 into jupyter:master Nov 1, 2017

4 checks passed

codecov/patch 100% of diff hit (target 0%)
Details
codecov/project 79.31% (+0.2%) compared to f763c03
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kevin-bates kevin-bates deleted the kevin-bates:fix-2957-add-reason-to-json-errors branch Nov 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.