Fix exception when disconnected by another client (#117)

Author: lgrahl

saltyrtc/server/protocol.py

@@ -124,7 +124,7 @@ def add_pending(self, client: 'PathClient') -> None:
         :class:`PathClient`.
         """
         if not self.attached:
-            raise ValueError('Patch has been detached!')
+            raise ValueError('Path has been detached!')
         self._pending.add(client)
 
     def has_client(self, client: 'PathClient') -> bool:
@@ -139,7 +139,7 @@ def has_client(self, client: 'PathClient') -> bool:
         :class:`PathClient`.
         """
         if not self.attached:
-            raise ValueError('Patch has been detached!')
+            raise ValueError('Path has been detached!')
 
         # Note: No need to check for an unassigned ID since the server's ID will never
         #       be available in the slots.
@@ -169,7 +169,7 @@ def get_initiator(self) -> 'PathClient':
               :class:`PathClient`.
         """
         if not self.attached:
-            raise ValueError('Patch has been detached!')
+            raise ValueError('Path has been detached!')
         if self._initiator is None:
             raise KeyError('No initiator present')
         return self._initiator
@@ -190,7 +190,7 @@ def set_initiator(self, initiator: 'PathClient') -> Optional['PathClient']:
         Return the previously set initiator or `None`.
         """
         if not self.attached:
-            raise ValueError('Patch has been detached!')
+            raise ValueError('Path has been detached!')
 
         # Remove initiator from 'pending' set
         self._pending.remove(initiator)
@@ -221,7 +221,7 @@ def get_responder(self, id_: ResponderAddress) -> 'PathClient':
               :class:`PathClient`.
         """
         if not self.attached:
-            raise ValueError('Patch has been detached!')
+            raise ValueError('Path has been detached!')
         return self._responders[id_]
 
     def get_responder_ids(self) -> Iterable[ResponderAddress]:
@@ -232,7 +232,7 @@ def get_responder_ids(self) -> Iterable[ResponderAddress]:
         :class:`PathClient`.
         """
         if not self.attached:
-            raise ValueError('Patch has been detached!')
+            raise ValueError('Path has been detached!')
         return self._responders.keys()
 
     def add_responder(self, responder: 'PathClient') -> ResponderAddress:
@@ -253,7 +253,7 @@ def add_responder(self, responder: 'PathClient') -> ResponderAddress:
         Return the assigned slot identifier.
         """
         if not self.attached:
-            raise ValueError('Patch has been detached!')
+            raise ValueError('Path has been detached!')
 
         # Calculate slot id
         id_ = len(self._responders) + 0x02
@@ -294,7 +294,7 @@ def remove_client(self, client: 'PathClient') -> None:
               :class:`PathClient`.
         """
         if not self.attached:
-            raise ValueError('Patch has been detached!')
+            raise ValueError('Path has been detached!')
 
         if client.state == ClientState.restricted:
             # Client has never been authenticated, so it should be in

saltyrtc/server/server.py

@@ -540,7 +540,14 @@ async def handle_client(self) -> None:
             # We can safely ignore this since clients will be removed immediately
             # from the path in case they are being dropped by another client.
             pass
-        self._server.paths.clean(path)
+        except ValueError:
+            # We can also safely ignore this since a client may have already removed
+            # itself from the path.
+            pass
+
+        # Clean the path (if still attached)
+        if path.attached:
+            self._server.paths.clean(path)
 
         # Done! Raise the result
         raise result
@@ -930,9 +937,6 @@ def get(self, initiator_key: InitiatorPublicPermanentKey) -> Path:
         return self.paths[initiator_key]
 
     def clean(self, path: Path) -> None:
-        if not path.attached:
-            self._log.error('Path {} has already been detached', path.number)
-            return
         if path.empty:
             path.attached = False
             try:

tests/test_server.py

@@ -945,7 +945,7 @@ async def handle_client(self):
         assert path.empty
         with pytest.raises(ValueError) as exc_info:
             path.has_client(initiator_path_client)
-        assert 'Patch has been detached!' in str(exc_info.value)
+        assert 'Path has been detached!' in str(exc_info.value)
 
         # Responder handshake
         responder, r = await client_factory(responder_handshake=True)
@@ -964,7 +964,7 @@ async def handle_client(self):
         assert path.empty
         with pytest.raises(ValueError) as exc_info:
             path.has_client(responder_path_client)
-        assert 'Patch has been detached!' in str(exc_info.value)
+        assert 'Path has been detached!' in str(exc_info.value)
 
         # Release initiator protocol instance
         responder_connected_future.set_result(None)