Switch WebSockets legacy implementation to Sans-I/O

[Kludex]

Given python-websockets/websockets#975 and python-websockets/websockets#1310 (comment) we should try to follow @aaugustin's recommendation.

We can discuss if we should deprecate the current WebSocketProtocol, or if we should replace it, and that's it.

Important

• We're using Polar.sh so you can upvote and help fund this issue.
• We receive the funding once the issue is completed & confirmed by you.
• Thank you in advance for helping prioritize & fund our backlog.

[aaugustin]

The documentation for this is here: https://websockets.readthedocs.io/en/stable/howto/sansio.html

I don't remember how uvicorn integrates websockets so I'm not sure how much work it would take to upgrade to the Sans-I/O implementation. It would probably result in a more straightforward and testable design, though.

As a reference, here's how websockets would use its own Sans-I/O layer: python-websockets/websockets#1245

You might find it helpful to look at Sanic's integration too: sanic-org/sanic#2158

I'm happy to answer questions and assist with reviews.

[Kludex]

Is it possible to send data as soon as I have available on the server with send_response?

I want to send the headers, and then the body, and then more body, and maybe more body after...

[aaugustin]

That can't be a WebSocket handshake response; it's unsupported in websockets.

You aren't going to involve websockets anymore after sending such a response. You can ignore websockets and write directly to the network connection.

If it makes the implementation cleaner or easier, you could call websockets's send_response method with only the headers; then you would write the body as you generate it, without involving websockets.

[Kludex]

If it makes the implementation cleaner or easier, you could call websockets's send_response method with only the headers; then you would write the body as you generate it, without involving websockets.

So I reject with websockets, and then write the body? Just to confirm.

[aaugustin]

Well, reject isn't going to be entirely helpful, because it wants to set Content-Length:

https://github.com/aaugustin/websockets/blob/ba1ed7a65cc876ff4e0fcd4dd4711402836475e2/src/websockets/server.py#L505

If you want to rely as much as possible on public API to avoid breaking websockets' invariants, here's my recommendation:

1. Call reject() with a dummy body. It will return a Response object that doesn't look exactly the way you want, since you didn't have the right body yet.
2. Here you have two options:
   a. alter response returned by reject e.g. del response.headers["Content-Length"]
   b. ignore it and build a new response = Response(status.value, status.phrase, headers, body) from scratch
3. send_response(response)
4. ... = data_to_send() --> send that to the network
5. send the rest of the body to the network

If you find that annoying, you can just send what you want to the network and ignore exceptions thrown by websockets.

[Kludex]

I don't have bandwidth to do this. Whoever wants to do it, feel free to. I can review, give feedback, and also guide you on how to do it.

[parthvnp]

@Kludex Happy to take this one!

[Kludex]

@Kludex Happy to take this one!

Go ahead. 🙏

You can use #1911 if it helps. 😁

[gourav-kandoria]

Hi @Kludex Is this task still open. If it is , I tried implementing it and have opened a PR for it. Could you please review it.
#2060

[valentin994]

To implement SansIO shouldn't we use Server SansIO, but as the websocket is pinned in the requirements file the module is not available.

Are there any breaking changes with using a newer websocket release?

[aaugustin]

See #1927. It looks like an issue in the current implementation. If you're replacing it with a new implementation, you can safely bump the dependency.

[gourav-kandoria]

@aaugustin I implemented it on version 10.4. (#2060) where ServerConnection was providing sans-I/O. Changing the version to 11.0 would require to change it to ServerProtocol. and I guess, the apis of both the classes is same. or should some changes also be required there.

[aaugustin]

It's just a renaming.

There's another minor change:

Sans-I/O protocol constructors now use keyword-only arguments.

but it doesn't affect you because you're already using keyword arguments in your PR.

[gourav-kandoria]

@Kludex As websockets version was changed to 11.0.3 on master. So changed the pr accordingly

[piercefreeman]

@Kludex Looks like websockets.legacy was formally deprecated (and is now console warning to that effect) in 14.0. Can you read me in on the #2060 or whatever the latest effort is here? I'm happy to contribute to try to get this over the finish line.

[aaugustin]

Hello, I didn't track the latest status of this effort — as a reminder:

• Please @ me if you'd like me to review a design or an implementation. I will help.
• I maintain backwards-compatibility for 5 years so you don't have your backs to a wall. But good to take this opportunity to upgrade!