miguelgrinberg / flask-sock

Modern WebSocket support for Flask.
MIT License
272 stars 24 forks source link

Question: Any plans to officially support Cheroot? #62

Closed Guiorgy closed 1 year ago

Guiorgy commented 1 year ago

Any plans to officially support Cheroot?

Replacing

elif environ.get('SERVER_SOFTWARE', '').startswith('gevent')

with

elif any(environ.get('SERVER_SOFTWARE', '').startswith(name) for name in ['gevent', 'Cheroot']):

seemed to work:

C:\WINDOWS\system32>wscat -P --connect http://127.0.0.1:5000/reverse
Connected (press CTRL+C to quit)
> 123
< 321
< Received ping
> abc
< cba
< Received ping
>

Although I had to set ping_interval to 5, since cheroot seems to timeout at around 10-15 seconds.

miguelgrinberg commented 1 year ago

I really have no knowledge of this web server and you are the first ever to mention it, so no, I have no plans to add support for it at this time.

Decreasing the ping_interval is a bad idea, as that creates additional overhead on your connections. It is better to make the timeout longer in your server.

Guiorgy commented 1 year ago

ping_interval was just for testing, it basically work almost out of the box.

There's this performance comparison where it performed pretty well (CheeryPy), at least better than Gunicorn.

Guiorgy commented 1 year ago

For anybody interested in the future:

Increasing the timeout works as expected.

The only small annoyance was an exception happening inside cheroot. However, after looking into it, it was happening because the start_response callback was never called since you overwrote the __call__ method and didn't call the super. The cheroot server can't seem to handle that.

Modifying the WebSocketResponse resolved the exception:

...
elif ws.mode == 'werkzeug':
    raise ConnectionError()
elif ws.mode == 'cheroot':
    super().__call__(*args, **kwargs)
    return []
else:
    return []

If I understand correctly, the intention is to tell the server that the request is already handled and to skip handling the response, but cheroot doesn't seem to have any way to do that.

miguelgrinberg commented 1 year ago

@Guiorgy for most server calling start_response causes the response header to be written out to the socket, which is not the correct way to handle a WebSocket request. Maybe calling this function happens to work in your case, but I would imagine the client will be receiving some garbage in the WebSocket connection, which may be tolerated by some clients, but not by others. The real problem here is that your web server does not recognize WebSocket connections, which is actually a very common problem to have with WSGI servers.