pallets / quart

An async Python micro framework for building web applications.
https://quart.palletsprojects.com
MIT License
3.01k stars 164 forks source link

Fix propagating exceptions #385

Open jakubsvehla opened 3 days ago

jakubsvehla commented 3 days ago

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 commented 3 days ago

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

jakubsvehla commented 3 days ago

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