Invalid response when closing connection using Sanic

[ENT8R]

I'm currently experiencing an issue similar but distinct to #120 when using Sanic as a framework together with Socket.IO
In case the request has been upgraded to a websocket connection, and this connection is closed by the client, the asynchronous socket _websocket_handler() method returns no value.

Sanic now checks whether the response returned by the handler is an instance of Sanic's BaseHTTPResponse, which is not the case, so no response is send to the client. Instead Sanic applies an additional check by checking whether handler.is_websocket is set, but this also fails as this property seems to be set only when using the internal websocket implementation of Sanic and handler is the handle_request method of Engine.IO
The affected code part can be found here: https://github.com/sanic-org/sanic/blob/main/sanic/app.py#L724-L740

A possible solution I could think of is replacing

python-engineio/engineio/async_drivers/sanic.py

Lines 19 to 20 in 845fc62

	app.add_route(engineio_server.handle_request, engineio_endpoint,
	methods=['GET', 'POST', 'OPTIONS'])

with the following snippet according to PEP-232 even though I don't know if this is the most elegant way to solve this...

engineio_server.handle_request.__func__.is_websocket = True
app.add_route(engineio_server.handle_request, engineio_endpoint,
              methods=['GET', 'POST', 'OPTIONS'])

[miguelgrinberg]

Isn't this a Sanic problem? Why is the is_websocket attribute not set correctly?

[ENT8R]

Mmhh... possibly yes. Sanic only sets this attribute if the corresponding route is initialized with websocket=True, but doing this would make it impossible to use your library's request handler, as Sanic then automatically overwrites the handler with their own implementation. I opened a new issue at sanic-org/sanic#2105

[ENT8R]

So, as I understand, Sanic wants to keep its focus on the internal websocket implementation and keep away additional complexity which may be required when handling external websocket library implementations. Then there is probably few left what your library could to in order to prevent the error message when closing a websocket connection, so I'm closing this issue for now...

I don't know whether it might be worth considering #120 (comment) (i.e. raising an asyncio.CancelledError) as a potential fix for this issue, as Sanic is not the only framework which can be used together with your library and I don't know how other frameworks/apps would handle this, so I leave this decision up to you.

[miguelgrinberg]

@ENT8R I'm not sure I follow this. This package does not have its own WebSocket implementation. Back in the day, Sanic did not have WebSocket support. I wrote the first implementation of it and contributed it to the project, just so that I can use it on this package. I'm not sure how things changed since then, but this package still relies on the WebSocket support provided by Sanic. This is why I think it is a Sanic bug that the is_websocket flag isn't being set.

I'll take a quick look to see if I can figure out what the problem is, or if things changed considerably in the Sanic side since I've done the initial work on WebSocket support.

[seanmkauffman]

Just to chime in here, I have also experienced this error while trying to use python-socketio with Sanic. In my case the error seems to occur when the client tries to upgrade the connection to ws, though, rather than when it tries to close the connection. I'm happy to provide more details if that would be helpful. I can also post in that Sanic bug, but they seemed a lot less interested in looking at it, as mentioned.

[miguelgrinberg]

@seanmkauffman I looked at the current state of their WebSocket support. They've changed an important design aspect and that makes it impossible (or at least hard) to address this error. Basically a route in Sanic has to be either HTTP or WebSocket, it can't be both. On a WebSocket route, they set is_websocket = True and that is what avoids issuing a response.

The Socket.IO protocol uses the same endpoint for HTTP and WebSocket connections, so it cannot be adapted to work in the way Sanic currently wants. Since they have no interest in addressing this use case, this will have to stay like this.

[seanmkauffman]

@miguelgrinberg Thanks for the update! I will try aiohttp then.