From ec3ae49ce3e65af3c18257b48fb143c7f159f66b Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 29 Jan 2024 20:01:19 -0500 Subject: [PATCH] Rework task exceptions loop. (#755) The original reason to do this was because mypy was having a difficult time ensuring that the final raised exception was valid. This fixes that problem. But this also should be much faster. Before we were iterating over the list 3 times; once to the collect the exceptions, once to filter out None, and once to them print them out. Instead, this just iterates once, dispatching everything properly. Signed-off-by: Chris Lalancette --- launch/launch/launch_service.py | 22 ++++++++++++------- .../launch/frontend/test_substitutions.py | 7 +++--- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/launch/launch/launch_service.py b/launch/launch/launch_service.py index 6dba4bc68..2e1719edf 100644 --- a/launch/launch/launch_service.py +++ b/launch/launch/launch_service.py @@ -327,14 +327,20 @@ def _on_exception(loop, context): return_when=asyncio.FIRST_COMPLETED ) # Propagate exception from completed tasks - completed_tasks_exceptions = [task.exception() for task in completed_tasks] - completed_tasks_exceptions = list(filter(None, completed_tasks_exceptions)) - if completed_tasks_exceptions: - self.__logger.debug('An exception was raised in an async action/event') - # in case there is more than one completed_task, log other exceptions - for completed_tasks_exception in completed_tasks_exceptions[1:]: - self.__logger.error(completed_tasks_exception) - raise completed_tasks_exceptions[0] + exception_to_raise = None + for task in completed_tasks: + exc = task.exception() + if exc is None: + continue + + if exception_to_raise is None: + self.__logger.debug('An exception was raised in an async action/event') + exception_to_raise = exc + else: + self.__logger.error(exc) + + if exception_to_raise is not None: + raise exception_to_raise except KeyboardInterrupt: continue diff --git a/launch/test/launch/frontend/test_substitutions.py b/launch/test/launch/frontend/test_substitutions.py index 1a5ce9538..75fbfd6a4 100644 --- a/launch/test/launch/frontend/test_substitutions.py +++ b/launch/test/launch/frontend/test_substitutions.py @@ -21,6 +21,7 @@ from launch import SomeSubstitutionsType from launch import Substitution from launch.actions import ExecuteProcess +from launch.frontend import Parser from launch.frontend.expose import expose_substitution from launch.frontend.parse_substitution import parse_if_substitutions from launch.frontend.parse_substitution import parse_substitution @@ -317,13 +318,13 @@ def test_parse_if_substitutions(): parse_if_substitutions(['$(test asd)', 1, 1.0]) -class MockParser: +class MockParser(Parser): - def parse_substitution(self, value: Text) -> SomeSubstitutionsType: + def parse_substitution(self, value: Text) -> List[Substitution]: return parse_substitution(value) -def test_execute_process_parse_cmd_line(): +def test_execute_process_parse_cmd_line() -> None: """Test ExecuteProcess._parse_cmd_line.""" parser = MockParser()