sanic
sanic copied to clipboard
Better request cancel handling
Currently, there is no way to distinguish between the following:
SCENARIO A: Something in the handler raises a CancelledError
@app.get("/")
async def handler(request: Request):
raise CancelledError
SCENARIO B: A client cancels a request in the middle of handling
@app.get("/")
async def handler(request: Request):
for _ in range(10):
print(".")
await sleep(1)
return json(None)
This is particularly noticeable and troublesome if there is an underlying library (like a DB driver) that decides to call CancelledError
. It causes a 503 and there really is no way to catch this. The following will NOT work, which is somewhat counterintuitive.
@app.exception(CancelledError)
async def exc_handler(request: Request, exc):
...
The idea here is to introduce a new exception: RequestCancelled
that will bubble up through the system. It should be catchable on its own, and also CancelledError
should also be catchable. Furthermore, it should not create a response object and attempt to push the bytes if the client connection has been lost.
Codecov Report
Base: 87.834% // Head: 87.844% // Increases project coverage by +0.009%
:tada:
Coverage data is based on head (
398e53e
) compared to base (7f894c4
). Patch coverage: 90.000% of modified lines in pull request are covered.
Additional details and impacted files
@@ Coverage Diff @@
## main #2513 +/- ##
=============================================
+ Coverage 87.834% 87.844% +0.009%
=============================================
Files 78 78
Lines 6461 6466 +5
Branches 1246 1247 +1
=============================================
+ Hits 5675 5680 +5
Misses 560 560
Partials 226 226
Impacted Files | Coverage Δ | |
---|---|---|
sanic/server/protocols/base_protocol.py | 65.384% <66.666%> (+0.449%) |
:arrow_up: |
sanic/exceptions.py | 98.924% <100.000%> (+0.035%) |
:arrow_up: |
sanic/http/http1.py | 85.211% <100.000%> (ø) |
|
sanic/models/server_types.py | 93.023% <100.000%> (+0.166%) |
:arrow_up: |
sanic/server/protocols/http_protocol.py | 81.617% <100.000%> (ø) |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.
It looks good to me. Let's make it ready and run the CI?
I need to add a few more unit tests.
Did you check how this affects ASGI, or how are connections lost with that anyway? The catching/handling logic in Sanic is extremely complicated but I see that you did a change to http1 but not to asgi, while both often need identical changes.