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

Namespace Disconnects called twice? #261

Closed
harmon opened this issue Feb 26, 2019 · 1 comment
Closed

Namespace Disconnects called twice? #261

harmon opened this issue Feb 26, 2019 · 1 comment
Assignees
Labels

Comments

@harmon
Copy link

harmon commented Feb 26, 2019

Main issue:
Calling client.disconnect() ends up triggering disconnect events for all namespaces twice. I think it should be called only once, but wanted to get your opinion.

Flow:
If I call socketio's client.disconnect(), it will call each namespace's disconnect listeners:

def disconnect(self):
"""Disconnect from the server."""
for n in self.namespaces:
self._trigger_event('disconnect', namespace=n)
self._send_packet(packet.Packet(packet.DISCONNECT, namespace=n))
self._trigger_event('disconnect', namespace='/')
self._send_packet(packet.Packet(
packet.DISCONNECT, namespace='/'))
self.eio.disconnect(abort=True)

It then calls engineio's disconnect:

https://github.com/miguelgrinberg/python-engineio/blob/d6a33d22cfd3ebe8b4d78cd5c27607de837d16e9/engineio/client.py#L191-L211

Which then ends up calling all the namespace's disconnects again:

def _handle_eio_disconnect(self):
"""Handle the Engine.IO disconnection event."""
self.logger.info('Engine.IO connection dropped')
for n in self.namespaces:
self._trigger_event('disconnect', namespace=n)
self._trigger_event('disconnect', namespace='/')
self.callbacks = {}
self._binary_packet = None
if self.eio.state == 'connected' and self.reconnection:
self._reconnect_task = self.start_background_task(
self._handle_reconnect)

Would it be better to just rely on the namespace disconnects getting called from _handle_eio_disconnect() by changing def disconnect() to:

def disconnect(self):
        """Disconnect from the server."""
       for n in self.namespaces:
#          self._trigger_event('disconnect', namespace=n)
           self._send_packet(packet.Packet(packet.DISCONNECT, namespace=n))
#       self._trigger_event('disconnect', namespace='/')
        self._send_packet(packet.Packet(
            packet.DISCONNECT, namespace='/'))
        self.eio.disconnect(abort=True)

PS: Thanks for the great library!

@harmon
Copy link
Author

harmon commented Feb 28, 2019

Damn, son! That was quick! I wasn't sure if it was a valid bug, but if I find any more I'll see if I can submit a PR.

Thanks!

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