websockets icon indicating copy to clipboard operation
websockets copied to clipboard

threading implementation with `process_request` raise ConnectionClosedError

Open Flowrey opened this issue 2 years ago • 3 comments

I'm using:

  • Python: 3.12
  • websockets: 12.0

When using the threading implementation like in this code:

import http
from websockets.sync.server import serve, ServerConnection, Request, Response
from websockets.http11 import datastructures

def health_check(websocket: ServerConnection, request: Request):
    if request.path == "/healthz":
        return Response(http.HTTPStatus.OK, "OK", datastructures.Headers([]), b"OK")
    else:
        return None


def echo(websocket):
    for message in websocket:
        websocket.send(message)

def main():
    with serve(echo, "localhost", 8765, process_request=health_check) as server:
        server.serve_forever()

main()

Any call to the /healthz endpoint works but raise a ConnectionCloseError:

connection handler failed
Traceback (most recent call last):
  File "/home/flowrey/Code/poc/venv/lib64/python3.12/site-packages/websockets/sync/server.py", line 499, in conn_handler
    handler(connection)
  File "/home/flowrey/Code/poc/server.py", line 13, in echo
    for message in websocket:
  File "/home/flowrey/Code/poc/venv/lib64/python3.12/site-packages/websockets/sync/connection.py", line 162, in __iter__
    yield self.recv()
          ^^^^^^^^^^^
  File "/home/flowrey/Code/poc/venv/lib64/python3.12/site-packages/websockets/sync/connection.py", line 201, in recv
    raise self.protocol.close_exc from self.recv_events_exc
websockets.exceptions.ConnectionClosedError: no close frame received or sent

Flowrey avatar Nov 28 '23 19:11 Flowrey

I noticed that it is receiving empty bytes as a data response, and this causes the socket to close. https://github.com/python-websockets/websockets/blob/e217458ef8b692e45ca6f66c5aeb7fad0aee97ee/src/websockets/sync/connection.py#L549-L550 https://github.com/python-websockets/websockets/blob/e217458ef8b692e45ca6f66c5aeb7fad0aee97ee/src/websockets/sync/connection.py#L614-L616

Another thing is that when the status code is not 101, the state does not end up being OPEN, which then closes the connection as well. https://github.com/python-websockets/websockets/blob/e217458ef8b692e45ca6f66c5aeb7fad0aee97ee/src/websockets/server.py#L541-L547 https://github.com/python-websockets/websockets/blob/e217458ef8b692e45ca6f66c5aeb7fad0aee97ee/src/websockets/sync/server.py#L143-L146

I have made this PR that solved this error for me, but I don't know if it broke tests or other functinalities https://github.com/python-websockets/websockets/pull/1440

MauroAntonino avatar Feb 18 '24 14:02 MauroAntonino

The change causes tests to hang -- because you removed the code paths that manages the end of a connection.

The issue looks legit but that's not the right fix.

aaugustin avatar Feb 19 '24 15:02 aaugustin

I think I figured it out now. The code is handling all connections as WebSocket connections. When it is an HTTP connection and a request is made, it is supposed to close the connection, which differs from a WebSocket, where it is expected to keep the connection alive. I created a checker to verify if it is a WebSocket before handling it.

I have created this new PR with this change: https://github.com/python-websockets/websockets/pull/1443

MauroAntonino avatar Feb 21 '24 16:02 MauroAntonino