Merge pull request #5821 from drew2a/senty/improve_gui

Improve GUI error handling

src/tribler-common/tribler_common/sentry_reporter/sentry_mixin.py

@@ -0,0 +1,12 @@
+from tribler_common.sentry_reporter.sentry_reporter import SentryReporter
+
+
+class AddBreadcrumbOnShowMixin:
+    """This class has been designed for extending QWidget and QDialog instances
+    and send breadcrumbs on a show event.
+    """
+
+    def showEvent(self, *args):
+        super().showEvent(*args)
+
+        SentryReporter.add_breadcrumb(message=f'{self.__class__.__name__}.Show', category='UI', level='info')

src/tribler-common/tribler_common/sentry_reporter/sentry_reporter.py

@@ -31,6 +31,7 @@
 BREADCRUMBS = 'breadcrumbs'
 LOGENTRY = 'logentry'
 REPORTER = 'reporter'
+VALUES = 'values'
 
 
 class SentryReporter:
@@ -110,6 +111,22 @@ def ignore_logger(logger_name):
         SentryReporter._logger.debug(f"Ignore logger: {logger_name}")
         ignore_logger(logger_name)
 
+    @staticmethod
+    def add_breadcrumb(message='', category='', level='info', **kwargs):
+        """ Adds a breadcrumb for current Sentry client.
+
+        It is necessary to specify a message, a category and a level to make this
+        breadcrumb visible in Sentry server.
+
+        Args:
+            **kwargs: named arguments that will be added to Sentry event as well
+        """
+        crumb = {'message': message, 'category': category, 'level': level}
+
+        SentryReporter._logger.debug(f"Add the breadcrumb: {crumb}")
+
+        return sentry_sdk.add_breadcrumb(crumb, **kwargs)
+
     @staticmethod
     def send_event(event, post_data=None, sys_info=None):
         """Send the event to the Sentry server

src/tribler-common/tribler_common/sentry_reporter/sentry_scrubber.py

@@ -9,12 +9,13 @@
     REPORTER,
     STACKTRACE,
     SYSINFO,
+    VALUES,
 )
-from tribler_common.sentry_reporter.sentry_tools import delete_item, modify_value
+from tribler_common.sentry_reporter.sentry_tools import delete_item, distinct_by, modify_value
 
 
 class SentryScrubber:
-    """ This class has been created to be responsible for scrubbing all sensitive
+    """This class has been created to be responsible for scrubbing all sensitive
     and unnecessary information from Sentry event.
     """
 
@@ -31,7 +32,7 @@ def __init__(self):
             'Documents and Settings',
         ]
 
-        self.event_fields_to_cut = ['modules']
+        self.event_fields_to_cut = []
 
         self.placeholder_user = '<user>'
         self.placeholder_ip = '<IP>'
@@ -48,8 +49,7 @@ def __init__(self):
         self._compile_re()
 
     def _compile_re(self):
-        """ Compile all regular expressions.
-        """
+        """Compile all regular expressions."""
         for folder in self.home_folders:
             folder_pattern = r'(?<=' + folder + r'[/\\])\w+(?=[/\\])'
             self.re_folders.append(re.compile(folder_pattern, re.I))
@@ -58,7 +58,7 @@ def _compile_re(self):
         self.re_hash = re.compile(r'\b[0-9a-f]{40}\b', re.I)
 
     def scrub_event(self, event):
-        """ Main method. Removes all sensitive and unnecessary information.
+        """Main method. Removes all sensitive and unnecessary information.
 
         Args:
             event: a Sentry event.
@@ -69,9 +69,18 @@ def scrub_event(self, event):
         if not event:
             return event
 
+        # remove unnecessary fields
         for field_name in self.event_fields_to_cut:
             delete_item(event, field_name)
 
+        # remove duplicates from breadcrumbs
+        # duplicates will be identifiers by the `timestamp` field
+        def _remove_duplicates_from_breadcrumbs(breadcrumbs):
+            return modify_value(breadcrumbs, VALUES, lambda values: distinct_by(values, 'timestamp'))
+
+        modify_value(event, BREADCRUMBS, _remove_duplicates_from_breadcrumbs)
+
+        # remove sensitive information
         modify_value(event, EXTRA, self.scrub_entity_recursively)
         modify_value(event, LOGENTRY, self.scrub_entity_recursively)
         modify_value(event, BREADCRUMBS, self.scrub_entity_recursively)
@@ -87,7 +96,7 @@ def scrub_event(self, event):
         return event
 
     def scrub_text(self, text):
-        """ Replace all sensitive information from `text` by corresponding
+        """Replace all sensitive information from `text` by corresponding
         placeholders.
 
         Sensitive information:

src/tribler-common/tribler_common/sentry_reporter/sentry_tools.py

@@ -75,3 +75,38 @@ def modify_value(d, key, function):
         d[key] = function(d[key])
 
     return d
+
+
+def distinct_by(list_of_dict, key):
+    """ This function removes all duplicates from a list of dictionaries. A duplicate
+    here is a dictionary that have the same value of the given key.
+
+    If no key field is presented in the item, then the item will not be considered
+    as a duplicate.
+
+    Args:
+        list_of_dict: list of dictionaries
+        key: a field key that will be used for items comparison
+
+    Returns:
+        Array of distinct items
+    """
+
+    if not list_of_dict or not key:
+        return list_of_dict
+
+    values_viewed = set()
+    result = []
+
+    for item in list_of_dict:
+        value = get_value(item, key, None)
+        if value is None:
+            result.append(item)
+            continue
+
+        if value not in values_viewed:
+            result.append(item)
+
+        values_viewed.add(value)
+
+    return result

src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_reporter.py

@@ -144,3 +144,10 @@ def test_event_from_exception(reporter):
     # sentry sdk is not initialised, so None will be returned
     SentryReporter.last_event = None
     assert not reporter.event_from_exception(Exception('test'))
+
+
+def test_add_breadcrumb(reporter):
+    # test: None does not produce error
+    assert reporter.add_breadcrumb(None, None, None) is None
+    assert reporter.add_breadcrumb('message', 'category', 'level') is None
+    assert reporter.add_breadcrumb('message', 'category', 'level', named_arg='some') is None

src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_scrubber.py

@@ -150,8 +150,9 @@ def test_scrub_event(scrubber):
         },
         BREADCRUMBS: {
             'values': [
-                {'type': 'log', 'message': 'Traceback File: /Users/username/Tribler/'},
-                {'type': 'log', 'message': 'IP: 192.168.1.1'}
+                {'type': 'log', 'message': 'Traceback File: /Users/username/Tribler/', 'timestamp': '1'},
+                {'type': 'log', 'message': 'Traceback File: /Users/username/Tribler/', 'timestamp': '1'},
+                {'type': 'log', 'message': 'IP: 192.168.1.1', 'timestamp': '2'}
             ]
         },
 
@@ -180,9 +181,11 @@ def test_scrub_event(scrubber):
         BREADCRUMBS: {
             'values': [
                 {'type': 'log',
-                 'message': f'Traceback File: /Users/{scrubber.placeholder_user}/Tribler/'},
+                 'message': f'Traceback File: /Users/{scrubber.placeholder_user}/Tribler/',
+                 'timestamp': '1'},
                 {'type': 'log',
-                 'message': f'IP: {scrubber.placeholder_ip}'}
+                 'message': f'IP: {scrubber.placeholder_ip}',
+                 'timestamp': '2'}
             ]
         },
     }
@@ -208,7 +211,6 @@ def test_entities_recursively(scrubber):
 def test_scrub_unnecessary_fields(scrubber):
     # default
     assert scrubber.scrub_event({'default': 'field'}) == {'default': 'field'}
-    assert scrubber.scrub_event({'modules': {}}) == {}
 
     # custom
     custom_scrubber = SentryScrubber()

src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_tools.py

@@ -1,5 +1,6 @@
 from tribler_common.sentry_reporter.sentry_tools import (
     delete_item,
+    distinct_by,
     get_first_item,
     get_last_item,
     get_value,
@@ -86,3 +87,13 @@ def test_safe_get():
 
     assert get_value({'key': 'value'}, 'key', {}) == 'value'
     assert get_value({'key': 'value'}, 'key1', {}) == {}
+
+
+def test_distinct():
+    assert distinct_by(None, None) is None
+    assert distinct_by([], None) == []
+    assert distinct_by([{'key': 'b'}, {'key': 'b'}, {'key': 'c'}, {'': ''}], 'key') == \
+           [{'key': 'b'}, {'key': 'c'}, {'': ''}]
+
+    # test nested
+    assert distinct_by([{'a': {}}], 'b') == [{'a': {}}]

src/tribler-core/tribler_core/restapi/events_endpoint.py

@@ -45,7 +45,7 @@ def passthrough(x):
     # Remote GigaChannel search results were received by Tribler. Contains received entries.
     NTFY.REMOTE_QUERY_RESULTS: passthrough,
     # An indicator that Tribler has completed the startup procedure and is ready to use.
-    NTFY.TRIBLER_STARTED: lambda *_: {"version": version_id},
+    NTFY.TRIBLER_STARTED: lambda public_key: {"version": version_id, "public_key": hexlify(public_key)},
     # Tribler is low on disk space for storing torrents
     NTFY.LOW_SPACE: passthrough,
 }
@@ -67,7 +67,7 @@ def __init__(self, session):
 
         # We need to know that Tribler completed its startup sequence
         self.tribler_started = False
-        self.session.notifier.add_observer(NTFY.TRIBLER_STARTED, self._tribler_started)
+        self.session.notifier.add_observer(NTFY.TRIBLER_STARTED, self.on_tribler_started)
 
         for event_type, event_lambda in reactions_dict.items():
             self.session.notifier.add_observer(event_type,
@@ -90,7 +90,7 @@ def on_circuit_removed(circuit, *args):
     async def on_shutdown(self, _):
         await self.shutdown_task_manager()
 
-    def _tribler_started(self):
+    def on_tribler_started(self, _):
         self.tribler_started = True
 
     def setup_routes(self):

src/tribler-core/tribler_core/session.py

@@ -53,6 +53,7 @@
 from tribler_core.upgrade.upgrade import TriblerUpgrader
 from tribler_core.utilities.crypto_patcher import patch_crypto_be_discovery
 from tribler_core.utilities.install_dir import get_lib_path
+from tribler_core.utilities.unicode import hexlify
 
 if sys.platform == 'win32':
     SOCKET_BLOCK_ERRORCODE = 10035  # WSAEWOULDBLOCK
@@ -280,7 +281,11 @@ async def start(self):
         self.get_ports_in_config()
         self.create_state_directory_structure()
         self.init_keypair()
-        SentryReporter.set_user(self.trustchain_keypair.key.pk)
+
+        # we have to represent `user_id` as a string to make it equal to the
+        # `user_id` on the GUI side
+        user_id_str = hexlify(self.trustchain_keypair.key.pk).encode('utf-8')
+        SentryReporter.set_user(user_id_str)
 
         # Start the REST API before the upgrader since we want to send interesting upgrader events over the socket
         if self.config.get_api_http_enabled() or self.config.get_api_https_enabled():
@@ -406,7 +411,7 @@ async def start(self):
         if self.config.get_bootstrap_enabled() and not self.core_test_mode:
             self.register_task('bootstrap_download', self.start_bootstrap_download)
 
-        self.notifier.notify(NTFY.TRIBLER_STARTED)
+        self.notifier.notify(NTFY.TRIBLER_STARTED, self.trustchain_keypair.key.pk)
 
     async def shutdown(self):
         """

src/tribler-gui/tribler_gui/dialogs/dialogcontainer.py

@@ -2,13 +2,14 @@
 from PyQt5.QtGui import QPainter
 from PyQt5.QtWidgets import QStyle, QStyleOption, QWidget
 
+from tribler_common.sentry_reporter.sentry_mixin import AddBreadcrumbOnShowMixin
+
 from tribler_gui.utilities import connect
 
 
-class DialogContainer(QWidget):
+class DialogContainer(AddBreadcrumbOnShowMixin, QWidget):
     def __init__(self, parent):
         QWidget.__init__(self, parent)
-
         self.setStyleSheet("background-color: rgba(30, 30, 30, 0.75);")
 
         self.dialog_widget = QWidget(self)

src/tribler-gui/tribler_gui/dialogs/feedbackdialog.py

@@ -8,6 +8,7 @@
 from PyQt5 import uic
 from PyQt5.QtWidgets import QAction, QApplication, QDialog, QMessageBox, QTreeWidgetItem
 
+from tribler_common.sentry_reporter.sentry_mixin import AddBreadcrumbOnShowMixin
 from tribler_common.sentry_reporter.sentry_reporter import SentryReporter
 
 from tribler_gui.event_request_manager import received_events
@@ -20,7 +21,7 @@
 from tribler_gui.utilities import connect, get_ui_file_path
 
 
-class FeedbackDialog(QDialog):
+class FeedbackDialog(AddBreadcrumbOnShowMixin, QDialog):
     def __init__(self, parent, exception_text, tribler_version, start_time, sentry_event=None):  # pylint: disable=R0914
         QDialog.__init__(self, parent)
 

src/tribler-gui/tribler_gui/event_request_manager.py

@@ -4,6 +4,7 @@
 from PyQt5.QtCore import QTimer, QUrl, pyqtSignal
 from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkRequest
 
+from tribler_common.sentry_reporter.sentry_reporter import SentryReporter
 from tribler_common.simpledefs import NTFY
 
 import tribler_core.utilities.json_util as json
@@ -27,7 +28,7 @@ class EventRequestManager(QNetworkAccessManager):
 
     node_info_updated = pyqtSignal(object)
     received_remote_query_results = pyqtSignal(object)
-    tribler_started = pyqtSignal(object)
+    tribler_started = pyqtSignal(object, str)  # arguments are version and public_key
     upgrader_tick = pyqtSignal(str)
     upgrader_finished = pyqtSignal()
     new_version_available = pyqtSignal(str)
@@ -58,16 +59,17 @@ def __init__(self, api_port, api_key):
             NTFY.LOW_SPACE.value: self.low_storage_signal.emit,
             NTFY.REMOTE_QUERY_RESULTS.value: self.received_remote_query_results.emit,
             NTFY.TRIBLER_SHUTDOWN_STATE.value: self.tribler_shutdown_signal.emit,
-            NTFY.EVENTS_START.value: self.events_start_received,
-            NTFY.TRIBLER_STARTED.value: lambda data: self.tribler_started.emit(data["version"]),
+            NTFY.EVENTS_START.value: lambda _: None,
+            NTFY.TRIBLER_STARTED.value: self.tribler_started_event,
         }
 
-    def events_start_received(self, event_dict):
-        if event_dict["tribler_started"]:
-            self.tribler_started.emit(event_dict["version"])
+    def tribler_started_event(self, event_dict):
+        self.tribler_started.emit(event_dict["version"], event_dict["public_key"])
 
     def on_error(self, error, reschedule_on_err):
         self._logger.info("Got Tribler core error: %s" % error)
+
+        SentryReporter.ignore_logger(self._logger.name)
         if self.remaining_connection_attempts <= 0:
             raise CoreConnectTimeoutError("Could not connect with the Tribler Core within 60 seconds")
 

src/tribler-gui/tribler_gui/tribler_window.py

@@ -443,10 +443,16 @@ def tray_show_message(self, title, message):
             except RuntimeError as e:
                 logging.error("Failed to set tray message: %s", str(e))
 
-    def on_tribler_started(self, version):
+    def on_tribler_started(self, version, public_key):
         if self.tribler_started:
             logging.warning("Received duplicate Tribler Core started event")
             return
+
+        if public_key:
+            # if public key format will be changed, don't forget to change it
+            # at the core side as well
+            SentryReporter.set_user(public_key.encode('utf-8'))
+
         self.tribler_started = True
         self.tribler_version = version
 

src/tribler-gui/tribler_gui/widgets/channelcontentswidget.py

@@ -6,6 +6,7 @@
 from PyQt5.QtGui import QIcon
 from PyQt5.QtWidgets import QAction, QFileDialog
 
+from tribler_common.sentry_reporter.sentry_mixin import AddBreadcrumbOnShowMixin
 from tribler_common.simpledefs import CHANNEL_STATE
 
 from tribler_core.modules.metadata_store.orm_bindings.channel_node import DIRTY_STATUSES, NEW
@@ -31,8 +32,8 @@
 
 widget_form, widget_class = uic.loadUiType(get_ui_file_path('torrents_list.ui'))
 
-
-class ChannelContentsWidget(widget_form, widget_class):
+# pylint: disable=too-many-instance-attributes, too-many-public-methods
+class ChannelContentsWidget(AddBreadcrumbOnShowMixin, widget_form, widget_class):
     def __init__(self, parent=None):
         super(widget_class, self).__init__(parent=parent)
         # FIXME!!! This is a dumb workaround for a bug(?) in PyQT bindings in Python 3.7