Handle network errors

- Move HTTP connect/disconnect logic to networkManager
- If a talk fails due to socket error, HTTP transport error or
  nio bad response that might change, retry every 2s until success
- Clean up some leftover debug prints

TODO.md

@@ -7,8 +7,6 @@
   it should be the peer's display name instead.
 - Support "Empty room (was ...)" after peer left
 
-- Catch network errors in socket operations
-
 - Proper logoff when closing client
 
 - Handle cases where an avatar char is # or @ (#alias room, @user\_id)

harmonyqml/backend/client.py

@@ -79,10 +79,7 @@ class Client(QObject):
     @pyqtSlot(str, str)
     @futurize
     def login(self, password: str, device_name: str = "") -> None:
-        self.net.write(self.nio.connect())
         response = self.net.talk(self.nio.login, password, device_name)
-
-        self.net_sync.write(self.nio_sync.connect())
         self.nio_sync.receive_response(response)
 
 
@@ -90,11 +87,8 @@ class Client(QObject):
     @futurize
     def resumeSession(self, user_id: str, token: str, device_id: str
                      ) -> None:
-        self.net.write(self.nio.connect())
         response = nr.LoginResponse(user_id, device_id, token)
         self.nio.receive_response(response)
-
-        self.net_sync.write(self.nio_sync.connect())
         self.nio_sync.receive_response(response)
 
 
@@ -102,8 +96,8 @@ class Client(QObject):
     @futurize
     def logout(self) -> None:
         self._stop_sync.set()
-        self.net.write(self.nio.disconnect())
-        self.net_sync.write(self.nio_sync.disconnect())
+        self.net.http_disconnect()
+        self.net_sync.http_disconnect()
 
 
     @pyqtSlot()
@@ -181,24 +175,19 @@ class Client(QObject):
         set_for_secs        = 5
         last_set, last_time = self._last_typing_set[room_id]
 
-        print(last_set, last_time)
-
         if not typing and last_set is False:
-            print("ignore 1")
             return
 
         if typing and time.time() - last_time < set_for_secs - 1:
-            print("ignore 2")
             return
 
-        print("SET", typing)
         self._last_typing_set[room_id] = (typing, time.time())
 
         self.net.talk(
             self.nio.room_typing,
-            room_id      = room_id,
-            typing_state = typing,
-            timeout      = set_for_secs * 1000,
+            room_id        = room_id,
+            typing_state   = typing,
+            timeout        = set_for_secs * 1000,
         )
 
 

harmonyqml/backend/html_filter.py

@@ -3,11 +3,12 @@
 
 import re
 
-import html_sanitizer.sanitizer as sanitizer
 import mistune
 from lxml.html import HtmlElement, etree
 from PyQt5.QtCore import QObject, pyqtProperty, pyqtSlot
 
+import html_sanitizer.sanitizer as sanitizer
+
 
 class HtmlFilter(QObject):
     link_regexes = [re.compile(r, re.IGNORECASE) for r in [

harmonyqml/backend/network_manager.py

@@ -11,6 +11,7 @@ from uuid import UUID
 
 import nio
 import nio.responses as nr
+from nio.exceptions import RemoteTransportError
 
 OptSock        = Optional[ssl.SSLSocket]
 NioRequestFunc = Callable[..., Tuple[UUID, bytes]]
@@ -48,7 +49,10 @@ class NetworkManager:
 
 
     @staticmethod
-    def _close_socket(sock: socket.socket) -> None:
+    def _close_socket(sock: Optional[socket.socket]) -> None:
+        if not sock:
+            return
+
         try:
             sock.shutdown(how=socket.SHUT_RDWR)
         except OSError:  # Already closer by server
@@ -56,6 +60,14 @@ class NetworkManager:
         sock.close()
 
 
+    def http_disconnect(self) -> None:
+        data = self.nio.disconnect()
+        try:
+            self.write(data)
+        except (OSError, RemoteTransportError):
+            pass
+
+
     def read(self, with_sock: OptSock = None) -> nr.Response:
         sock = with_sock or self._get_socket()
 
@@ -78,9 +90,6 @@ class NetworkManager:
 
 
     def write(self, data: bytes, with_sock: OptSock = None) -> None:
-        if not data:
-            return
-
         sock = with_sock or self._get_socket()
         sock.sendall(data)
 
@@ -88,34 +97,47 @@ class NetworkManager:
             self._close_socket(sock)
 
 
-    def talk(self, nio_func: NioRequestFunc, *args, **kwargs) -> nr.Response:
+    def talk(self,
+             nio_func: NioRequestFunc,
+             *args,
+             **kwargs) -> nr.Response:
         with self._lock:
             while True:
-                to_send = nio_func(*args, **kwargs)[1]
-                sock    = self._get_socket()
+                sock = None
 
                 try:
+                    sock = self._get_socket()
+
+                    if not self.nio.connection:
+                        # Establish HTTP protocol connection:
+                        self.write(self.nio.connect(), sock)
+
+                    to_send = nio_func(*args, **kwargs)[1]
                     self.write(to_send, sock)
                     response = self.read(sock)
 
+                except OSError as err:
+                    logging.error("Socket error for %s: %s",
+                                  nio_func.__name__, err.strerror)
+                    self._close_socket(sock)
+                    time.sleep(2)
+
+                except RemoteTransportError as err:
+                    logging.error("HTTP transport error for %s: %s",
+                                  nio_func.__name__, err)
+                    self._close_socket(sock)
+                    self.http_disconnect()
+                    time.sleep(2)
+
                 except NioErrorResponse as err:
-                    logging.error("bad read for %s: %s", nio_func, err)
+                    logging.error("Nio response error for %s: %s",
+                                  nio_func.__name__, err)
                     self._close_socket(sock)
 
-                    if self._should_abort_talk(err):
-                        logging.error("aborting talk")
-                        break
+                    if err.response.status_code in self.http_retry_codes:
+                        return response
 
-                    time.sleep(10)
+                    time.sleep(2)
 
                 else:
-                    break
-
-            self._close_socket(sock)
-            return response
-
-
-    def _should_abort_talk(self, err: NioErrorResponse) -> bool:
-        if err.response.status_code in self.http_retry_codes:
-            return False
-        return True
+                    return response

harmonyqml/components/chat/SendBox.qml

@@ -62,7 +62,6 @@ Rectangle {
                     }
 
                     if (textArea.text === "") { return }
-
                     Backend.clientManager.clients[chatPage.user_id]
                            .sendMarkdown(chatPage.room.room_id, textArea.text)
                     textArea.clear()