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

Fix propagating exceptions #385

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jakubsvehla
Copy link

Fixes propagating exceptions when the PROPAGATE_EXCEPTIONS config setting is set to True by fixing exception handling in ASGIHTTPConnection. Currently, when PROPAGATE_EXCEPTIONS is set to True it returns the traceback as HTML instead of propagating the exception.

@davidism
Copy link
Member

Please verify that this modified truth table matches the behavior in Flask.

@jakubsvehla
Copy link
Author

The PROPAGATE_EXCEPTIONS flag in Flask behaves differently in two ways:

  1. None is not treated as False (as currently in Quart). When it’s None, it takes the value of the testing flag.
  2. The PROPAGATE_EXCEPTIONS flag has precedence over the testing flag. So if PROPAGATE_EXCEPTIONS is False and testing True, then it won’t propagate exceptions (in Quart it would right now).

So Flask’s truth table (when ignoring the debug flag) is as follows:

PROPAGATE_EXCEPTIONS testing Raise (Flask) Raise (Quart after the current fix)
None True True True
None False False False
True True True True
True False True True
False True False True <- This line differs
False False False False

The difference is that in Flask, setting PROPAGATE_EXCEPTIONS to False overrides the testing flag.

So the question is whether we want to replicate Flask’s behavior or simply fix the bug. I think both behaviors make sense but the previous Quart’s behavior (before this fix) is in my opinion clearly wrong since it does not propagate exceptions when the PROPAGATE_EXCEPTIONS is True which I think should be the expected behavior.

You can find the relevant code in Flask here: https://github.com/pallets/flask/blob/bc098406af9537aacc436cb2ea777fbc9ff4c5aa/src/flask/app.py#L841

As far as I know, there are no tests for the interaction of the two flags. They are tested independently here: https://github.com/pallets/flask/blob/bc098406af9537aacc436cb2ea777fbc9ff4c5aa/tests/test_basic.py#L1574

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.

Exception propagation via PROPAGATE_EXCEPTIONS config setting does not work
2 participants