PR fixes

src/README.md

@@ -25,6 +25,10 @@ Export to PYTHONPATH the following directories:
 * tribler-core
 * tribler-gui
 
+Shortcut for macOS:
+```shell script
+export PYTHONPATH=${PYTHONPATH}:`echo {pyipv8,tribler-common,tribler-core,tribler-gui} | tr " " :`
+```
 Execute:
 ```
 python3 -m pytest tribler-core

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

@@ -6,7 +6,7 @@
 import sentry_sdk
 from sentry_sdk.integrations.logging import LoggingIntegration
 
-from tribler_common.sentry_reporter.sentry_tools import first, parse_os_environ, parse_stacktrace, safe_delete
+from tribler_common.sentry_reporter.sentry_tools import first, parse_os_environ, parse_stacktrace, safe_delete, safe_get
 
 STACKTRACE = '_stacktrace'
 SYSINFO = 'sysinfo'
@@ -109,12 +109,11 @@ def scrub_event(self, event):
                     event_level=logging.ERROR  # Send errors as events
                 ),
             ],
-            # attach_stacktrace=True,
             before_send=SentryReporter._before_send,
         )
 
     @staticmethod
-    def send(post_data, event, sys_info):
+    def send(event, post_data, sys_info):
         """Send the event to the Sentry server
 
         This method
@@ -129,57 +128,58 @@ def send(post_data, event, sys_info):
         will be raised, will be sent to Sentry automatically.
 
         Args:
-            post_data: dictionary made by the feedbackdialog.py
             event: event to send. It should be taken from SentryReporter at
+            post_data: dictionary made by the feedbackdialog.py
                 previous stages of executing.
             sys_info: dictionary made by the feedbackdialog.py
 
         Returns:
-            None
+            Event that was sent to Sentry server
         """
         SentryReporter._logger.info(f"Send: {post_data}, {event}")
 
-        if not post_data or not event:
-            return
+        if event is None:
+            return event
 
         with AllowSentryReports(value=True, description='SentryReporter.send()'):
-            try:
-                # prepare event
-                if CONTEXTS not in event:
-                    event[CONTEXTS] = {}
+            # prepare event
+            if CONTEXTS not in event:
+                event[CONTEXTS] = {}
+
+            if TAGS not in event:
+                event[TAGS] = {}
 
-                if TAGS not in event:
-                    event[TAGS] = {}
+            event[CONTEXTS][REPORTER] = {}
 
-                event[CONTEXTS][REPORTER] = {}
+            # tags
+            tags = event[TAGS]
+            tags['version'] = safe_get(post_data, 'version', None)
+            tags['machine'] = safe_get(post_data, 'machine', None)
+            tags['os'] = safe_get(post_data, 'os', None)
+            tags['platform'] = first(safe_get(sys_info, 'platform'
+                                              , None), None)
+            tags['platform.details'] = first(safe_get(sys_info, 'platform.details',
+                                                      None), None)
 
-                # set tags
-                tags = event[TAGS]
+            # context
+            context = event[CONTEXTS]
+            reporter = context[REPORTER]
+            version = post_data.get('version', None) if post_data else None
 
-                tags['version'] = post_data['version']
-                tags['machine'] = post_data['machine']
-                tags['os'] = first(sys_info['os'])
-                tags['platform'] = first(sys_info['platform'])
-                tags['platform.details'] = first(['platform.details'])
+            context['browser'] = {
+                'version': version,
+                'name': 'Tribler'}
 
-                # set context
-                context = event[CONTEXTS]
-                context['browser'] = {
-                    'version': post_data['version'],
-                    'name': 'Tribler'}
+            reporter[STACKTRACE] = parse_stacktrace(safe_get(post_data, 'stack', None))
+            reporter['comments'] = safe_get(post_data, 'comments', None)
 
-                reporter = context[REPORTER]
-                reporter[OS_ENVIRON] = parse_os_environ(sys_info[OS_ENVIRON])
-                safe_delete(sys_info, OS_ENVIRON)
-                reporter[SYSINFO] = sys_info
+            reporter[OS_ENVIRON] = parse_os_environ(safe_get(sys_info, OS_ENVIRON, None))
+            safe_delete(sys_info, OS_ENVIRON)
+            reporter[SYSINFO] = sys_info
 
-                reporter[STACKTRACE] = parse_stacktrace(post_data['stack'])
-                reporter['comments'] = post_data['comments']
+            sentry_sdk.capture_event(event)
 
-                sentry_sdk.capture_event(event)
-            except Exception as e:
-                SentryReporter._logger.exception(e)
-                sentry_sdk.capture_exception(e)
+            return event
 
     @staticmethod
     def set_user(user_id):
@@ -238,7 +238,7 @@ def allow_sending(value, info=None):
         SentryReporter._allow_sending = value
 
     @staticmethod
-    def _before_send(event, hint):
+    def _before_send(event, _):
         """The method that is called before each send. Both allowed and
         disallowed.
 
@@ -263,7 +263,7 @@ def _before_send(event, hint):
         # until user clicked on "send crash report"
         if not SentryReporter._allow_sending:
             SentryReporter._logger.info(
-                f"Suppress sending. Storing the event.")
+                "Suppress sending. Storing the event.")
             SentryReporter.last_event = event
             return None
 
@@ -306,7 +306,7 @@ def __init__(self, value=True, description='', reporter=None):
     def __enter__(self):
         """Set SentryReporter.allow_sending(value)
         """
-        self._logger.info(f'Enter')
+        self._logger.info('Enter')
         self._saved_state = self._reporter.get_allow_sending()
 
         self._reporter.allow_sending(self._value, 'AllowSentryReports.__enter__()')
@@ -315,5 +315,5 @@ def __enter__(self):
     def __exit__(self, exc_type, exc_val, exc_tb):
         """Restore SentryReporter.allow_sending(old_value)
         """
-        self._logger.info(f'Exit')
+        self._logger.info('Exit')
         self._reporter.allow_sending(self._saved_state, 'AllowSentryReports.__exit__()')

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

@@ -91,6 +91,20 @@ def safe_delete(d, key):
     return d
 
 
+def safe_get(d, key, default=None):
+    """ Safe get value from dictionary
+
+    Args:
+        d: a dictionary
+        key: a key
+        default: the value that will be returned in case of 'key' is unreached
+
+    Returns:
+        Value addressed by key or `default`
+    """
+    return d.get(key, default) if d else default
+
+
 def modify(d, key, function):
     """ Safe modify value in a dictionary.
 

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

@@ -1,23 +1,24 @@
 import pytest
 
-from tribler_common.sentry_reporter.sentry_reporter import AllowSentryReports, SentryReporter
+from tribler_common.sentry_reporter.sentry_reporter import AllowSentryReports, OS_ENVIRON, SentryReporter
+from tribler_common.sentry_reporter.sentry_scrubber import SentryScrubber
 
 
-@pytest.fixture
-def reporter():
+@pytest.fixture(name="reporter")  # this workaround implemented only for pylint
+def fixture_reporter():
     return SentryReporter()
 
 
-@pytest.fixture
-def mock_reporter():
+@pytest.fixture(name="mock_reporter")  # this workaround implemented only for pylint
+def fixture_mock_reporter():
     class MockReporter:
         def __init__(self):
             self._allow_sending = None
 
         def get_allow_sending(self):
             return self._allow_sending
 
-        def allow_sending(self, value, info=None):
+        def allow_sending(self, value, _=None):
             self._allow_sending = value
 
     return MockReporter()
@@ -58,11 +59,6 @@ def test_set_user(reporter):
             'username': 'Jennifer Herrera'}
 
 
-def test_before_send(reporter):
-    assert reporter._before_send({}, {}) == {}
-    assert reporter._before_send(None, {}) is None
-
-
 def test_allow_sentry_reports(mock_reporter):
     # test base logic
     mock_reporter.allow_sending(None)
@@ -86,3 +82,107 @@ def test_allow_sentry_reports(mock_reporter):
     # test default initialisation
     with AllowSentryReports() as reporter:
         assert reporter
+
+
+def test_send(reporter):
+    assert reporter.send(None, None, None) is None
+
+    # test return defaults
+    assert reporter.send({}, None, None) == {
+        'contexts': {'browser': {'name': 'Tribler', 'version': None},
+                     'reporter': {'_stacktrace': [],
+                                  'comments': None,
+                                  'os.environ': {},
+                                  'sysinfo': None}},
+        'tags': {'machine': None,
+                 'os': None,
+                 'platform': None,
+                 'platform.details': None,
+                 'version': None}}
+
+    # test post_data
+    post_data = {
+        "version": '0.0.0',
+        "machine": 'x86_64',
+        "os": 'posix',
+        "timestamp": 42,
+        "sysinfo": '',
+        "comments": 'comment',
+        "stack": 'some\nstack',
+    }
+
+    assert reporter.send({'a': 'b'}, post_data, None) == {
+        'a': 'b',
+        'contexts': {'browser': {'name': 'Tribler', 'version': '0.0.0'},
+                     'reporter': {'_stacktrace': ['some', 'stack'],
+                                  'comments': 'comment',
+                                  'os.environ': {},
+                                  'sysinfo': None}},
+        'tags': {'machine': 'x86_64',
+                 'os': 'posix',
+                 'platform': None,
+                 'platform.details': None,
+                 'version': '0.0.0'}}
+
+    # test sys_info
+    post_data = {
+        "sysinfo": 'key\tvalue\nkey1\tvalue1\n',
+    }
+
+    assert reporter.send({}, post_data, None) == {
+        'contexts': {'browser': {'name': 'Tribler', 'version': None},
+                     'reporter': {'_stacktrace': [],
+                                  'comments': None,
+                                  'os.environ': {},
+                                  'sysinfo': None}},
+        'tags': {'machine': None,
+                 'os': None,
+                 'platform': None,
+                 'platform.details': None,
+                 'version': None}}
+
+    sys_info = {'platform': ['darwin'], 'platform.details': ['details'],
+                OS_ENVIRON: ['KEY:VALUE', 'KEY1:VALUE1']}
+    assert reporter.send({}, None, sys_info) == {
+        'contexts': {'browser': {'name': 'Tribler', 'version': None},
+                     'reporter': {'_stacktrace': [],
+                                  'comments': None,
+                                  'os.environ': {'KEY': 'VALUE',
+                                                 'KEY1': 'VALUE1'},
+                                  'sysinfo': sys_info}},
+        'tags': {'machine': None,
+                 'os': None,
+                 'platform': 'darwin',
+                 'platform.details': 'details',
+                 'version': None}}
+
+
+def test_before_send(reporter):
+    scrubber = SentryScrubber()
+    reporter.init('', scrubber)
+
+    # pylint: disable=protected-access
+
+    assert reporter._before_send({}, {}) == {}
+    assert reporter._before_send(None, {}) is None
+    assert reporter._before_send(None, None) is None
+
+    reporter.allow_sending(False)
+    assert reporter.last_event is None
+    assert not reporter.get_allow_sending()
+
+    # check that an event is stored
+    assert reporter._before_send({'a': 'b'}, None) is None
+    assert reporter.last_event == {'a': 'b'}
+
+    # check an event has been processed
+    reporter.allow_sending(True)
+    assert reporter.get_allow_sending()
+    assert reporter._before_send({'c': 'd'}, None) == {'c': 'd'}
+    assert reporter.last_event == {'a': 'b'}
+
+    # check information has been scrubbed
+    assert reporter._before_send({'contexts': {
+        'reporter': {'_stacktrace': ['/Users/username/']}}}, None) == \
+           {'contexts': {
+               'reporter': {'_stacktrace': [f'/Users/{scrubber.placeholder_user}/']}}}

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

@@ -8,8 +8,8 @@
 from tribler_common.sentry_reporter.sentry_scrubber import SentryScrubber
 
 
-@pytest.fixture
-def scrubber():
+@pytest.fixture(name="scrubber")  # this workaround implemented only for pylint
+def fixture_scrubber():
     return SentryScrubber()
 
 
@@ -161,7 +161,7 @@ def test_scrub_event(scrubber):
         CONTEXTS: {
             REPORTER: {
                 OS_ENVIRON: {
-                    f'PATH': f'/users/{scrubber.placeholder_user}/apps'
+                    'PATH': f'/users/{scrubber.placeholder_user}/apps'
                 },
                 STACKTRACE: ['Traceback (most recent call last):',
                              f'File "/Users/{scrubber.placeholder_user}/Tribler/tribler/src/tribler-gui/tribler_gui/"'],

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

@@ -1,5 +1,5 @@
 from tribler_common.sentry_reporter.sentry_tools import first, last, modify, parse_os_environ, parse_stacktrace, \
-    safe_delete
+    safe_delete, safe_get
 
 
 def test_first():
@@ -66,3 +66,13 @@ def test_modify():
     assert modify({}, 'key', lambda value: '') == {}
     assert modify({'a': 'b'}, 'key', lambda value: '') == {'a': 'b'}
     assert modify({'a': 'b', 'key': 'value'}, 'key', lambda value: '') == {'a': 'b', 'key': ''}
+
+
+def test_safe_get():
+    assert safe_get(None, None, None) is None
+    assert safe_get(None, None, {}) == {}
+
+    assert safe_get(None, 'key', {}) == {}
+
+    assert safe_get({'key': 'value'}, 'key', {}) == 'value'
+    assert safe_get({'key': 'value'}, 'key1', {}) == {}

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

@@ -18,7 +18,7 @@
 
 
 class FeedbackDialog(QDialog):
-    def __init__(self, parent, exception_text, tribler_version, start_time,
+    def __init__(self, parent, exception_text, tribler_version, start_time,  # pylint: disable=R0914
                  backend_sentry_event=None):
         QDialog.__init__(self, parent)
 
@@ -152,7 +152,7 @@ def on_send_clicked(self):
 
         # if no backend_sentry_event, then try to restore GUI exception
         sentry_event = self.backend_sentry_event or SentryReporter.last_event
-        SentryReporter.send(post_data, sentry_event, sys_info_dict)
+        SentryReporter.send(sentry_event, post_data, sys_info_dict)
 
         TriblerNetworkRequest(endpoint, self.on_report_sent, raw_data=tribler_urlencode(post_data), method='POST')
 

src/tribler-gui/tribler_gui/tribler_window.py

@@ -108,7 +108,7 @@ def on_exception(self, *exc_info):
         # Enable "allow sending" because if error will be raised in 'on_exception`
         # method, then no error will be sent at all.
         SentryReporter.allow_sending(True, 'in TriblerWindow.on_exception()')
-        info_type, info_error, info_traceback = exc_info
+        info_type, info_error, _ = exc_info
         exception_text = "".join(traceback.format_exception(*exc_info))
 
         backend_sentry_event = None