Merge pull request #5747 from drew2a/feature/5715

Better error handling

src/run_tribler.py

@@ -88,13 +88,18 @@ async def start_tribler():
     get_event_loop().run_forever()
 
 
+def excepthook(exctype, value, traceback): # pylint: disable=unused-argument
+    SentryReporter.send_exception_with_confirmation(value)
+
+
 if __name__ == "__main__":
     SentryReporter.init(sentry_url=sentry_url, scrubber=SentryScrubber())
     SentryReporter.allow_sending_globally(False, 'run_tribler.__main__()')
 
+    sys.excepthook = excepthook
+
     # Get root state directory (e.g. from environment variable or from system default)
     root_state_dir = get_root_state_directory()
-
     # Check whether we need to start the core or the user interface
     if 'CORE_PROCESS' in os.environ:
         # Check for missing Core dependencies

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

@@ -1,7 +1,10 @@
 import logging
+import sys
 from contextvars import ContextVar
 from hashlib import md5
 
+from PyQt5.QtWidgets import QApplication, QMessageBox
+
 from faker import Faker
 
 import sentry_sdk
@@ -83,8 +86,7 @@ def scrub_event(self, event):
         Returns:
             Sentry Guard.
         """
-        SentryReporter._logger.info(f"Init: {sentry_url}")
-
+        SentryReporter._logger.debug(f"Init: {sentry_url}")
         SentryReporter._scrubber = scrubber
         return sentry_sdk.init(
             sentry_url,
@@ -101,7 +103,7 @@ def scrub_event(self, event):
         )
 
     @staticmethod
-    def send(event, post_data, sys_info):
+    def send_event(event, post_data=None, sys_info=None):
         """Send the event to the Sentry server
 
         This method
@@ -165,6 +167,40 @@ def send(event, post_data, sys_info):
 
             return event
 
+    @staticmethod
+    def send_exception_with_confirmation(exception):
+        """Send exception with a confirmation dialog.
+        This method should be used in case standard GUI "sending reports mechanism"
+        is no available.
+
+        There are two message boxes, that will be triggered:
+        1. Message box with the error_text
+        2. Message box with confirmation about sending this report to the Tribler
+            team.
+
+        Args:
+            exception: exception to be sent.
+        """
+        SentryReporter._logger.debug(f"Send exception with confirmation: {exception}")
+
+        _ = QApplication(sys.argv)
+        messagebox = QMessageBox(icon=QMessageBox.Critical, text=f'{exception}.')
+        messagebox.setWindowTitle("Error")
+        messagebox.exec()
+
+        messagebox = QMessageBox(
+            icon=QMessageBox.Question,
+            text='Do you want to send this crash report to the Tribler team? '
+            'We anonymize all your data, who you are and what you downloaded.',
+        )
+        messagebox.setWindowTitle("Error")
+        messagebox.setStandardButtons(QMessageBox.Yes | QMessageBox.No)
+        if messagebox.exec() == QMessageBox.Yes:
+            with AllowSentryReports(True):
+                sentry_sdk.capture_exception(exception)
+
+        return exception
+
     @staticmethod
     def set_user(user_id):
         """ Set the user to identify the event on a Sentry server
@@ -184,7 +220,7 @@ def set_user(user_id):
         # calculate hash to keep real `user_id` in secret
         user_id_hash = md5(user_id).hexdigest()
 
-        SentryReporter._logger.info(f"Set user: {user_id_hash}")
+        SentryReporter._logger.debug(f"Set user: {user_id_hash}")
 
         Faker.seed(user_id_hash)
         user_name = Faker().name()
@@ -223,7 +259,7 @@ def allow_sending_globally(value, info=None):
         Returns:
             None
         """
-        SentryReporter._logger.info(f"Allow sending globally: {value}. Info: {info}")
+        SentryReporter._logger.debug(f"Allow sending globally: {value}. Info: {info}")
         SentryReporter._allow_sending_global = value
 
     @staticmethod
@@ -243,7 +279,7 @@ def allow_sending_in_thread(value, info=None):
         Returns:
             None
         """
-        SentryReporter._logger.info(f"Allow sending in thread: {value}. Info: {info}")
+        SentryReporter._logger.debug(f"Allow sending in thread: {value}. Info: {info}")
         SentryReporter._allow_sending_in_thread.set(value)
 
     @staticmethod
@@ -266,17 +302,17 @@ def _before_send(event, _):
         if not event:
             return event
 
-        SentryReporter._logger.info(f"Before send event: {event}")
-        SentryReporter._logger.info(f"Is allow sending: {SentryReporter._allow_sending_global}")
+        SentryReporter._logger.debug(f"Before send event: {event}")
+        SentryReporter._logger.debug(f"Is allow sending: {SentryReporter._allow_sending_global}")
         # to synchronise error reporter and sentry, we should suppress all events
         # until user clicked on "send crash report"
         if not SentryReporter.get_allow_sending():
-            SentryReporter._logger.info("Suppress sending. Storing the event.")
+            SentryReporter._logger.debug("Suppress sending. Storing the event.")
             SentryReporter.last_event = event
             return None
 
         # clean up the event
-        SentryReporter._logger.info(f"Clean up the event with scrubber: {SentryReporter._scrubber}")
+        SentryReporter._logger.debug(f"Clean up the event with scrubber: {SentryReporter._scrubber}")
         if SentryReporter._scrubber:
             event = SentryReporter._scrubber.scrub_event(event)
 
@@ -308,7 +344,7 @@ def __init__(self, value=True, description='', reporter=None):
                 testing purposes.
         """
         self._logger = logging.getLogger(self.__class__.__name__)
-        self._logger.info(f'Value: {value}, description: {description}')
+        self._logger.debug(f'Value: {value}, description: {description}')
 
         self._value = value
         self._saved_state = None
@@ -317,7 +353,7 @@ def __init__(self, value=True, description='', reporter=None):
     def __enter__(self):
         """Set SentryReporter.allow_sending(value)
         """
-        self._logger.info('Enter')
+        self._logger.debug('Enter')
         self._saved_state = self._reporter.get_allow_sending()
 
         self._reporter.allow_sending_in_thread(self._value, 'AllowSentryReports.__enter__()')
@@ -326,5 +362,5 @@ def __enter__(self):
     def __exit__(self, exc_type, exc_val, exc_tb):
         """Restore SentryReporter.allow_sending(old_value)
         """
-        self._logger.info('Exit')
+        self._logger.debug('Exit')
         self._reporter.allow_sending_in_thread(self._saved_state, 'AllowSentryReports.__exit__()')

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

@@ -110,10 +110,10 @@ def test_allow_sentry_reports(reporter):
 
 
 def test_send(reporter):
-    assert reporter.send(None, None, None) is None
+    assert reporter.send_event(None, None, None) is None
 
     # test return defaults
-    assert reporter.send({}, None, None) == {
+    assert reporter.send_event({}, None, None) == {
         'contexts': {
             'browser': {'name': 'Tribler', 'version': None},
             'reporter': {'_stacktrace': [], 'comments': None, OS_ENVIRON: {}, 'sysinfo': None},
@@ -132,7 +132,7 @@ def test_send(reporter):
         "stack": 'some\nstack',
     }
 
-    assert reporter.send({'a': 'b'}, post_data, None) == {
+    assert reporter.send_event({'a': 'b'}, post_data, None) == {
         'a': 'b',
         'contexts': {
             'browser': {'name': 'Tribler', 'version': '0.0.0'},
@@ -144,7 +144,7 @@ def test_send(reporter):
     # test sys_info
     post_data = {"sysinfo": 'key\tvalue\nkey1\tvalue1\n'}
 
-    assert reporter.send({}, post_data, None) == {
+    assert reporter.send_event({}, post_data, None) == {
         'contexts': {
             'browser': {'name': 'Tribler', 'version': None},
             'reporter': {'_stacktrace': [], 'comments': None, 'os.environ': {}, 'sysinfo': None},
@@ -153,7 +153,7 @@ def test_send(reporter):
     }
 
     sys_info = {'platform': ['darwin'], 'platform.details': ['details'], OS_ENVIRON: ['KEY:VALUE', 'KEY1:VALUE1']}
-    assert reporter.send({}, None, sys_info) == {
+    assert reporter.send_event({}, None, sys_info) == {
         'contexts': {
             'browser': {'name': 'Tribler', 'version': None},
             'reporter': {

src/tribler-core/tribler_core/session.py

@@ -219,48 +219,51 @@ def unhandled_error_observer(self, loop, context):
         This method is called when an unhandled error in Tribler is observed.
         It broadcasts the tribler_exception event.
         """
-        exception = context.get('exception')
-        ignored_message = None
         try:
-            ignored_message = IGNORED_ERRORS.get(
-                (exception.__class__, exception.errno),
-                IGNORED_ERRORS.get(exception.__class__))
-        except (ValueError, AttributeError):
-            pass
-        if ignored_message is not None:
-            self._logger.error(ignored_message if ignored_message != "" else context.get('message'))
-            return
-
-        text = str(exception or context.get('message'))
-
-        # We already have a check for invalid infohash when adding a torrent, but if somehow we get this
-        # error then we simply log and ignore it.
-        if isinstance(exception, RuntimeError) and 'invalid info-hash' in text:
-            self._logger.error("Invalid info-hash found")
-            return
-
-        text_long = text
-        exc = context.get('exception')
-        if exc:
-            with StringIO() as buffer:
-                print_exception(type(exc), exc, exc.__traceback__, file=buffer)
-                text_long = text_long + "\n--LONG TEXT--\n" + buffer.getvalue()
-        text_long = text_long + "\n--CONTEXT--\n" + str(context)
-
-        description = 'session.unhandled_error_observer()'
-        with AllowSentryReports(value=False, description=description):
-            self._logger.error("Unhandled exception occurred! %s", text_long)
-
-        sentry_event = SentryReporter.last_event
-
-        if not self.api_manager:
-            return
-
-        events = self.api_manager.get_endpoint('events')
-        events.on_tribler_exception(text_long, sentry_event)
-
-        state = self.api_manager.get_endpoint('state')
-        state.on_tribler_exception(text_long, sentry_event)
+            exception = context.get('exception')
+            ignored_message = None
+            try:
+                ignored_message = IGNORED_ERRORS.get(
+                    (exception.__class__, exception.errno),
+                    IGNORED_ERRORS.get(exception.__class__))
+            except (ValueError, AttributeError):
+                pass
+            if ignored_message is not None:
+                self._logger.error(ignored_message if ignored_message != "" else context.get('message'))
+                return
+
+            text = str(exception or context.get('message'))
+
+            # We already have a check for invalid infohash when adding a torrent, but if somehow we get this
+            # error then we simply log and ignore it.
+            if isinstance(exception, RuntimeError) and 'invalid info-hash' in text:
+                self._logger.error("Invalid info-hash found")
+                return
+
+            text_long = text
+            exc = context.get('exception')
+            if exc:
+                with StringIO() as buffer:
+                    print_exception(type(exc), exc, exc.__traceback__, file=buffer)
+                    text_long = text_long + "\n--LONG TEXT--\n" + buffer.getvalue()
+            text_long = text_long + "\n--CONTEXT--\n" + str(context)
+
+            description = 'session.unhandled_error_observer()'
+            with AllowSentryReports(value=False, description=description):
+                self._logger.error("Unhandled exception occurred! %s", text_long)
+            sentry_event = SentryReporter.last_event
+
+            if not self.api_manager:
+                return
+
+            events = self.api_manager.get_endpoint('events')
+            events.on_tribler_exception(text_long, sentry_event)
+
+            state = self.api_manager.get_endpoint('state')
+            state.on_tribler_exception(text_long, sentry_event)
+        except Exception as e:
+            SentryReporter.send_exception_with_confirmation(e)
+            raise e
 
     def get_tribler_statistics(self):
         """Return a dictionary with general Tribler statistics."""