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

reconnection fails when server returns op_code = ABNF.OPCODE_CLOSE #976

Open
polypauldeep opened this issue Mar 19, 2024 · 5 comments · May be fixed by #983
Open

reconnection fails when server returns op_code = ABNF.OPCODE_CLOSE #976

polypauldeep opened this issue Mar 19, 2024 · 5 comments · May be fixed by #983

Comments

@polypauldeep
Copy link

polypauldeep commented Mar 19, 2024

Hello, I'm an active user of this library, using it to gather data via websocket.

I noticed the reconnect option available when employing the run_forever method. However, I've encountered a situation where if the server issues an OPCODE_CLOSE (8) message, the run_forever execution concludes. This occurs because the control flow is directed to:

if op_code == ABNF.OPCODE_CLOSE:
return teardown(frame)

resulting in the teardown of the connection.

Is this behavior by design? My understanding was that the connection should attempt a reconnect using the reconnect parameter, irrespective of the server's message. It appears that this situation arises because it's not captured by WebSocketConnectionClosedException, leading to keep_running returning false prior to reaching:

while self.keep_running:
_logging.debug(
f"Calling dispatcher reconnect [{len(inspect.stack())} frames in stack]"
)
dispatcher.reconnect(reconnect, setSock)

I conducted a test using the ws.run_forever method with the following parameters:

ws.run_forever(
    ping_interval=10,
    ping_timeout=5,
    reconnect=30
)
@polypauldeep
Copy link
Author

polypauldeep commented Mar 19, 2024

I've made an adjustment to one line in order to resolve the issue mentioned above.
polypauldeep@360c33f

Let's have a discussion before I proceed with creating a pull request for this change.

@engn33r
Copy link
Collaborator

engn33r commented Mar 24, 2024

This looks similar to PR #972, which was merged a couple weeks ago but I forgot to push a new version since then. Can you pull the latest code from master and check if that solves your problem? I can release a new version after you confirm whether this helps.

@polypauldeep
Copy link
Author

I tested the scenario described above using the current master branch and observed that the issue remains unresolved.

To reproduce the issue, I created a simple websocket server using the websockets library.

  1. This server sends a "wow" message to the clients every 500 ms once a connection is established.
  2. If the server receives a "hello" message from a client, it responds with a close message containing "wrong message."
import asyncio
import websockets

async def send_wow_periodically(websocket):
    try:
        while True:
            await websocket.send("wow")
            await asyncio.sleep(0.5)  # Wait for 500ms before sending the next message
    except websockets.exceptions.ConnectionClosed:
        print("Connection closed, stopping periodic messages.")

async def websocket_handler(websocket, path):
    task = asyncio.create_task(send_wow_periodically(websocket))

    try:
        async for message in websocket:
            if message == "hello":
                # If the message is "hello", send "wrong message" and cancel the periodic task
                await websocket.send("wrong message")
                print("Sent 'wrong message', closing connection.")
                task.cancel()  # Cancel the periodic "wow" messages
                try:
                    await task  # Optionally wait for the task cancellation to complete
                except asyncio.CancelledError:
                    pass
                await websocket.close(reason="Wrong message received")
                break
    except websockets.exceptions.ConnectionClosed as e:
        print(f"Connection closed with error: {e}")
        task.cancel()  # Ensure the task is cancelled if the connection is closed unexpectedly

async def main():
    # Define the server to listen on localhost:9001
    async with websockets.serve(websocket_handler, "0.0.0.0", 9001):
        print("WebSocket Server Started on ws://0.0.0.0:9001")
        await asyncio.Future()  # This keeps the server running indefinitely

# Run the server
asyncio.run(main())

Due to the below line,

if op_code == ABNF.OPCODE_CLOSE:
return teardown(frame)

client will be closed without reconnection with below debug message

++Sent decoded: fin=1 opcode=1 data=b'hello'
++Rcv raw: b'\x88\x18\x03\xe8Wrong message received'
++Rcv decoded: fin=1 opcode=8 data=b'\x03\xe8Wrong message received'
++Sent raw: b'\x88\x82\xd4#\xeb\xc8\xd7\xcb'
++Sent decoded: fin=1 opcode=8 data=b'\x03\xe8'
code 1000: Wrong message received

With modified version

  if op_code == ABNF.OPCODE_CLOSE and not reconnect:
      return teardown(frame)

It will not teardown connection by client itself, and try to reconnect as intended

websocket._exceptions.WebSocketConnectionClosedException: Connection to remote host was lost.
Connection to remote host was lost. - reconnect
Calling dispatcher reconnect [5 frames in stack]
reconnect() - retrying in 5 seconds [6 frames in stack]

@yaroshoris
Copy link

I faced with the same problem after getting from binance 1008 error with message "Pong timeout".

if op_code == ABNF.OPCODE_CLOSE and not reconnect:
    return teardown(frame)

after this improvements all seems to work smoothly.

@polypauldeep
Copy link
Author

@engn33r Could you review this issue (PR open: #978 ) or provide comments to help resolve it?

@bubbleboy14 bubbleboy14 linked a pull request May 9, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants