Merge pull request #6596 from ichorid/fix/gui_core_start

Fixes for GUI startup sequence

src/tribler-core/tribler_core/components/reporter/exception_handler.py

@@ -40,6 +40,7 @@ class CoreExceptionHandler:
     def __init__(self):
         self.logger = logging.getLogger("CoreExceptionHandler")
         self.report_callback: Optional[Callable[[ReportedError], None]] = None
+        self.unreported_error: Optional[ReportedError] = None
 
     @staticmethod
     def _get_long_text_from(exception: Exception):
@@ -101,6 +102,12 @@ def unhandled_error_observer(self, _, context):
             )
             if self.report_callback:
                 self.report_callback(reported_error)  # pylint: disable=not-callable
+            else:
+                if not self.unreported_error:
+                    # We only remember the first unreported error,
+                    # as that was probably the root cause for # the crash
+                    self.unreported_error = reported_error
+
 
         except Exception as ex:
             SentryReporter.capture_exception(ex)

src/tribler-core/tribler_core/components/reporter/tests/test_exception_handler.py

@@ -96,6 +96,12 @@ async def test_unhandled_error_observer_only_message(exception_handler):
     assert reported_error.should_stop
 
 
+async def test_unhandled_error_observer_store_unreported_error(exception_handler):
+    context = {'message': 'Any'}
+    exception_handler.unhandled_error_observer(None, context)
+    assert exception_handler.unreported_error
+
+
 async def test_unhandled_error_observer_ignored(exception_handler):
     # test that exception from list IGNORED_ERRORS_BY_CODE never sends to the GUI
     context = {'exception': OSError(113, '')}

src/tribler-core/tribler_core/components/restapi/restapi_component.py

@@ -126,6 +126,10 @@ def report_callback(reported_error: ReportedError):
             self._events_endpoint.on_tribler_exception(reported_error)
 
         self._core_exception_handler.report_callback = report_callback
+        # Reraise the unreported error, if there is one
+        if self._core_exception_handler.unreported_error:
+            report_callback(self._core_exception_handler.unreported_error)
+            self._core_exception_handler.unreported_error = None
 
     async def shutdown(self):
         await super().shutdown()

src/tribler-gui/tribler_gui/event_request_manager.py

@@ -3,18 +3,19 @@
 import time
 
 from PyQt5.QtCore import QTimer, QUrl, pyqtSignal
-from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkRequest
+from PyQt5.QtNetwork import QNetworkAccessManager, QNetworkReply, QNetworkRequest
 
 from tribler_common.reported_error import ReportedError
 from tribler_common.sentry_reporter.sentry_reporter import SentryReporter
 from tribler_common.simpledefs import NTFY
 
-from tribler_gui.exceptions import CoreConnectTimeoutError
+from tribler_gui.exceptions import CoreConnectTimeoutError, CoreConnectionError
 from tribler_gui.utilities import connect
 
 received_events = []
 
 CORE_CONNECTION_ATTEMPTS_LIMIT = 120
+RECONNECT_INTERVAL_MS = 500
 
 
 class EventRequestManager(QNetworkAccessManager):
@@ -44,7 +45,8 @@ def __init__(self, api_port, api_key, error_handler):
         self.reply = None
         self.shutting_down = False
         self.error_handler = error_handler
-        self._logger = logging.getLogger('TriblerGUI')
+        self._logger = logging.getLogger(self.__class__.__name__)
+        # This flag is used to prevent race condition when starting GUI tests
         self.tribler_started_flag = False
         self.reactions_dict = {
             NTFY.CHANNEL_ENTITY_UPDATED.value: self.node_info_updated.emit,
@@ -59,8 +61,12 @@ def __init__(self, api_port, api_key, error_handler):
             NTFY.TRIBLER_EXCEPTION.value: lambda data: self.error_handler.core_error(ReportedError(**data)),
         }
 
+        self.connect_timer.setSingleShot(True)
+        connect(self.connect_timer.timeout, self.connect)
+
     def events_start_received(self, event_dict):
         if event_dict["version"]:
+            self.tribler_started_flag = True
             self.tribler_started.emit(event_dict["version"])
             # if public key format will be changed, don't forget to change it
             # at the core side as well
@@ -69,20 +75,22 @@ def events_start_received(self, event_dict):
                 SentryReporter.set_user(public_key.encode('utf-8'))
 
     def on_error(self, error, reschedule_on_err):
-        self._logger.info(f"Got Tribler core error: {error}")
+        if error == QNetworkReply.ConnectionRefusedError:
+            self._logger.debug("Tribler Core refused connection, retrying...")
+        else:
+            raise CoreConnectionError(f"Error {error} while trying to connect to Tribler Core")
 
-        SentryReporter.ignore_logger(self._logger.name)
         if self.remaining_connection_attempts <= 0:
-            raise CoreConnectTimeoutError("Could not connect with the Tribler Core within 60 seconds")
+            raise CoreConnectTimeoutError(
+                f"Could not connect with the Tribler Core \
+                within {RECONNECT_INTERVAL_MS*CORE_CONNECTION_ATTEMPTS_LIMIT} seconds"
+            )
 
         self.remaining_connection_attempts -= 1
 
         if reschedule_on_err:
             # Reschedule an attempt
-            self.connect_timer = QTimer()
-            self.connect_timer.setSingleShot(True)
-            connect(self.connect_timer.timeout, self.connect)
-            self.connect_timer.start(500)
+            self.connect_timer.start(RECONNECT_INTERVAL_MS)
 
     def on_read_data(self):
         if self.receivers(self.finished) == 0:
@@ -118,14 +126,12 @@ def on_finished(self):
             return
         self._logger.warning("Events connection dropped, attempting to reconnect")
         self.remaining_connection_attempts = CORE_CONNECTION_ATTEMPTS_LIMIT
-
-        self.connect_timer = QTimer()
-        self.connect_timer.setSingleShot(True)
-        self.connect_timer.timeout.connect(self.connect)
-        self.connect_timer.start(500)
+        self.connect_timer.start(RECONNECT_INTERVAL_MS)
 
     def connect(self, reschedule_on_err=True):
-        self._logger.info("Will connect to events endpoint")
+        self._logger.debug("Will connect to events endpoint")
+        if self.reply is not None:
+            self.reply.deleteLater()
         self.reply = self.get(self.request)
 
         connect(self.reply.readyRead, self.on_read_data)

src/tribler-gui/tribler_gui/exceptions.py

@@ -2,6 +2,10 @@ class CoreError(Exception):
     """This is the base class for exceptions that causes GUI shutdown"""
 
 
+class CoreConnectionError(CoreError):
+    ...
+
+
 class CoreConnectTimeoutError(CoreError):
     ...
 

src/tribler-gui/tribler_gui/tests/test_gui.py

@@ -38,8 +38,12 @@ def window(tmpdir_factory):
     root_state_dir = str(tmpdir_factory.mktemp('tribler_state_dir'))
 
     app = TriblerApplication("triblerapp-guitest", sys.argv)
+    # We must create a separate instance of QSettings and clear it.
+    # Otherwise, previous runs of the same app will affect this run.
+    settings = QSettings("tribler-guitest")
+    settings.clear()
     window = TriblerWindow(  # pylint: disable=W0621
-        QSettings(),
+        settings,
         root_state_dir,
         api_key=api_key,
         core_args=[str(RUN_TRIBLER_PY.absolute()), '--core', '--gui-test-mode'],
@@ -565,6 +569,7 @@ def test_close_dialog_with_esc_button(window):
     assert not window.findChildren(NewChannelDialog)
 
 
+@pytest.mark.guitest
 def test_tags_dialog(window):
     """
     Test the behaviour of the dialog where a user can edit tags.
@@ -666,6 +671,7 @@ def test_tags_dialog(window):
     QTest.qWait(200)  # It can take a bit of time to hide the dialog
 
 
+@pytest.mark.guitest
 def test_no_tags(window):
     """
     Test removing all tags from a content item.

src/tribler-gui/tribler_gui/tribler_window.py

@@ -226,8 +226,7 @@ def __init__(
 
         self.search_results_page.initialize(hide_xxx=self.hide_xxx)
         connect(
-            self.core_manager.events_manager.received_remote_query_results,
-            self.search_results_page.received_remote_results.emit,
+            self.core_manager.events_manager.received_remote_query_results, self.search_results_page.update_loading_page
         )
         self.settings_page.initialize_settings_page(version_history=self.version_history)
         self.downloads_page.initialize_downloads_page()

src/tribler-gui/tribler_gui/upgrade_manager.py

@@ -119,11 +119,13 @@ def start(self):
         # Otherwise, if we use our own connect(x,y) wrapper, Tribler just freezes
         self._upgrade_thread.started.connect(self._upgrade_worker.run)
 
+        # ACHTUNG!!! the following signals cannot be properly handled by our "connect" method.
+        # These must be connected directly to prevent problems with disconnecting and thread handling.
+        self._upgrade_worker.status_update.connect(self.upgrader_tick.emit)
         self._upgrade_thread.finished.connect(self._upgrade_thread.deleteLater)
-        connect(self._upgrade_worker.finished, self._upgrade_thread.quit)
-        connect(self._upgrade_worker.status_update, self.upgrader_tick.emit)
-        connect(self._upgrade_worker.finished, self.upgrader_finished.emit)
-        connect(self._upgrade_worker.finished, self._upgrade_worker.deleteLater)
+        self._upgrade_worker.finished.connect(self._upgrade_thread.quit)
+        self._upgrade_worker.finished.connect(self.upgrader_finished.emit)
+        self._upgrade_worker.finished.connect(self._upgrade_worker.deleteLater)
 
         self._upgrade_thread.start()
 

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

@@ -1,9 +1,9 @@
-from PyQt5.QtSvg import QGraphicsSvgItem, QSvgRenderer
-from PyQt5.QtWidgets import QGraphicsScene, QWidget
+from PyQt5.QtWidgets import QWidget
 
 from tribler_common.sentry_reporter.sentry_mixin import AddBreadcrumbOnShowMixin
 
-from tribler_gui.utilities import connect, get_image_path
+from tribler_gui.utilities import connect
+from tribler_gui.widgets.loadingpage import LOADING_ANIMATION
 
 
 class DiscoveringPage(AddBreadcrumbOnShowMixin, QWidget):
@@ -18,16 +18,7 @@ def __init__(self):
         self.is_discovering = False
 
     def initialize_discovering_page(self):
-        svg_container = QGraphicsScene(self.window().discovering_svg_view)
-        svg_item = QGraphicsSvgItem()
-
-        svg = QSvgRenderer(get_image_path("loading_animation.svg"))
-        connect(svg.repaintNeeded, svg_item.update)
-        svg_item.setSharedRenderer(svg)
-        svg_container.addItem(svg_item)
-
-        self.window().discovering_svg_view.setScene(svg_container)
-
+        self.window().discovering_svg_view.setScene(LOADING_ANIMATION)
         connect(self.window().core_manager.events_manager.discovered_channel, self.on_discovered_channel)
 
     def on_discovered_channel(self, _):

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

@@ -6,6 +6,21 @@
 from tribler_gui.utilities import connect, get_image_path
 
 
+def load_gears_animation():
+    svg_container = QGraphicsScene()
+    svg_item = QGraphicsSvgItem()
+
+    svg = QSvgRenderer(get_image_path("loading_animation.svg"))
+    svg.repaintNeeded.connect(svg_item.update)
+    svg_item.setSharedRenderer(svg)
+
+    svg_container.addItem(svg_item)
+    return svg_container
+
+
+LOADING_ANIMATION = load_gears_animation()
+
+
 class LoadingPage(AddBreadcrumbOnShowMixin, QWidget):
     """
     This page is presented when Tribler is starting.
@@ -17,15 +32,7 @@ def __init__(self):
         self.upgrading = False
 
     def initialize_loading_page(self):
-        svg_container = QGraphicsScene(self.window().loading_svg_view)
-        svg_item = QGraphicsSvgItem()
-
-        svg = QSvgRenderer(get_image_path("loading_animation.svg"))
-        connect(svg.repaintNeeded, svg_item.update)
-        svg_item.setSharedRenderer(svg)
-        svg_container.addItem(svg_item)
-
-        self.window().loading_svg_view.setScene(svg_container)
+        self.window().loading_svg_view.setScene(LOADING_ANIMATION)
         connect(self.window().upgrade_manager.upgrader_tick, self.on_upgrader_tick)
         connect(self.window().upgrade_manager.upgrader_finished, self.upgrader_finished)
         connect(self.window().core_manager.events_manager.change_loading_text, self.change_loading_text)

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

@@ -4,7 +4,6 @@
 from dataclasses import dataclass, field
 
 from PyQt5 import uic
-from PyQt5.QtCore import pyqtSignal
 
 from tribler_common.sentry_reporter.sentry_mixin import AddBreadcrumbOnShowMixin
 from tribler_common.utilities import to_fts_query
@@ -48,8 +47,6 @@ def complete(self):
 
 
 class SearchResultsWidget(AddBreadcrumbOnShowMixin, widget_form, widget_class):
-    received_remote_results = pyqtSignal(object)
-
     def __init__(self, parent=None):
         widget_class.__init__(self, parent=parent)
 
@@ -67,7 +64,6 @@ def initialize(self, hide_xxx=False):
         self.hide_xxx = hide_xxx
         self.results_page.initialize_content_page(hide_xxx=hide_xxx)
         self.results_page.channel_torrents_filter_input.setHidden(True)
-        connect(self.received_remote_results, self.update_loading_page)
         connect(self.timeout_progress_bar.timeout, self.show_results)
         connect(self.show_results_button.clicked, self.show_results)