Improve websockets integration?

[aaugustin]

Hello,

I'm the primary author of websockets (the Python library) and I was happy to discover that sanic uses it. Making it possible for third-parties to reuse the protocol was part of my original design. It's the first time I see a project actually taking advantage of this :-)

I see that you're using WebSocketCommonProtocol and re-implementing the opening handshake here: https://github.com/channelcat/sanic/blob/e1c90202682d76361c77adb7bb2176730038466a/sanic/websocket.py#L58

Unfortunately, you're missing some features I added recently, mainly support for the permessage-deflate extension which compresses messages. It can provide nice performance improvements e.g. when exchanging JSON messages.

Quite obviously you can't take advantage of WebSocketServerProtocol.handshake (which handles subprotocol and extensions negotiation) as it currently stands. It starts with
path, request_headers = yield from self.read_http_request() — which you need to replace with sanic's own HTTP code.

Less important potential problems:

• I'm likely to break your integration when I make changes to websockets — I already did in previous versions.
• websockets is very careful about closing TCP connections correctly. I'm not sure if you need this and take advantage of this

I'd like to support your use case better, by providing a public API that you can hook into.

Are you interested in discussing a better design and coordinating its implementation?

Related issues in websockets: python-websockets/websockets#210, python-websockets/websockets#256, python-websockets/websockets#386.

[aaugustin]

Possible options:

• overriding WebSocketServerProtocol.read/write_http_request in sanic
• making a no-IO version of the core of WebSocketServerProtocol.handshake so sanic can reuse it

[aaugustin]

FWIW I've also talked with Tom Christie about websockets integration in uvicorn and I'm thinking about writing a sans-I/O layer to make such integrations easier, see python-websockets/websockets#466

[sjsadowski]

Ping @ahopkins - This may help with the other websockets issue.

[ahopkins]

@aaugustin I think this is an interesting idea. The handling of websockets in Sanic needs better integration. To the extent that we can work with you and @tomchristie, I'm on board to start those discussions.

One thing that I had brought up in the past that probably bears a discussion in this issue is how much websocket support should be bound inside of the Sanic core framework. On the one hand, there is tight bundling as it currently is (you get websockets as a dependency whether you intend to use them or not. On the other, it could be moved to a separate repository (pip install sanic-websockets).

I think the ideal may be in the middle where websockets receive first class status and are well supported in the API. But, that the dependency is not a requirement (ie pip install sanic[websockets]).

[stale]

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.

[aaugustin]

Well, I filed it for you rather than for me. It seems valuable to keep track of this even if it takes some years to get done.

[ahopkins]

Keeping open since websockets integration is still on the table.

[aaugustin]

This issue is now fully superseded by #2000.