sanic
sanic copied to clipboard
Implement catchable WebsocketClosed
See #2220
In v21.9, a new WebsocketClosed
exception was added.
While you can to do the following:
@app.websocker("/")
async def handler(request, ws):
try:
...
except asyncio.CancelledError:
print("connection closed")
We would like to add support for this pattern:
@app.websocker("/")
async def handler(request, ws):
try:
...
except sanic.exceptions.WebsocketClosed:
print("connection closed")
@ahopkins I think there might've been some miscommunication around what the WebsocketClosed exception is for.
In #2248, WebsocketsClosed exception was added as a replacement for throwing sanic.ServerError
exceptions if a user tries to read or write to a websocket connection that is closed or is closing. This also prevented the KeepAlive-ping background task from emitting a ServerError needlessly when the websocket connection was closed.
In theory (if the timing it right) it is possible to catch it like this:
@app.websocker("/")
async def handler(request, ws):
try:
while(True):
ws.write("hello") # <-- this could throw WebsocketClosed when the socket closes
except sanic.exceptions.WebsocketClosed:
print("connection closed")
However also in #2248, I introduced more capability for the websockets connection to raise asyncio.CancelledError
into the handler, to give the handler early warning that the connection is closing, and prevent the handler from continuing to make send()
and recv()
calls. So this change significantly decreases the possibility of the handler seeing a WebsocketClosed
exception, because it would see asyncio.CancelledError
first, and break from its handling.
If we really want the ability to catch a websocket-specific exception, rather than asyncio.CancelledError
, then I suggest we create a new exception like WebsocketCancelled
that subclasses asyncio.CancelledError, and use that instead.
Then the handler could use code like this to cover both:
@app.websocker("/")
async def handler(request, ws):
try:
...
except (WebsocketClosed, WebsocketCancelled)
print("connection closed")
@ashleysommer What is the distinction between WebsocketClosed and WebsocketCancelled from an app developer standpoint? Looks like it is all down to random chance of socket closing timing. Using any sort of CancelledError for a loss of connection seems overkill, and I would prefer reserving those to timeouts and non-graceful exit.
The other thing about CancelledError
that would be nice to avoid (if possible) is that it trips a lot of people up when they think they can catch it with Exception
.
The difference as I see it: WebsocketClosed
is an error. It is triggered by trying to do something you cannot do. Writing to a closed socket is not possible, so your action raises that error. Like dividing by zero.
On the other hand, CancelledError
is an asyncio exception that can act like it signal, it gets thrown into the handler when we want to notify the handler the connection is going down. It is not triggered by something the user is trying to do, but by the connection itself. Eg, this would also work with CancelledError:
@app.websocker("/")
async def handler(request, ws):
try:
await asyncio.sleep(9999)
except asyncio.CancelledError:
print("connection closing")
There is no way to do that with a regular exception like WebsocketClosed
.
I'm also expecting new exception like WebsocketClosed, otherwise it is impossible determine reason of closed connection.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.
...
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.
May be impacted by: https://github.com/sanic-org/sanic/pull/2513