Merge pull request #4094 from SomberNight/labels_plugin_catch_exc

labels plugin: better exception handling

plugins/labels/labels.py

@@ -45,7 +45,7 @@ class LabelsPlugin(BasePlugin):
 
     @hook
     def set_label(self, wallet, item, label):
-        if not wallet in self.wallets:
+        if wallet not in self.wallets:
             return
         if not item:
             return
@@ -55,7 +55,7 @@ class LabelsPlugin(BasePlugin):
                   "walletNonce": nonce,
                   "externalId": self.encode(wallet, item),
                   "encryptedLabel": self.encode(wallet, label)}
-        t = threading.Thread(target=self.do_request,
+        t = threading.Thread(target=self.do_request_safe,
                              args=["POST", "/label", False, bundle])
         t.setDaemon(True)
         t.start()
@@ -78,8 +78,18 @@ class LabelsPlugin(BasePlugin):
             raise BaseException(response["error"])
         return response
 
+    def do_request_safe(self, *args, **kwargs):
+        try:
+            self.do_request(*args, **kwargs)
+        except BaseException as e:
+            #traceback.print_exc(file=sys.stderr)
+            self.print_error('error doing request')
+
     def push_thread(self, wallet):
-        wallet_id = self.wallets[wallet][2]
+        wallet_data = self.wallets.get(wallet, None)
+        if not wallet_data:
+            raise Exception('Wallet {} not loaded'.format(wallet))
+        wallet_id = wallet_data[2]
         bundle = {"labels": [],
                   "walletId": wallet_id,
                   "walletNonce": self.get_nonce(wallet)}
@@ -95,42 +105,47 @@ class LabelsPlugin(BasePlugin):
         self.do_request("POST", "/labels", True, bundle)
 
     def pull_thread(self, wallet, force):
-        wallet_id = self.wallets[wallet][2]
+        wallet_data = self.wallets.get(wallet, None)
+        if not wallet_data:
+            raise Exception('Wallet {} not loaded'.format(wallet))
+        wallet_id = wallet_data[2]
         nonce = 1 if force else self.get_nonce(wallet) - 1
         self.print_error("asking for labels since nonce", nonce)
+        response = self.do_request("GET", ("/labels/since/%d/for/%s" % (nonce, wallet_id) ))
+        if response["labels"] is None:
+            self.print_error('no new labels')
+            return
+        result = {}
+        for label in response["labels"]:
+            try:
+                key = self.decode(wallet, label["externalId"])
+                value = self.decode(wallet, label["encryptedLabel"])
+            except:
+                continue
+            try:
+                json.dumps(key)
+                json.dumps(value)
+            except:
+                self.print_error('error: no json', key)
+                continue
+            result[key] = value
+
+        for key, value in result.items():
+            if force or not wallet.labels.get(key):
+                wallet.labels[key] = value
+
+        self.print_error("received %d labels" % len(response))
+        # do not write to disk because we're in a daemon thread
+        wallet.storage.put('labels', wallet.labels)
+        self.set_nonce(wallet, response["nonce"] + 1)
+        self.on_pulled(wallet)
+
+    def pull_thread_safe(self, wallet, force):
         try:
-            response = self.do_request("GET", ("/labels/since/%d/for/%s" % (nonce, wallet_id) ))
+            self.pull_thread(wallet, force)
-            if response["labels"] is None:
+        except BaseException as e:
-                self.print_error('no new labels')
+            # traceback.print_exc(file=sys.stderr)
-                return
+            self.print_error('could not retrieve labels')
-            result = {}
-            for label in response["labels"]:
-                try:
-                    key = self.decode(wallet, label["externalId"])
-                    value = self.decode(wallet, label["encryptedLabel"])
-                except:
-                    continue
-                try:
-                    json.dumps(key)
-                    json.dumps(value)
-                except:
-                    self.print_error('error: no json', key)
-                    continue
-                result[key] = value
-
-            for key, value in result.items():
-                if force or not wallet.labels.get(key):
-                    wallet.labels[key] = value
-
-            self.print_error("received %d labels" % len(response))
-            # do not write to disk because we're in a daemon thread
-            wallet.storage.put('labels', wallet.labels)
-            self.set_nonce(wallet, response["nonce"] + 1)
-            self.on_pulled(wallet)
-
-        except Exception as e:
-            traceback.print_exc(file=sys.stderr)
-            self.print_error("could not retrieve labels")
 
     def start_wallet(self, wallet):
         nonce = self.get_nonce(wallet)
@@ -144,7 +159,7 @@ class LabelsPlugin(BasePlugin):
         wallet_id = hashlib.sha256(mpk).hexdigest()
         self.wallets[wallet] = (password, iv, wallet_id)
         # If there is an auth token we can try to actually start syncing
-        t = threading.Thread(target=self.pull_thread, args=(wallet, False))
+        t = threading.Thread(target=self.pull_thread_safe, args=(wallet, False))
         t.setDaemon(True)
         t.start()
 

plugins/labels/qt.py

@@ -1,4 +1,6 @@
 from functools import partial
+import traceback
+import sys
 
 from PyQt5.QtGui import *
 from PyQt5.QtCore import *
@@ -37,10 +39,12 @@ class Plugin(LabelsPlugin):
         hbox.addWidget(QLabel("Label sync options:"))
         upload = ThreadedButton("Force upload",
                                 partial(self.push_thread, wallet),
-                                partial(self.done_processing, d))
+                                partial(self.done_processing_success, d),
+                                partial(self.done_processing_error, d))
         download = ThreadedButton("Force download",
                                   partial(self.pull_thread, wallet, True),
-                                  partial(self.done_processing, d))
+                                  partial(self.done_processing_success, d),
+                                  partial(self.done_processing_error, d))
         vbox = QVBoxLayout()
         vbox.addWidget(upload)
         vbox.addWidget(download)
@@ -54,13 +58,20 @@ class Plugin(LabelsPlugin):
     def on_pulled(self, wallet):
         self.obj.labels_changed_signal.emit(wallet)
 
-    def done_processing(self, dialog, result):
+    def done_processing_success(self, dialog, result):
         dialog.show_message(_("Your labels have been synchronised."))
 
+    def done_processing_error(self, dialog, result):
+        traceback.print_exception(*result, file=sys.stderr)
+        dialog.show_error(_("Error synchronising labels") + ':\n' + str(result[:2]))
+
     @hook
-    def on_new_window(self, window):
+    def load_wallet(self, wallet, window):
+        # FIXME if the user just enabled the plugin, this hook won't be called
+        # as the wallet is already loaded, and hence the plugin will be in
+        # a non-functional state for that window
         self.obj.labels_changed_signal.connect(window.update_tabs)
-        self.start_wallet(window.wallet)
+        self.start_wallet(wallet)
 
     @hook
     def on_close_window(self, window):