Skip to content

Commit

Permalink
Improved handling of rejected connections (#391 #487 #447)
Browse files Browse the repository at this point in the history
  • Loading branch information
@miguelgrinberg
miguelgrinberg committed May 22, 2020
1 parent b051893 commit 8d08096
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 28 deletions.
3 changes: 3 additions & 0 deletions socketio/asyncio_client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ async def connect(self, url, headers={}, transports=None,
transports=transports,
engineio_path=socketio_path)
except engineio.exceptions.ConnectionError as exc:
await self._trigger_event(
'connect_error', '/',
exc.args[1] if len(exc.args) > 1 else exc.args[0])
six.raise_from(exceptions.ConnectionError(exc.args[0]), None)
self.connected = True

Expand Down
7 changes: 4 additions & 3 deletions socketio/asyncio_server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,12 +426,13 @@ async def _handle_connect(self, sid, namespace):
self.manager.pre_disconnect(sid, namespace)
await self._send_packet(sid, packet.Packet(
packet.DISCONNECT, data=fail_reason, namespace=namespace))
self.manager.disconnect(sid, namespace)
if not self.always_connect:
elif namespace != '/':
await self._send_packet(sid, packet.Packet(
packet.ERROR, data=fail_reason, namespace=namespace))
if sid in self.environ: # pragma: no cover
self.manager.disconnect(sid, namespace)
if namespace == '/' and sid in self.environ: # pragma: no cover
del self.environ[sid]
return fail_reason or False
elif not self.always_connect:
await self._send_packet(sid, packet.Packet(packet.CONNECT,
namespace=namespace))
Expand Down
3 changes: 3 additions & 0 deletions socketio/client.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,9 @@ def connect(self, url, headers={}, transports=None,
self.eio.connect(url, headers=headers, transports=transports,
engineio_path=socketio_path)
except engineio.exceptions.ConnectionError as exc:
self._trigger_event(
'connect_error', '/',
exc.args[1] if len(exc.args) > 1 else exc.args[0])
six.raise_from(exceptions.ConnectionError(exc.args[0]), None)
self.connected = True

Expand Down
7 changes: 4 additions & 3 deletions socketio/server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -635,12 +635,13 @@ def _handle_connect(self, sid, namespace):
self.manager.pre_disconnect(sid, namespace)
self._send_packet(sid, packet.Packet(
packet.DISCONNECT, data=fail_reason, namespace=namespace))
self.manager.disconnect(sid, namespace)
if not self.always_connect:
elif namespace != '/':
self._send_packet(sid, packet.Packet(
packet.ERROR, data=fail_reason, namespace=namespace))
if sid in self.environ: # pragma: no cover
self.manager.disconnect(sid, namespace)
if namespace == '/' and sid in self.environ: # pragma: no cover
del self.environ[sid]
return fail_reason or False
elif not self.always_connect:
self._send_packet(sid, packet.Packet(packet.CONNECT,
namespace=namespace))
Expand Down
55 changes: 43 additions & 12 deletions tests/asyncio/test_asyncio_server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,29 +313,29 @@ def test_handle_connect_namespace(self, eio):
s.eio.send.mock.assert_any_call('123', '0/foo', binary=False)

def test_handle_connect_rejected(self, eio):
eio.return_value.send = AsyncMock()
mgr = self._get_mock_manager()
s = asyncio_server.AsyncServer(client_manager=mgr)
handler = mock.MagicMock(return_value=False)
s.on('connect', handler)
_run(s._handle_eio_connect('123', 'environ'))
ret = _run(s._handle_eio_connect('123', 'environ'))
self.assertFalse(ret)
handler.assert_called_once_with('123', 'environ')
self.assertEqual(s.manager.connect.call_count, 1)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})
s.eio.send.mock.assert_called_once_with('123', '4', binary=False)

def test_handle_connect_namespace_rejected(self, eio):
eio.return_value.send = AsyncMock()
mgr = self._get_mock_manager()
s = asyncio_server.AsyncServer(client_manager=mgr)
handler = mock.MagicMock(return_value=False)
s.on('connect', handler, namespace='/foo')
_run(s._handle_eio_connect('123', 'environ'))
ret = _run(s._handle_eio_connect('123', 'environ'))
_run(s._handle_eio_message('123', '0/foo'))
self.assertIsNone(ret)
self.assertEqual(s.manager.connect.call_count, 2)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})
self.assertEqual(s.environ, {'123': 'environ'})
s.eio.send.mock.assert_any_call('123', '4/foo', binary=False)

def test_handle_connect_rejected_always_connect(self, eio):
Expand All @@ -345,7 +345,8 @@ def test_handle_connect_rejected_always_connect(self, eio):
always_connect=True)
handler = mock.MagicMock(return_value=False)
s.on('connect', handler)
_run(s._handle_eio_connect('123', 'environ'))
ret = _run(s._handle_eio_connect('123', 'environ'))
self.assertFalse(ret)
handler.assert_called_once_with('123', 'environ')
self.assertEqual(s.manager.connect.call_count, 1)
self.assertEqual(s.manager.disconnect.call_count, 1)
Expand All @@ -360,11 +361,12 @@ def test_handle_connect_namespace_rejected_always_connect(self, eio):
always_connect=True)
handler = mock.MagicMock(return_value=False)
s.on('connect', handler, namespace='/foo')
_run(s._handle_eio_connect('123', 'environ'))
ret = _run(s._handle_eio_connect('123', 'environ'))
_run(s._handle_eio_message('123', '0/foo'))
self.assertFalse(ret)
self.assertEqual(s.manager.connect.call_count, 2)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})
self.assertEqual(s.environ, {'123': 'environ'})
s.eio.send.mock.assert_any_call('123', '0/foo', binary=False)
s.eio.send.mock.assert_any_call('123', '1/foo', binary=False)

Expand All @@ -375,11 +377,24 @@ def test_handle_connect_rejected_with_exception(self, eio):
handler = mock.MagicMock(
side_effect=exceptions.ConnectionRefusedError('fail_reason'))
s.on('connect', handler)
_run(s._handle_eio_connect('123', 'environ'))
ret = _run(s._handle_eio_connect('123', 'environ'))
self.assertEqual(ret, 'fail_reason')
self.assertEqual(s.manager.connect.call_count, 1)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})

def test_handle_connect_rejected_with_empty_exception(self, eio):
eio.return_value.send = AsyncMock()
mgr = self._get_mock_manager()
s = asyncio_server.AsyncServer(client_manager=mgr)
handler = mock.MagicMock(
side_effect=exceptions.ConnectionRefusedError())
s.on('connect', handler)
ret = _run(s._handle_eio_connect('123', 'environ'))
self.assertFalse(ret)
self.assertEqual(s.manager.connect.call_count, 1)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})
s.eio.send.mock.assert_any_call('123', '4"fail_reason"', binary=False)

def test_handle_connect_namespace_rejected_with_exception(self, eio):
eio.return_value.send = AsyncMock()
Expand All @@ -388,14 +403,30 @@ def test_handle_connect_namespace_rejected_with_exception(self, eio):
handler = mock.MagicMock(
side_effect=exceptions.ConnectionRefusedError('fail_reason', 1))
s.on('connect', handler, namespace='/foo')
_run(s._handle_eio_connect('123', 'environ'))
ret = _run(s._handle_eio_connect('123', 'environ'))
_run(s._handle_eio_message('123', '0/foo'))
self.assertIsNone(ret)
self.assertEqual(s.manager.connect.call_count, 2)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})
self.assertEqual(s.environ, {'123': 'environ'})
s.eio.send.mock.assert_any_call('123', '4/foo,["fail_reason",1]',
binary=False)

def test_handle_connect_namespace_rejected_with_empty_exception(self, eio):
eio.return_value.send = AsyncMock()
mgr = self._get_mock_manager()
s = asyncio_server.AsyncServer(client_manager=mgr)
handler = mock.MagicMock(
side_effect=exceptions.ConnectionRefusedError())
s.on('connect', handler, namespace='/foo')
ret = _run(s._handle_eio_connect('123', 'environ'))
_run(s._handle_eio_message('123', '0/foo'))
self.assertIsNone(ret)
self.assertEqual(s.manager.connect.call_count, 2)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {'123': 'environ'})
s.eio.send.mock.assert_any_call('123', '4/foo', binary=False)

def test_handle_disconnect(self, eio):
eio.return_value.send = AsyncMock()
mgr = self._get_mock_manager()
Expand Down
55 changes: 45 additions & 10 deletions tests/common/test_server.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,30 +285,33 @@ def test_handle_connect_rejected(self, eio):
s = server.Server(client_manager=mgr)
handler = mock.MagicMock(return_value=False)
s.on('connect', handler)
s._handle_eio_connect('123', 'environ')
ret = s._handle_eio_connect('123', 'environ')
self.assertFalse(ret)
handler.assert_called_once_with('123', 'environ')
self.assertEqual(s.manager.connect.call_count, 1)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})
s.eio.send.assert_called_once_with('123', '4', binary=False)

def test_handle_connect_namespace_rejected(self, eio):
mgr = mock.MagicMock()
s = server.Server(client_manager=mgr)
handler = mock.MagicMock(return_value=False)
s.on('connect', handler, namespace='/foo')
s._handle_eio_connect('123', 'environ')
ret = s._handle_eio_connect('123', 'environ')
s._handle_eio_message('123', '0/foo')
self.assertIsNone(ret)
self.assertEqual(s.manager.connect.call_count, 2)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {'123': 'environ'})
s.eio.send.assert_any_call('123', '4/foo', binary=False)

def test_handle_connect_rejected_always_connect(self, eio):
mgr = mock.MagicMock()
s = server.Server(client_manager=mgr, always_connect=True)
handler = mock.MagicMock(return_value=False)
s.on('connect', handler)
s._handle_eio_connect('123', 'environ')
ret = s._handle_eio_connect('123', 'environ')
self.assertFalse(ret)
handler.assert_called_once_with('123', 'environ')
self.assertEqual(s.manager.connect.call_count, 1)
self.assertEqual(s.manager.disconnect.call_count, 1)
Expand All @@ -321,37 +324,69 @@ def test_handle_connect_namespace_rejected_always_connect(self, eio):
s = server.Server(client_manager=mgr, always_connect=True)
handler = mock.MagicMock(return_value=False)
s.on('connect', handler, namespace='/foo')
s._handle_eio_connect('123', 'environ')
ret = s._handle_eio_connect('123', 'environ')
s._handle_eio_message('123', '0/foo')
self.assertIsNone(ret)
self.assertEqual(s.manager.connect.call_count, 2)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {'123': 'environ'})
s.eio.send.assert_any_call('123', '0/foo', binary=False)
s.eio.send.assert_any_call('123', '1/foo', binary=False)

def test_handle_connect_rejected_with_exception(self, eio):
mgr = mock.MagicMock()
s = server.Server(client_manager=mgr)
handler = mock.MagicMock(
side_effect=exceptions.ConnectionRefusedError('fail_reason'))
s.on('connect', handler)
ret = s._handle_eio_connect('123', 'environ')
self.assertEqual(ret, 'fail_reason')
handler.assert_called_once_with('123', 'environ')
self.assertEqual(s.manager.connect.call_count, 1)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})

def test_handle_connect_rejected_with_empty_exception(self, eio):
mgr = mock.MagicMock()
s = server.Server(client_manager=mgr)
handler = mock.MagicMock(
side_effect=exceptions.ConnectionRefusedError())
s.on('connect', handler)
s._handle_eio_connect('123', 'environ')
ret = s._handle_eio_connect('123', 'environ')
self.assertFalse(ret)
handler.assert_called_once_with('123', 'environ')
self.assertEqual(s.manager.connect.call_count, 1)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {})
s.eio.send.assert_any_call('123', '4', binary=False)

def test_handle_connect_namespace_rejected_with_exception(self, eio):
mgr = mock.MagicMock()
s = server.Server(client_manager=mgr)
handler = mock.MagicMock(
side_effect=exceptions.ConnectionRefusedError(u'fail_reason'))
side_effect=exceptions.ConnectionRefusedError(u'fail_reason', 1))
s.on('connect', handler, namespace='/foo')
s._handle_eio_connect('123', 'environ')
ret = s._handle_eio_connect('123', 'environ')
s._handle_eio_message('123', '0/foo')
self.assertIsNone(ret)
self.assertEqual(s.manager.connect.call_count, 2)
self.assertEqual(s.manager.disconnect.call_count, 1)
self.assertEqual(s.environ, {'123': 'environ'})
s.eio.send.assert_any_call('123', '4/foo,["fail_reason",1]',
binary=False)

def test_handle_connect_namespace_rejected_with_empty_exception(self, eio):
mgr = mock.MagicMock()
s = server.Server(client_manager=mgr)
handler = mock.MagicMock(
side_effect=exceptions.ConnectionRefusedError())
s.on('connect', handler, namespace='/foo')
ret = s._handle_eio_connect('123', 'environ')
s._handle_eio_message('123', '0/foo')
self.assertIsNone(ret)
self.assertEqual(s.manager.connect.call_count, 2)
self.assertEqual(s.manager.disconnect.call_count, 1)
s.eio.send.assert_any_call('123', '4/foo,"fail_reason"', binary=False)
self.assertEqual(s.environ, {'123': 'environ'})
s.eio.send.assert_any_call('123', '4/foo', binary=False)

def test_handle_disconnect(self, eio):
mgr = mock.MagicMock()
Expand Down

0 comments on commit 8d08096

Please sign in to comment.