ros2 / ros2/launch

Do not swallow errors from ExecuteProcess

Open
#586 2 comments 0 reactions 1 assignee View on GitHub

@hidmic is already working on this.

Since Feb 3, 2022.

bug
Dominant language
Python
Stars
155
Forks
182
Avg merge
2d 14h
Merged PRs (30d)
6

Description

Bug report

Required Info:

  • Operating System:
    • Ubuntu 20.04
  • Installation type:
    • binaries
  • Version or commit hash:
    • 0.17.0
  • DDS implementation:
    • N/A
  • Client library (if applicable):
    • N/A
Steps to reproduce issue

repro.py

from launch import LaunchDescription, LaunchService
from launch.actions import ExecuteProcess

def test_fakecommand():
    def check_ok_process(event, context):
        assert 0 == event.returncode
    ls = LaunchService()
    ls.include_launch_description(
        LaunchDescription(
            [
                ExecuteProcess(
                    cmd=["fakecommand"],
                    on_exit=check_ok_process
                )
            ]
        )
    )
    assert 0 == ls.run()

then run

python3 -m pytest -rP repro.py
Expected behavior

Either expect LaunchService::run() to return a nonzero exit code, or the process to return a nonzero exit code. Either of these cases should cause the test to fail. This sort of test allows me to write automated tests that should validate my launch files.

Actual behavior

The test passes, and the error is swallowed, without an easy way to catch it in the test.

================================= test session starts ==================================
platform linux -- Python 3.8.10, pytest-4.6.9, py-1.8.1, pluggy-0.13.0
rootdir: ...
plugins: ament-lint-0.10.6, ament-mypy-0.10.6, ament-flake8-0.10.6, launch-testing-ros-0.14.2, ament-copyright-0.10.6, ament-pep257-0.10.6, launch-testing-0.17.0, ament-xmllint-0.10.6, colcon-core-0.6.1, cov-2.8.1
collected 1 item                                                                       
repro.py .                               [100%]

======================================== PASSES ========================================
________________________________ test_fakecommand ________________________________
--------------------------------- Captured stdout call ---------------------------------
[INFO] [launch]: All log files can be found below ...
[INFO] [launch]: Default logging verbosity is set to INFO
[ERROR] [fakecommand-1]: exception occurred while executing process:
Traceback (most recent call last):
  File "/opt/ros/galactic/lib/python3.8/site-packages/launch/actions/execute_process.py", line 737, in __execute_process
    transport, self._subprocess_protocol = await async_execute_process(
  File "/opt/ros/galactic/lib/python3.8/site-packages/osrf_pycommon/process_utils/async_execute_process_asyncio/impl.py", line 139, in async_execute_process
    transport, protocol = await _async_execute_process_nopty(
  File "/opt/ros/galactic/lib/python3.8/site-packages/osrf_pycommon/process_utils/async_execute_process_asyncio/impl.py", line 45, in _async_execute_process_nopty
    transport, protocol = await loop.subprocess_exec(
  File "/usr/lib/python3.8/asyncio/base_events.py", line 1630, in subprocess_exec
    transport = await self._make_subprocess_transport(
  File "/usr/lib/python3.8/asyncio/unix_events.py", line 197, in _make_subprocess_transport
    transp = _UnixSubprocessTransport(self, protocol, args, shell,
  File "/usr/lib/python3.8/asyncio/base_subprocess.py", line 36, in __init__
    self._start(args=args, shell=shell, stdin=stdin, stdout=stdout,
  File "/usr/lib/python3.8/asyncio/unix_events.py", line 789, in _start
    self._proc = subprocess.Popen(
  File "/usr/lib/python3.8/subprocess.py", line 858, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.8/subprocess.py", line 1704, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: 'fakecommand'

=============================== 1 passed in 0.02 seconds ===============================
Additional information

This behavior is aligns with the exception being swallowed here: https://github.com/ros2/launch/blob/32dbb67a5b74e67c62483c213d3346906d64669e/launch/launch/actions/execute_local.py#L551-L556

I understand that not every error in a process should mean that the entire launch service should return an error. But anything that is a process-level error should result in a ProcessExited event or something to handle it with. This sort of error lives in-between, with nothing good to catch it with.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.