From cd627f6319a958fdd031a6b1292218004dff3222 Mon Sep 17 00:00:00 2001 From: drew2a Date: Thu, 11 Nov 2021 14:47:51 +0100 Subject: [PATCH] Make CoreExceptionHandler scope-dependent --- .../components/reporter/exception_handler.py | 46 +++++++------- .../reporter/tests/test_exception_handler.py | 63 ++++++++++--------- .../components/restapi/restapi_component.py | 10 ++- .../restapi/tests/test_restapi_component.py | 38 +++++++---- src/tribler-core/tribler_core/start_core.py | 5 +- 5 files changed, 92 insertions(+), 70 deletions(-) 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 afc98d46a1f..d22dfbeff58 100644 --- a/src/tribler-core/tribler_core/components/reporter/exception_handler.py +++ b/src/tribler-core/tribler_core/components/reporter/exception_handler.py @@ -11,7 +11,6 @@ from tribler_common.sentry_reporter.sentry_reporter import SentryReporter from tribler_core.components.base import ComponentStartupException -from tribler_core.utilities.utilities import froze_it # There are some errors that we are ignoring. IGNORED_ERRORS_BY_CODE = { @@ -32,15 +31,15 @@ } -@froze_it class CoreExceptionHandler: """ - This singleton handles Python errors arising in the Core by catching them, adding necessary context, + This class handles Python errors arising in the Core by catching them, adding necessary context, and sending them to the GUI through the events endpoint. It must be connected to the Asyncio loop. """ - _logger = logging.getLogger("CoreExceptionHandler") - report_callback: Optional[Callable[[ReportedError], None]] = None + def __init__(self): + self.logger = logging.getLogger("CoreExceptionHandler") + self.report_callback: Optional[Callable[[ReportedError], None]] = None @staticmethod def _get_long_text_from(exception: Exception): @@ -48,14 +47,8 @@ def _get_long_text_from(exception: Exception): print_exception(type(exception), exception, exception.__traceback__, file=buffer) return buffer.getvalue() - @classmethod - def _create_exception_from(cls, message: str): - text = f'Received error without exception: {message}' - cls._logger.warning(text) - return Exception(text) - - @classmethod - def _is_ignored(cls, exception: Exception): + @staticmethod + def _is_ignored(exception: Exception): exception_class = exception.__class__ error_number = exception.errno if hasattr(exception, 'errno') else None @@ -68,31 +61,35 @@ def _is_ignored(cls, exception: Exception): pattern = IGNORED_ERRORS_BY_REGEX[exception_class] return re.search(pattern, str(exception)) is not None - @classmethod - def unhandled_error_observer(cls, loop, context): # pylint: disable=unused-argument + def _create_exception_from(self, message: str): + text = f'Received error without exception: {message}' + self.logger.warning(text) + return Exception(text) + + def unhandled_error_observer(self, _, context): """ This method is called when an unhandled error in Tribler is observed. It broadcasts the tribler_exception event. """ try: - SentryReporter.ignore_logger(cls._logger.name) + SentryReporter.ignore_logger(self.logger.name) should_stop = True context = context.copy() message = context.pop('message', 'no message') - exception = context.pop('exception', None) or cls._create_exception_from(message) + exception = context.pop('exception', None) or self._create_exception_from(message) # Exception text = str(exception) if isinstance(exception, ComponentStartupException): should_stop = exception.component.tribler_should_stop_on_component_error exception = exception.__cause__ - if cls._is_ignored(exception): - cls._logger.warning(exception) + if self._is_ignored(exception): + self.logger.warning(exception) return - long_text = cls._get_long_text_from(exception) - cls._logger.error(f"Unhandled exception occurred! {exception}\n{long_text}") + long_text = self._get_long_text_from(exception) + self.logger.error(f"Unhandled exception occurred! {exception}\n{long_text}") reported_error = ReportedError( type=exception.__class__.__name__, @@ -102,9 +99,12 @@ def unhandled_error_observer(cls, loop, context): # pylint: disable=unused-argu event=SentryReporter.event_from_exception(exception) or {}, should_stop=should_stop ) - if cls.report_callback: - cls.report_callback(reported_error) # pylint: disable=not-callable + if self.report_callback: + self.report_callback(reported_error) # pylint: disable=not-callable except Exception as ex: SentryReporter.capture_exception(ex) raise ex + + +default_core_exception_handler = CoreExceptionHandler() 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 7040069a168..b990dd092e2 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 @@ -11,9 +11,14 @@ pytestmark = pytest.mark.asyncio -# pylint: disable=protected-access +# pylint: disable=protected-access, redefined-outer-name # fmt: off +@pytest.fixture +def exception_handler(): + return CoreExceptionHandler() + + def raise_error(error): # pylint: disable=inconsistent-return-statements try: raise error @@ -21,51 +26,51 @@ def raise_error(error): # pylint: disable=inconsistent-return-statements return e -async def test_is_ignored(): +async def test_is_ignored(exception_handler): # test that CoreExceptionHandler ignores specific exceptions # by type - assert CoreExceptionHandler._is_ignored(OSError(113, 'Any')) - assert CoreExceptionHandler._is_ignored(ConnectionResetError(10054, 'Any')) + assert exception_handler._is_ignored(OSError(113, 'Any')) + assert exception_handler._is_ignored(ConnectionResetError(10054, 'Any')) # by class - assert CoreExceptionHandler._is_ignored(gaierror('Any')) + assert exception_handler._is_ignored(gaierror('Any')) # by class and substring - assert CoreExceptionHandler._is_ignored(RuntimeError('Message that contains invalid info-hash')) + assert exception_handler._is_ignored(RuntimeError('Message that contains invalid info-hash')) -async def test_is_not_ignored(): +async def test_is_not_ignored(exception_handler): # test that CoreExceptionHandler do not ignore exceptions out of # IGNORED_ERRORS_BY_CODE and IGNORED_ERRORS_BY_SUBSTRING - assert not CoreExceptionHandler._is_ignored(OSError(1, 'Any')) - assert not CoreExceptionHandler._is_ignored(RuntimeError('Any')) - assert not CoreExceptionHandler._is_ignored(AttributeError()) + assert not exception_handler._is_ignored(OSError(1, 'Any')) + assert not exception_handler._is_ignored(RuntimeError('Any')) + assert not exception_handler._is_ignored(AttributeError()) -async def test_create_exception_from(): +async def test_create_exception_from(exception_handler): # test that CoreExceptionHandler can create an Exception from a string - assert isinstance(CoreExceptionHandler._create_exception_from('Any'), Exception) + assert isinstance(exception_handler._create_exception_from('Any'), Exception) -async def test_get_long_text_from(): +async def test_get_long_text_from(exception_handler): # test that CoreExceptionHandler can generate stacktrace from an Exception error = raise_error(AttributeError('Any')) - actual_string = CoreExceptionHandler._get_long_text_from(error) + actual_string = exception_handler._get_long_text_from(error) assert 'raise_error' in actual_string @patch(f'{sentry_reporter.__name__}.{SentryReporter.__name__}.{SentryReporter.event_from_exception.__name__}', new=MagicMock(return_value={'sentry': 'event'})) -async def test_unhandled_error_observer_exception(): +async def test_unhandled_error_observer_exception(exception_handler): # test that unhandled exception, represented by Exception, reported to the GUI context = {'exception': raise_error(AttributeError('Any')), 'Any key': 'Any value'} - CoreExceptionHandler.report_callback = MagicMock() - CoreExceptionHandler.unhandled_error_observer(None, context) - CoreExceptionHandler.report_callback.assert_called() + exception_handler.report_callback = MagicMock() + exception_handler.unhandled_error_observer(None, context) + exception_handler.report_callback.assert_called() # get the argument that has been passed to the report_callback - reported_error = CoreExceptionHandler.report_callback.call_args_list[-1][0][0] + reported_error = exception_handler.report_callback.call_args_list[-1][0][0] assert reported_error.type == 'AttributeError' assert reported_error.text == 'Any' assert 'raise_error' in reported_error.long_text @@ -74,15 +79,15 @@ async def test_unhandled_error_observer_exception(): assert reported_error.should_stop -async def test_unhandled_error_observer_only_message(): +async def test_unhandled_error_observer_only_message(exception_handler): # test that unhandled exception, represented by message, reported to the GUI context = {'message': 'Any'} - CoreExceptionHandler.report_callback = MagicMock() - CoreExceptionHandler.unhandled_error_observer(None, context) - CoreExceptionHandler.report_callback.assert_called() + exception_handler.report_callback = MagicMock() + exception_handler.unhandled_error_observer(None, context) + exception_handler.report_callback.assert_called() # get the argument that has been passed to the report_callback - reported_error = CoreExceptionHandler.report_callback.call_args_list[-1][0][0] + reported_error = exception_handler.report_callback.call_args_list[-1][0][0] assert reported_error.type == 'Exception' assert reported_error.text == 'Received error without exception: Any' assert reported_error.long_text == 'Exception: Received error without exception: Any\n' @@ -91,11 +96,11 @@ async def test_unhandled_error_observer_only_message(): assert reported_error.should_stop -async def test_unhandled_error_observer_ignored(): +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, '')} - CoreExceptionHandler.report_callback = MagicMock() - with patch.object(CoreExceptionHandler._logger, 'warning') as mocked_warning: - CoreExceptionHandler.unhandled_error_observer(None, context) + exception_handler.report_callback = MagicMock() + with patch.object(exception_handler.logger, 'warning') as mocked_warning: + exception_handler.unhandled_error_observer(None, context) mocked_warning.assert_called_once() - CoreExceptionHandler.report_callback.assert_not_called() + exception_handler.report_callback.assert_not_called() 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 c22e61cd439..cbd6d6daab8 100644 --- a/src/tribler-core/tribler_core/components/restapi/restapi_component.py +++ b/src/tribler-core/tribler_core/components/restapi/restapi_component.py @@ -7,7 +7,7 @@ from tribler_common.simpledefs import STATE_START_API from tribler_core.components.base import Component -from tribler_core.components.reporter.exception_handler import CoreExceptionHandler +from tribler_core.components.reporter.exception_handler import CoreExceptionHandler, default_core_exception_handler from tribler_core.components.reporter.reporter_component import ReporterComponent from tribler_core.components.restapi.rest.debug_endpoint import DebugEndpoint from tribler_core.components.restapi.rest.events_endpoint import EventsEndpoint @@ -75,6 +75,7 @@ class RESTComponent(Component): _events_endpoint: EventsEndpoint _state_endpoint: StateEndpoint + _core_exception_handler: CoreExceptionHandler = default_core_exception_handler async def run(self): await super().run() @@ -115,10 +116,13 @@ def report_callback(reported_error: ReportedError): self._events_endpoint.on_tribler_exception(reported_error) self._state_endpoint.on_tribler_exception(reported_error.text) - CoreExceptionHandler.report_callback = report_callback + self._core_exception_handler.report_callback = report_callback async def shutdown(self): await super().shutdown() - CoreExceptionHandler.report_callback = None + + if self._core_exception_handler: + self._core_exception_handler.report_callback = None + if self.rest_manager: await self.rest_manager.stop() diff --git a/src/tribler-core/tribler_core/components/restapi/tests/test_restapi_component.py b/src/tribler-core/tribler_core/components/restapi/tests/test_restapi_component.py index eaefe8519eb..e010961107a 100644 --- a/src/tribler-core/tribler_core/components/restapi/tests/test_restapi_component.py +++ b/src/tribler-core/tribler_core/components/restapi/tests/test_restapi_component.py @@ -1,4 +1,4 @@ -from unittest.mock import MagicMock +from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -7,6 +7,7 @@ from tribler_core.components.base import Session from tribler_core.components.key.key_component import KeyComponent from tribler_core.components.reporter.exception_handler import CoreExceptionHandler +from tribler_core.components.restapi.rest.rest_manager import RESTManager from tribler_core.components.restapi.restapi_component import RESTComponent pytestmark = pytest.mark.asyncio @@ -14,17 +15,6 @@ # pylint: disable=protected-access, not-callable -def assert_report_callback_is_correct(component: RESTComponent): - assert CoreExceptionHandler.report_callback - component._events_endpoint.on_tribler_exception = MagicMock() - component._state_endpoint.on_tribler_exception = MagicMock() - - error = ReportedError(type='', text='text', event={}) - CoreExceptionHandler.report_callback(error) - - component._events_endpoint.on_tribler_exception.assert_called_with(error) - component._state_endpoint.on_tribler_exception.assert_called_with(error.text) - async def test_restful_component(tribler_config): components = [KeyComponent(), RESTComponent()] @@ -35,5 +25,27 @@ async def test_restful_component(tribler_config): comp = RESTComponent.instance() assert comp.started_event.is_set() and not comp.failed assert comp.rest_manager - assert_report_callback_is_correct(comp) await session.shutdown() + + +@patch.object(RESTComponent, 'get_component', new=AsyncMock()) +@patch.object(RESTManager, 'start', new=AsyncMock()) +async def test_report_callback_set_up_correct(): + component = RESTComponent() + component.session = MagicMock() + + component._core_exception_handler = CoreExceptionHandler() + + await component.run() + + # mock callbacks + component._events_endpoint.on_tribler_exception = MagicMock() + component._state_endpoint.on_tribler_exception = MagicMock() + + # try to call report_callback from core_exception_handler and assert + # that corresponding methods in events_endpoint and state_endpoint have been called + + error = ReportedError(type='', text='text', event={}) + component._core_exception_handler.report_callback(error) + component._events_endpoint.on_tribler_exception.assert_called_with(error) + component._state_endpoint.on_tribler_exception.assert_called_with(error.text) diff --git a/src/tribler-core/tribler_core/start_core.py b/src/tribler-core/tribler_core/start_core.py index e0fff49237b..cb79afd903e 100644 --- a/src/tribler-core/tribler_core/start_core.py +++ b/src/tribler-core/tribler_core/start_core.py @@ -23,7 +23,7 @@ from tribler_core.components.metadata_store.metadata_store_component import MetadataStoreComponent from tribler_core.components.payout.payout_component import PayoutComponent from tribler_core.components.popularity.popularity_component import PopularityComponent -from tribler_core.components.reporter.exception_handler import CoreExceptionHandler +from tribler_core.components.reporter.exception_handler import default_core_exception_handler from tribler_core.components.reporter.reporter_component import ReporterComponent from tribler_core.components.resource_monitor.resource_monitor_component import ResourceMonitorComponent from tribler_core.components.restapi.restapi_component import RESTComponent @@ -172,7 +172,8 @@ def start_tribler_core(base_path, api_port, api_key, root_state_dir, gui_test_mo asyncio.set_event_loop(asyncio.SelectorEventLoop()) loop = asyncio.get_event_loop() - loop.set_exception_handler(CoreExceptionHandler.unhandled_error_observer) + exception_handler = default_core_exception_handler + loop.set_exception_handler(exception_handler.unhandled_error_observer) loop.run_until_complete(core_session(config, components=list(components_gen(config))))