-
-
Notifications
You must be signed in to change notification settings - Fork 876
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
Call exception handlers even if response started #758
Conversation
merge from head
… it's yet to be rendered
fb267fe
to
fade736
Compare
Added tests, 100% coverage :) |
@@ -58,11 +58,26 @@ def test_websockets_should_raise(): | |||
|
|||
|
|||
def test_handled_exc_after_response(): | |||
# A 406 HttpException is raised *after* the response has already been sent. | |||
# The exception middleware should raise a RuntimeError. | |||
with pytest.raises(RuntimeError): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would change the test much more simply here. A handled exception after the response is rendered isn't a problem now, we should just expect a 200 status code. We don't particularly need to check for side-effects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't we want to make sure that the exception handler runs? In case the response is already rendered, we get the same result even if it's not called
This change allows background tasks exceptions to be caught by exception handlers.
The implication is that handlers are always called, but response is rendered by the handlers only if not rendered yet.
** THIS IS A DRAFT ** I will add tests and document changes after initial approval.