Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed Behaviour Change 14.0 #1538

Closed
cl-dqh opened this issue Nov 10, 2024 · 5 comments
Closed

Closed Behaviour Change 14.0 #1538

cl-dqh opened this issue Nov 10, 2024 · 5 comments
Labels

Comments

@cl-dqh
Copy link

cl-dqh commented Nov 10, 2024

One of our tests started failing as of 14.0 - if the server sends a message then closes the websocket before the client has recv() the message, then the client will not see the message before the ConnectionClosed event.

This test passes in 13.1, but fails in 14.0:

import asyncio
import websockets

server_done_event   = None
client_ws           = None

async def server_handler(ws):
    # Send a message, then immediately close the connection
    await ws.send("Hello, Client!")
    await ws.close()
    # Signal that the server has completed its task
    server_done_event.set()

async def server_start():
    # Start the WebSocket server
    server = await websockets.serve(server_handler, "localhost", 8765)
    print("Server started on ws://localhost:8765")
    return server

async def client_connect():
    global client_ws
    # Connect to the WebSocket server and handle messages
    client_ws = await websockets.connect("ws://localhost:8765")

async def client_recv():
    recieved_message = False
    while True:
        try:
            # Then attempt to receive any remaining message frames
            message = await client_ws.recv()
            print("Client received message:", message)
            recieved_message = True
        except websockets.ConnectionClosed:
            if not recieved_message:
                print("FAIL: Client did not recieve any messages before the connection was closed.")
            else:
                print("SUCCESS: Client received message.")
            break

async def main():
    global server_done_event

    # Create an event to signal when the server is done
    server_done_event   = asyncio.Event()
    server              = await server_start()

    # Wait for the client to connect, and for the server to respond to it
    await asyncio.gather(
        asyncio.create_task(client_connect()),
        server_done_event.wait()
    )

    # Now attempt to recieve the message from the server
    await client_recv()

# Run the script
asyncio.run(main())
@aaugustin
Copy link
Member

Thank you for reporting this problem.

Here's an initial guess at what could be happening:

  • the Sans-I/O layer receives the message and the close frame back-to-back;
  • the asyncio layer queues the message with Assembler.put then immediately closes the queue with Assembler.close (TBD where this happens exactly) — I believe that this clears the contents of the queue as well;
  • Assembler.get says that the queue is closed.

I agree that it's reasonable to expect that you can read the message in this scenario (although there may be other scenarios indistinguishable from this one where you'd expect data to be discarded).

@aaugustin aaugustin added the bug label Nov 11, 2024
@aaugustin
Copy link
Member

As a short term fix, you can use websockets.legacy.client.connect instead of websockets.connect. See https://websockets.readthedocs.io/en/latest/project/changelog.html and https://websockets.readthedocs.io/en/latest/howto/upgrade.html for context.

@aaugustin
Copy link
Member

(Then you'll have five years to switch to websockets.asyncio.client.connect.)

@aaugustin
Copy link
Member

Thank you for the effort to provide a minimal reproduction script. It passes on the PR:

‹issue-1538›$ PYTHONPATH=src python issue-1538.py
Server started on ws://localhost:8765
Client received message: Hello, Client!
SUCCESS: Client received message.

while failing on the main branch:

‹main›$ PYTHONPATH=src python issue-1538.py
Server started on ws://localhost:8765
FAIL: Client did not recieve any messages before the connection was closed.

@aaugustin
Copy link
Member

FYI: I'm planning to wait a few days for other reports of incompatibilities between the legacy and new implementation, then I'll make a release with that fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants