Skip to content

Commit

Permalink
Rework task exceptions loop. (#755)
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
clalancette authored Jan 30, 2024
1 parent 749a6e5 commit ec3ae49
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 11 deletions.
22 changes: 14 additions & 8 deletions launch/launch/launch_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions launch/test/launch/frontend/test_substitutions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit ec3ae49

Please sign in to comment.