miguelgrinberg / flask-sock

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

Why is a StopIteration raised after closing a websocket connection? #64

Open jakeane opened 1 year ago

jakeane commented 1 year ago

I feel like I do not understand the reasoning behind the logic of Sock.route, specifically this section

class WebSocketResponse(Response):
    def __call__(self, *args, **kwargs):
        if ws.mode == 'eventlet':
            try:
                from eventlet.wsgi import WSGI_LOCAL
                ALREADY_HANDLED = []
            except ImportError:
                from eventlet.wsgi import ALREADY_HANDLED
                WSGI_LOCAL = None

            if hasattr(WSGI_LOCAL, 'already_handled'):
                WSGI_LOCAL.already_handled = True
            return ALREADY_HANDLED
        elif ws.mode == 'gunicorn':
            raise StopIteration()
        elif ws.mode == 'werkzeug':
            raise ConnectionError()
        else:
            return []

return WebSocketResponse()

Why is StopIteration raised in the case where ws.mode == gunicorn? I feel like there is something I am missing here. As far as I can see, it is giving the impression in Datadog that the websocket is failing, even though it isn't.

If raising an error is intended, then is there a workaround to this? Right now I am creating another decorator to catch/ignore this error:

def catch_websocket_stop_iteration(f):
    @wraps(f)
    def wrapper(*args, **kwargs):
        try:
            return f(*args, **kwargs)
        except StopIteration:
            return []
    return wrapper

If an explanation could be provided for this, that would be appreciated.

miguelgrinberg commented 1 year ago

First of all, the StopIteration exception is not an error, it is a perfectly normal exception to raise when there is no more items to get from an iterable. If Datadog considers this an error, then Datadog is wrong.

The reason is that WebSocket is not supported by WSGI, so WSGI servers implement a variety of non-standard ways to support WebSocket as an extension to WSGI. A key part of this support is how to tell the web server that the WebSocket connection ended, so that the server closes the request cleanly, without writing an HTTP response and without causing any errors. For Gunicorn, raising StopIteration achieves this goal.

jakeane commented 1 year ago

Thank you for the explanation. So if I did try catching the StopIteration, would that potentially cause issues? And what exactly does raising StopIteration do? Just prevent an HTTP response from being written?

miguelgrinberg commented 1 year ago

The StopIteration is handled by Gunicorn:

https://github.com/benoitc/gunicorn/blob/ab9c8301cb9ae573ba597154ddeea16f0326fc15/gunicorn/workers/sync.py#L128-L139

If you prevent this and return [] instead then Gunicorn is going to write an empty HTTP response on the WebSocket connection. That may or may not cause any visible problems, but it is nonetheless the wrong way to close the socket.

jakeane commented 1 year ago

I see. Right now, we are using gevent as the worker for our server (via gunicorn -w 8 -k gevent ...), and it seems like it doesn't catch the error. I don't think it uses the code you just shared.

It seems like ws.mode = gunicorn, not sure if it should be ws.mode = gevent. Not sure how the gevent workers operate, so I don't know if there is handling for StopIteration there.

Does something seem off here to you? I can provide more detail if needed.

miguelgrinberg commented 1 year ago

@jakeane My goal was to support Gunicorn with the threaded worker. I now realize that the link I shared above points to the sync worker instead. The threaded worker does have similar logic to abort a connection without issuing a response when StopIteration is raised. From a quick review it appears that the eventlet worker also uses this pattern, but the gevent worker does not.

If you are interested in continuing with gevent, then you can try dropping Gunicorn and using gevent alone, which is what is meant when the mode is gevent. See the documentation for details on how to start the gevent WSGI server.