diff --git a/src/README.md b/src/README.md index 104b361f804..8618c2bb770 100644 --- a/src/README.md +++ b/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 diff --git a/src/tribler-common/tribler_common/sentry_reporter/sentry_reporter.py b/src/tribler-common/tribler_common/sentry_reporter/sentry_reporter.py index 971f4a3e6be..089a09ce32f 100644 --- a/src/tribler-common/tribler_common/sentry_reporter/sentry_reporter.py +++ b/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__()') diff --git a/src/tribler-common/tribler_common/sentry_reporter/sentry_tools.py b/src/tribler-common/tribler_common/sentry_reporter/sentry_tools.py index 487bbd99f32..0cb5b6797ac 100644 --- a/src/tribler-common/tribler_common/sentry_reporter/sentry_tools.py +++ b/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. diff --git a/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_reporter.py b/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_reporter.py index 15ee1af87f1..98b11c1fe55 100644 --- a/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_reporter.py +++ b/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_reporter.py @@ -1,15 +1,16 @@ 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 @@ -17,7 +18,7 @@ def __init__(self): 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}/']}}} diff --git a/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_scrubber.py b/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_scrubber.py index 3848165978c..19d608512eb 100644 --- a/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_scrubber.py +++ b/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/"'], diff --git a/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_tools.py b/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_tools.py index 26870b0a830..ed921b7d8c2 100644 --- a/src/tribler-common/tribler_common/sentry_reporter/tests/test_sentry_tools.py +++ b/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', {}) == {} diff --git a/src/tribler-gui/tribler_gui/dialogs/feedbackdialog.py b/src/tribler-gui/tribler_gui/dialogs/feedbackdialog.py index 813e3f16e88..7024c9c6898 100644 --- a/src/tribler-gui/tribler_gui/dialogs/feedbackdialog.py +++ b/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') diff --git a/src/tribler-gui/tribler_gui/tribler_window.py b/src/tribler-gui/tribler_gui/tribler_window.py index b58b4233790..507bec81a51 100644 --- a/src/tribler-gui/tribler_gui/tribler_window.py +++ b/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