Skip to content

Commit

Permalink
Avoid double calls to client disconnect handlers
Browse files Browse the repository at this point in the history
Fixes #261
  • Loading branch information
miguelgrinberg committed Feb 27, 2019
1 parent 1fb1f6d commit f752312
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 18 deletions.
4 changes: 2 additions & 2 deletions socketio/asyncio_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,11 +235,11 @@ async def disconnect(self):
Note: this method is a coroutine.
"""
# here we just request the disconnection
# later in _handle_eio_disconnect we invoke the disconnect handler
for n in self.namespaces:
await self._trigger_event('disconnect', namespace=n)
await self._send_packet(packet.Packet(packet.DISCONNECT,
namespace=n))
await self._trigger_event('disconnect', namespace='/')
await self._send_packet(packet.Packet(
packet.DISCONNECT, namespace='/'))
await self.eio.disconnect(abort=True)
Expand Down
6 changes: 3 additions & 3 deletions socketio/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ def call(self, event, data=None, namespace=None, timeout=60):
def event_callback(*args):
callback_args.append(args)
callback_event.set()

self.emit(event, data=data, namespace=namespace,
callback=event_callback)
if not callback_event.wait(timeout=timeout):
Expand All @@ -328,10 +328,10 @@ def event_callback(*args):

def disconnect(self):
"""Disconnect from the server."""
# here we just request the disconnection
# later in _handle_eio_disconnect we invoke the disconnect handler
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)
Expand Down
15 changes: 8 additions & 7 deletions tests/asyncio/test_asyncio_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,9 @@ def test_disconnect(self):
c._send_packet = AsyncMock()
c.eio = mock.MagicMock()
c.eio.disconnect = AsyncMock()
c.eio.state = 'connected'
_run(c.disconnect())
c._trigger_event.mock.assert_called_once_with(
'disconnect', namespace='/')
self.assertEqual(c._trigger_event.mock.call_count, 0)
self.assertEqual(c._send_packet.mock.call_count, 1)
expected_packet = packet.Packet(packet.DISCONNECT, namespace='/')
self.assertEqual(c._send_packet.mock.call_args_list[0][0][0].encode(),
Expand All @@ -305,12 +305,11 @@ def test_disconnect_namespaces(self):
c.namespaces = ['/foo', '/bar']
c._trigger_event = AsyncMock()
c._send_packet = AsyncMock()
c.eio = mock.MagicMock()
c.eio.disconnect = AsyncMock()
c.eio.state = 'connected'
_run(c.disconnect())
self.assertEqual(c._trigger_event.mock.call_args_list, [
mock.call('disconnect', namespace='/foo'),
mock.call('disconnect', namespace='/bar'),
mock.call('disconnect', namespace='/')
])
self.assertEqual(c._trigger_event.mock.call_count, 0)
self.assertEqual(c._send_packet.mock.call_count, 3)
expected_packet = packet.Packet(packet.DISCONNECT, namespace='/foo')
self.assertEqual(c._send_packet.mock.call_args_list[0][0][0].encode(),
Expand Down Expand Up @@ -632,6 +631,7 @@ def test_handle_eio_message(self):
def test_eio_disconnect(self):
c = asyncio_client.AsyncClient()
c._trigger_event = AsyncMock()
c.eio.state = 'connected'
_run(c._handle_eio_disconnect())
c._trigger_event.mock.assert_called_once_with(
'disconnect', namespace='/')
Expand All @@ -640,6 +640,7 @@ def test_eio_disconnect_namespaces(self):
c = asyncio_client.AsyncClient()
c.namespaces = ['/foo', '/bar']
c._trigger_event = AsyncMock()
c.eio.state = 'connected'
_run(c._handle_eio_disconnect())
c._trigger_event.mock.assert_any_call('disconnect', namespace='/foo')
c._trigger_event.mock.assert_any_call('disconnect', namespace='/bar')
Expand Down
15 changes: 9 additions & 6 deletions tests/common/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,9 @@ def test_disconnect(self):
c._trigger_event = mock.MagicMock()
c._send_packet = mock.MagicMock()
c.eio = mock.MagicMock()
c.eio.state = 'connected'
c.disconnect()
c._trigger_event.assert_called_once_with('disconnect', namespace='/')
self.assertEqual(c._trigger_event.call_count, 0)
self.assertEqual(c._send_packet.call_count, 1)
expected_packet = packet.Packet(packet.DISCONNECT, namespace='/')
self.assertEqual(c._send_packet.call_args_list[0][0][0].encode(),
Expand All @@ -389,12 +390,10 @@ def test_disconnect_namespaces(self):
c.namespaces = ['/foo', '/bar']
c._trigger_event = mock.MagicMock()
c._send_packet = mock.MagicMock()
c.eio = mock.MagicMock()
c.eio.state = 'connected'
c.disconnect()
self.assertEqual(c._trigger_event.call_args_list, [
mock.call('disconnect', namespace='/foo'),
mock.call('disconnect', namespace='/bar'),
mock.call('disconnect', namespace='/')
])
self.assertEqual(c._trigger_event.call_count, 0)
self.assertEqual(c._send_packet.call_count, 3)
expected_packet = packet.Packet(packet.DISCONNECT, namespace='/foo')
self.assertEqual(c._send_packet.call_args_list[0][0][0].encode(),
Expand Down Expand Up @@ -731,13 +730,17 @@ def test_handle_eio_message(self):
def test_eio_disconnect(self):
c = client.Client()
c._trigger_event = mock.MagicMock()
c.start_background_task = mock.MagicMock()
c.eio.state = 'connected'
c._handle_eio_disconnect()
c._trigger_event.assert_called_once_with('disconnect', namespace='/')

def test_eio_disconnect_namespaces(self):
c = client.Client()
c.namespaces = ['/foo', '/bar']
c._trigger_event = mock.MagicMock()
c.start_background_task = mock.MagicMock()
c.eio.state = 'connected'
c._handle_eio_disconnect()
c._trigger_event.assert_any_call('disconnect', namespace='/foo')
c._trigger_event.assert_any_call('disconnect', namespace='/bar')
Expand Down

0 comments on commit f752312

Please sign in to comment.