Merge pull request #7593 from drew2a/feature/7424

Refactor the FeedbackDialog

src/tribler/core/sentry_reporter/sentry_reporter.py

@@ -33,7 +33,6 @@
 NAME = 'name'
 VERSION = 'version'
 BROWSER = 'browser'
-PLATFORM_DETAILS = 'platform.details'
 STACKTRACE = '_stacktrace'
 STACKTRACE_EXTRA = f'{STACKTRACE}_extra'
 STACKTRACE_CONTEXT = f'{STACKTRACE}_context'

src/tribler/core/sentry_reporter/sentry_scrubber.py

@@ -52,9 +52,11 @@ def remove_breadcrumbs(event: Dict) -> Dict:
 
     def _compile_re(self):
         """Compile all regular expressions."""
+        slash = r'[/\\]'
         for folder in self.home_folders:
-            folder_pattern = r'(?<=' + folder + r'[/\\])[\w\s~]+(?=[/\\])'
-            self.re_folders.append(re.compile(folder_pattern, re.I))
+            for separator in [slash, slash * 2]:
+                folder_pattern = rf'(?<={folder}{separator})[\w\s~]+(?={separator})'
+                self.re_folders.append(re.compile(folder_pattern, re.I))
 
         self.re_ip = re.compile(r'(?<!\.)\b(\d{1,3}\.){3}\d{1,3}\b(?!\.)', re.I)
         self.re_hash = re.compile(r'\b[0-9a-f]{40}\b', re.I)

src/tribler/core/sentry_reporter/tests/test_sentry_scrubber.py

@@ -30,6 +30,11 @@ def scrubber():
     'Documents and Settings\\username\\some',
     'C:\\Users\\Some User\\',
     'C:\\Users\\USERNAM~1\\',
+
+    # double slashes (could be present as errors during a serialisation)
+    'C:\\\\Users\\\\username\\\\',
+    '//home//username//some//',
+
 ]
 
 FOLDERS_NEGATIVE_MATCH = [

src/tribler/gui/dialogs/feedbackdialog.py

@@ -1,25 +1,22 @@
 from __future__ import annotations
 
+import json
 import os
 import platform
 import sys
 import time
-from collections import defaultdict
-from typing import TYPE_CHECKING
+from typing import Any, Optional, TYPE_CHECKING
 
 from PyQt5 import uic
-from PyQt5.QtWidgets import QAction, QDialog, QMessageBox, QTreeWidgetItem
+from PyQt5.QtWidgets import QAction, QDialog, QMessageBox
 
 from tribler.core.components.reporter.reported_error import ReportedError
 from tribler.core.sentry_reporter.sentry_reporter import ADDITIONAL_INFORMATION, COMMENTS, LAST_PROCESSES, MACHINE, \
     OS, \
     OS_ENVIRON, PLATFORM, \
-    PLATFORM_DETAILS, \
     SYSINFO, SentryReporter, \
     VERSION
 from tribler.core.sentry_reporter.sentry_scrubber import SentryScrubber
-from tribler.core.sentry_reporter.sentry_tools import delete_item, \
-    get_first_item
 from tribler.gui.sentry_mixin import AddBreadcrumbOnShowMixin
 from tribler.gui.tribler_action_menu import TriblerActionMenu
 from tribler.gui.utilities import connect, get_ui_file_path, tr
@@ -28,6 +25,53 @@
     from tribler.gui.tribler_window import TriblerWindow
 
 
+def dump(obj: Optional[Any], indent: int = 0) -> str:
+    """
+    Dump a value to a string
+    Args:
+        obj: The value to dump
+        indent: The indentation level
+
+    Returns:
+        The dumped value
+    """
+    ind = ' ' * indent
+
+    def join(strings):
+        joined = ',\n'.join(strings)
+        return f"\n{joined}\n{ind}"
+
+    if isinstance(obj, dict):
+        items = (f"{ind}  {repr(k)}: {dump(v, indent + 2)}" for k, v in obj.items())
+        return f'{{{join(items)}}}'
+
+    if isinstance(obj, (list, tuple)):
+        closing = ['(', ')'] if isinstance(obj, tuple) else ['[', ']']
+        items = (f"{ind}  {dump(x, indent + 2)}" for x in obj)
+        return f'{closing[0]}{join(items)}{closing[1]}'
+
+    return repr(obj)
+
+
+def dump_with_name(name: str, value: Optional[str | dict], start: str = '\n\n', delimiter: str = '=' * 40) -> str:
+    """
+    Dump a value to a string with a name
+    Args:
+        name: The name of the value
+        value: The value to dump
+        start: The start of the string
+        delimiter: The delimiter to use
+
+    Returns:
+        The dumped value
+    """
+    text = start + delimiter
+    text += f'\n{name}:\n'
+    text += delimiter + '\n'
+    text += dump(value)
+    return text
+
+
 class FeedbackDialog(AddBreadcrumbOnShowMixin, QDialog):
     def __init__(  # pylint: disable=too-many-arguments, too-many-locals
             self,
@@ -40,19 +84,60 @@ def __init__(  # pylint: disable=too-many-arguments, too-many-locals
             additional_tags=None,
     ):
         QDialog.__init__(self, parent)
-        self.core_manager = parent.core_manager
-        self.process_manager = parent.process_manager
-
+        self.scrubber = SentryScrubber()
+        sentry_reporter.collecting_breadcrumbs_allowed = False  # stop collecting breadcrumbs while the dialog is open
         uic.loadUi(get_ui_file_path('feedback_dialog.ui'), self)
+        self.setWindowTitle(reported_error.type)
 
-        self.setWindowTitle(tr("Unexpected error"))
-        self.tribler_version = tribler_version
+        self.core_manager = parent.core_manager
+        self.process_manager = parent.process_manager
         self.reported_error = reported_error
-        self.scrubber = SentryScrubber()
         self.sentry_reporter = sentry_reporter
         self.stop_application_on_close = stop_application_on_close
+        self.tribler_version = tribler_version
         self.additional_tags = additional_tags or {}
-        sentry_reporter.collecting_breadcrumbs_allowed = False  # stop collecting breadcrumbs while the dialog is open
+        # tags
+        self.additional_tags.update({
+            VERSION: tribler_version,
+            MACHINE: platform.machine(),
+            OS: platform.platform(),
+            PLATFORM: sys.platform
+        })
+
+        self.info = {
+            '_error_text': self.reported_error.text,
+            '_error_long_text': self.reported_error.long_text,
+            '_error_context': self.reported_error.context,
+            COMMENTS: self.comments_text_edit.toPlainText(),
+            SYSINFO: {
+                'os.getcwd': f'{os.getcwd()}',
+                'sys.executable': f'{sys.executable}',
+                'os': os.name,
+                'platform.machine': platform.machine(),
+                'python.version': sys.version,
+                'in_debug': str(__debug__),
+                'tribler_uptime': f"{time.time() - start_time}",
+                'sys.argv': list(sys.argv),
+                'sys.path': list(sys.path)
+            },
+            OS_ENVIRON: os.environ,
+            ADDITIONAL_INFORMATION: self.reported_error.additional_information,
+            LAST_PROCESSES: [str(p) for p in self.process_manager.get_last_processes()]
+        }
+
+        text = dump_with_name('Stacktrace', self.reported_error.long_text, start='')
+        text += dump_with_name('Info', self.info)
+        text += dump_with_name('Additional tags', self.additional_tags)
+        text += dump_with_name('Event', json.dumps(reported_error.event, indent=4))
+        text = text.replace('\\n', '\n')
+        text = self.scrubber.scrub_text(text)
+        self.error_text_edit.setPlainText(text)
+
+        self.send_automatically = SentryReporter.is_in_test_mode()
+        if self.send_automatically:
+            self.stop_application_on_close = True
+            self.on_send_clicked(True)
+
         # Qt 5.2 does not have the setPlaceholderText property
         if hasattr(self.comments_text_edit, "setPlaceholderText"):
             placeholder = tr(
@@ -61,53 +146,9 @@ def __init__(  # pylint: disable=too-many-arguments, too-many-locals
             )
             self.comments_text_edit.setPlaceholderText(placeholder)
 
-        def add_item_to_info_widget(key, value):
-            item = QTreeWidgetItem(self.env_variables_list)
-            item.setText(0, key)
-            scrubbed_value = self.scrubber.scrub_text(value)
-            item.setText(1, scrubbed_value)
-
-        text_for_viewing = '\n'.join(
-            (
-                reported_error.text,
-                reported_error.long_text,
-                reported_error.context,
-            )
-        )
-        stacktrace = self.scrubber.scrub_text(text_for_viewing.rstrip())
-        self.error_text_edit.setPlainText(stacktrace)
         connect(self.cancel_button.clicked, self.on_cancel_clicked)
         connect(self.send_report_button.clicked, self.on_send_clicked)
 
-        # Add machine information to the tree widget
-        add_item_to_info_widget('os.getcwd', f'{os.getcwd()}')
-        add_item_to_info_widget('sys.executable', f'{sys.executable}')
-
-        add_item_to_info_widget('os', os.name)
-        add_item_to_info_widget('platform', sys.platform)
-        add_item_to_info_widget('platform.details', platform.platform())
-        add_item_to_info_widget('platform.machine', platform.machine())
-        add_item_to_info_widget('python.version', sys.version)
-        add_item_to_info_widget('indebug', str(__debug__))
-        add_item_to_info_widget('tribler_uptime', f"{time.time() - start_time}")
-
-        for argv in sys.argv:
-            add_item_to_info_widget('sys.argv', f'{argv}')
-
-        for path in sys.path:
-            add_item_to_info_widget('sys.path', f'{path}')
-
-        for key in os.environ.keys():
-            add_item_to_info_widget('os.environ', f'{key}: {os.environ[key]}')
-
-        # Users can remove specific lines in the report
-        connect(self.env_variables_list.customContextMenuRequested, self.on_right_click_item)
-
-        self.send_automatically = SentryReporter.is_in_test_mode()
-        if self.send_automatically:
-            self.stop_application_on_close = True
-            self.on_send_clicked(True)
-
     def on_remove_entry(self, index):
         self.env_variables_list.takeTopLevelItem(index)
 
@@ -130,39 +171,10 @@ def on_send_clicked(self, checked):
         self.send_report_button.setEnabled(False)
         self.send_report_button.setText(tr("SENDING..."))
 
-        sys_info = defaultdict(lambda: [])
-        for ind in range(self.env_variables_list.topLevelItemCount()):
-            item = self.env_variables_list.topLevelItem(ind)
-            key = item.text(0)
-            value = item.text(1)
-
-            sys_info[key].append(value)
-
-        # tags
-        self.additional_tags[VERSION] = self.tribler_version
-        self.additional_tags[MACHINE] = platform.machine()
-        self.additional_tags[OS] = platform.platform()
-        self.additional_tags[PLATFORM] = get_first_item(sys_info[PLATFORM])
-        self.additional_tags[PLATFORM_DETAILS] = get_first_item(sys_info[PLATFORM_DETAILS])
-
-        # info
-        info = {}
-
-        info['_error_text'] = self.reported_error.text
-        info['_error_long_text'] = self.reported_error.long_text
-        info['_error_context'] = self.reported_error.context
-        info[COMMENTS] = self.comments_text_edit.toPlainText()
-        info[SYSINFO] = sys_info
-        info[OS_ENVIRON] = sys_info[OS_ENVIRON]
-        delete_item(info[SYSINFO], OS_ENVIRON)
-
-        info[ADDITIONAL_INFORMATION] = self.reported_error.additional_information
-        info[LAST_PROCESSES] = [str(p) for p in self.process_manager.get_last_processes()]
-
         self.sentry_reporter.send_event(
             event=self.reported_error.event,
             tags=self.additional_tags,
-            info=info,
+            info=self.info,
             last_core_output=self.reported_error.last_core_output,
             tribler_version=self.tribler_version
         )

src/tribler/gui/dialogs/tests/test_feedbackdialog.py

@@ -0,0 +1,53 @@
+from unittest.mock import Mock, patch
+
+from tribler.gui.dialogs.feedbackdialog import dump, dump_with_name
+
+
+def test_dump_with_name_start():
+    """ Test dump_with_name with a start value"""
+    actual = dump_with_name('name', 'value', 'start')
+    expected = ('start========================================\n'
+                'name:\n'
+                '========================================\n'
+                "'value'")
+
+    assert actual == expected
+
+
+@patch('tribler.gui.dialogs.feedbackdialog.dump')
+def test_dump_with_name_str(mock_dump: Mock):
+    """ Test that dump_with_name calls the `dump` function"""
+    dump_with_name('name', 'value')
+    assert mock_dump.called
+
+
+def test_dump_none():
+    """ Test dump with a None value"""
+    assert dump(None) == 'None'
+
+
+def test_dump_dict():
+    """ Test dump with a complex dict value"""
+    actual = dump(
+        {
+
+            'key': {
+                'key1': 'value1'
+            },
+            'key2': 'value2',
+            'key3': ['value3', 'value4']
+        }
+    )
+
+    expected = ('{\n'
+                "  'key': {\n"
+                "    'key1': 'value1'\n"
+                '  },\n'
+                "  'key2': 'value2',\n"
+                "  'key3': [\n"
+                "    'value3',\n"
+                "    'value4'\n"
+                '  ]\n'
+                '}')
+
+    assert actual == expected