From f0b7f13d61a59d4e90c32fd724c51b0774ba55b7 Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Tue, 30 Nov 2021 18:52:03 +0100 Subject: [PATCH 1/9] Enable logging for EventRequestManager --- src/tribler-gui/tribler_gui/event_request_manager.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tribler-gui/tribler_gui/event_request_manager.py b/src/tribler-gui/tribler_gui/event_request_manager.py index bc865aad805..7547dd99b9e 100644 --- a/src/tribler-gui/tribler_gui/event_request_manager.py +++ b/src/tribler-gui/tribler_gui/event_request_manager.py @@ -44,7 +44,7 @@ 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__) self.tribler_started_flag = False self.reactions_dict = { NTFY.CHANNEL_ENTITY_UPDATED.value: self.node_info_updated.emit, @@ -71,7 +71,6 @@ def events_start_received(self, event_dict): def on_error(self, error, reschedule_on_err): self._logger.info(f"Got Tribler core error: {error}") - SentryReporter.ignore_logger(self._logger.name) if self.remaining_connection_attempts <= 0: raise CoreConnectTimeoutError("Could not connect with the Tribler Core within 60 seconds") From 73754657ee430d49e305cf24fa36dd9a502cee60 Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Thu, 25 Nov 2021 14:35:06 +0100 Subject: [PATCH 2/9] Use original connect method to prevent problems on Mac --- src/tribler-gui/tribler_gui/upgrade_manager.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tribler-gui/tribler_gui/upgrade_manager.py b/src/tribler-gui/tribler_gui/upgrade_manager.py index 550fcce9717..f62b4820b9f 100644 --- a/src/tribler-gui/tribler_gui/upgrade_manager.py +++ b/src/tribler-gui/tribler_gui/upgrade_manager.py @@ -120,10 +120,10 @@ def start(self): self._upgrade_thread.started.connect(self._upgrade_worker.run) 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.status_update.connect(self.upgrader_tick.emit) + self._upgrade_worker.finished.connect(self.upgrader_finished.emit) + self._upgrade_worker.finished.connect(self._upgrade_worker.deleteLater) self._upgrade_thread.start() From 9405da642034bc2f44232a6d6797a6a569cc7230 Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Tue, 30 Nov 2021 19:24:47 +0100 Subject: [PATCH 3/9] Use direct connection for signal-to-signal connects --- src/tribler-gui/tribler_gui/tribler_window.py | 3 +-- src/tribler-gui/tribler_gui/upgrade_manager.py | 4 +++- src/tribler-gui/tribler_gui/widgets/discoveringpage.py | 2 +- src/tribler-gui/tribler_gui/widgets/loadingpage.py | 2 +- src/tribler-gui/tribler_gui/widgets/searchresultswidget.py | 4 ---- 5 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/tribler-gui/tribler_gui/tribler_window.py b/src/tribler-gui/tribler_gui/tribler_window.py index d7f710b099a..870446f298a 100644 --- a/src/tribler-gui/tribler_gui/tribler_window.py +++ b/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() diff --git a/src/tribler-gui/tribler_gui/upgrade_manager.py b/src/tribler-gui/tribler_gui/upgrade_manager.py index f62b4820b9f..d6ecd3f93bf 100644 --- a/src/tribler-gui/tribler_gui/upgrade_manager.py +++ b/src/tribler-gui/tribler_gui/upgrade_manager.py @@ -119,9 +119,11 @@ 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) self._upgrade_worker.finished.connect(self._upgrade_thread.quit) - self._upgrade_worker.status_update.connect(self.upgrader_tick.emit) self._upgrade_worker.finished.connect(self.upgrader_finished.emit) self._upgrade_worker.finished.connect(self._upgrade_worker.deleteLater) diff --git a/src/tribler-gui/tribler_gui/widgets/discoveringpage.py b/src/tribler-gui/tribler_gui/widgets/discoveringpage.py index a821b4ea83b..4ff71e2e1aa 100644 --- a/src/tribler-gui/tribler_gui/widgets/discoveringpage.py +++ b/src/tribler-gui/tribler_gui/widgets/discoveringpage.py @@ -22,7 +22,7 @@ def initialize_discovering_page(self): svg_item = QGraphicsSvgItem() svg = QSvgRenderer(get_image_path("loading_animation.svg")) - connect(svg.repaintNeeded, svg_item.update) + svg.repaintNeeded.connect(svg_item.update) svg_item.setSharedRenderer(svg) svg_container.addItem(svg_item) diff --git a/src/tribler-gui/tribler_gui/widgets/loadingpage.py b/src/tribler-gui/tribler_gui/widgets/loadingpage.py index 6599aafc348..4ffcb468684 100644 --- a/src/tribler-gui/tribler_gui/widgets/loadingpage.py +++ b/src/tribler-gui/tribler_gui/widgets/loadingpage.py @@ -21,7 +21,7 @@ def initialize_loading_page(self): svg_item = QGraphicsSvgItem() svg = QSvgRenderer(get_image_path("loading_animation.svg")) - connect(svg.repaintNeeded, svg_item.update) + svg.repaintNeeded.connect(svg_item.update) svg_item.setSharedRenderer(svg) svg_container.addItem(svg_item) diff --git a/src/tribler-gui/tribler_gui/widgets/searchresultswidget.py b/src/tribler-gui/tribler_gui/widgets/searchresultswidget.py index 582b25b3845..14a73661130 100644 --- a/src/tribler-gui/tribler_gui/widgets/searchresultswidget.py +++ b/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) From c8f62f06fceb0b263e93fcf0fb249b8f0eef2af8 Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Tue, 30 Nov 2021 19:54:31 +0100 Subject: [PATCH 4/9] Deduplicate gears animation loading code --- .../tribler_gui/widgets/discoveringpage.py | 17 +++---------- .../tribler_gui/widgets/loadingpage.py | 25 ++++++++++++------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/tribler-gui/tribler_gui/widgets/discoveringpage.py b/src/tribler-gui/tribler_gui/widgets/discoveringpage.py index 4ff71e2e1aa..9e66bb1b601 100644 --- a/src/tribler-gui/tribler_gui/widgets/discoveringpage.py +++ b/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")) - svg.repaintNeeded.connect(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, _): diff --git a/src/tribler-gui/tribler_gui/widgets/loadingpage.py b/src/tribler-gui/tribler_gui/widgets/loadingpage.py index 4ffcb468684..78bb8e71020 100644 --- a/src/tribler-gui/tribler_gui/widgets/loadingpage.py +++ b/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")) - svg.repaintNeeded.connect(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) From 8e8ed4418687355cecfee7eba1e1f47b5c587e67 Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Tue, 30 Nov 2021 19:58:06 +0100 Subject: [PATCH 5/9] Restore tribler_started flag in the GUI --- src/tribler-gui/tribler_gui/event_request_manager.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tribler-gui/tribler_gui/event_request_manager.py b/src/tribler-gui/tribler_gui/event_request_manager.py index 7547dd99b9e..6433fb126c6 100644 --- a/src/tribler-gui/tribler_gui/event_request_manager.py +++ b/src/tribler-gui/tribler_gui/event_request_manager.py @@ -45,6 +45,7 @@ def __init__(self, api_port, api_key, error_handler): self.shutting_down = False self.error_handler = error_handler 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, @@ -61,6 +62,7 @@ def __init__(self, api_port, api_key, error_handler): 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 From 70eea042a480db3d53726bf2a3131daf4c5bdc3c Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Wed, 1 Dec 2021 18:36:45 +0100 Subject: [PATCH 6/9] Fix reconnect on error timer and logging --- .../tribler_gui/event_request_manager.py | 33 +++++++++++-------- src/tribler-gui/tribler_gui/exceptions.py | 4 +++ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/tribler-gui/tribler_gui/event_request_manager.py b/src/tribler-gui/tribler_gui/event_request_manager.py index 6433fb126c6..825dcd64270 100644 --- a/src/tribler-gui/tribler_gui/event_request_manager.py +++ b/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): @@ -60,6 +61,9 @@ 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 @@ -71,19 +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") 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: @@ -119,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) diff --git a/src/tribler-gui/tribler_gui/exceptions.py b/src/tribler-gui/tribler_gui/exceptions.py index 13be8f2e935..cd3a57ff243 100644 --- a/src/tribler-gui/tribler_gui/exceptions.py +++ b/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): ... From b280044a36f068c519879d1e009e666ad3e32fdb Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Wed, 1 Dec 2021 19:17:37 +0100 Subject: [PATCH 7/9] Fix buffering error report --- .../tribler_core/components/reporter/exception_handler.py | 7 +++++++ .../components/reporter/tests/test_exception_handler.py | 6 ++++++ .../tribler_core/components/restapi/restapi_component.py | 4 ++++ 3 files changed, 17 insertions(+) diff --git a/src/tribler-core/tribler_core/components/reporter/exception_handler.py b/src/tribler-core/tribler_core/components/reporter/exception_handler.py index d22dfbeff58..6507b53119e 100644 --- a/src/tribler-core/tribler_core/components/reporter/exception_handler.py +++ b/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) diff --git a/src/tribler-core/tribler_core/components/reporter/tests/test_exception_handler.py b/src/tribler-core/tribler_core/components/reporter/tests/test_exception_handler.py index b990dd092e2..00a7c58130f 100644 --- a/src/tribler-core/tribler_core/components/reporter/tests/test_exception_handler.py +++ b/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, '')} diff --git a/src/tribler-core/tribler_core/components/restapi/restapi_component.py b/src/tribler-core/tribler_core/components/restapi/restapi_component.py index 2b9bcad6d99..845edb8f177 100644 --- a/src/tribler-core/tribler_core/components/restapi/restapi_component.py +++ b/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() From 9c722d3e959857818f05e2f2219734224cb3ecbb Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Wed, 1 Dec 2021 19:44:06 +0100 Subject: [PATCH 8/9] Bring back guitests mark to tags test --- src/tribler-gui/tribler_gui/tests/test_gui.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tribler-gui/tribler_gui/tests/test_gui.py b/src/tribler-gui/tribler_gui/tests/test_gui.py index 470941a8a4f..bcbeac07333 100644 --- a/src/tribler-gui/tribler_gui/tests/test_gui.py +++ b/src/tribler-gui/tribler_gui/tests/test_gui.py @@ -565,6 +565,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 +667,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. From 7d0491787be6220466582bdc038da209d3ae6826 Mon Sep 17 00:00:00 2001 From: "V.G. Bulavintsev" Date: Thu, 2 Dec 2021 15:20:24 +0100 Subject: [PATCH 9/9] Clear GUI settings on GUI test runs --- src/tribler-gui/tribler_gui/tests/test_gui.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/tribler-gui/tribler_gui/tests/test_gui.py b/src/tribler-gui/tribler_gui/tests/test_gui.py index bcbeac07333..064599e2334 100644 --- a/src/tribler-gui/tribler_gui/tests/test_gui.py +++ b/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'],