Skip to content

Commit 00e4615

Browse files
committed
Keep what a pipe process wrote before exiting early
When cmd2 captures a pipe process's output, as in a pyscript, that process writes to pipes that a ProcReader reads. Only a pipe that was not the terminal's job got its ProcReader after the startup check, so one that had already exited was reported with RedirectionError without being read: what it wrote, such as the shell's "command not found", was lost, and its pipes were left for garbage collection to close. Create the ProcReader right after the spawn for every pipe, and reap through it on the early exit.
1 parent 984d3f2 commit 00e4615

3 files changed

Lines changed: 29 additions & 9 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
- When a pipe's process cannot be started, for instance because no more processes can be created
2121
or `$SHELL` names a program that cannot run, both ends of the pipe are now closed rather than
2222
left open
23+
- When a pipe's process exits before the command runs, what it wrote, such as the shell's
24+
"command not found", is no longer lost when cmd2 captures the output, as in a pyscript
2325

2426
## 4.2.4 (September 8, 2026)
2527

‎cmd2/cmd2.py‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3393,16 +3393,16 @@ def _redirect_output(self, statement: Statement) -> utils.RedirectionSavedState:
33933393
shell=True,
33943394
**kwargs,
33953395
)
3396+
cmd_pipe_proc_reader = utils.ProcReader(proc, self.stdout, sys.stderr, terminal_fd=terminal_fd)
33963397
if terminal_fd is not None:
3397-
cmd_pipe_proc_reader = utils.ProcReader(proc, self.stdout, sys.stderr, terminal_fd=terminal_fd)
33983398
pipe_stack.enter_context(cmd_pipe_proc_reader._manage_terminal())
33993399

34003400
# Popen was called with shell=True so the user can chain pipe commands and redirect their output
34013401
# like: !ls -l | grep user | wc -l > out.txt. But this makes it difficult to know if the pipe process started
34023402
# OK, since the shell itself always starts. Therefore, we will wait a short time and check if the pipe process
34033403
# is still running.
34043404
with contextlib.suppress(subprocess.TimeoutExpired):
3405-
if cmd_pipe_proc_reader is None:
3405+
if terminal_fd is None:
34063406
proc.wait(0.2)
34073407
else:
34083408
# Open the start gate only once the pipeline owns the terminal.
@@ -3411,14 +3411,11 @@ def _redirect_output(self, statement: Statement) -> utils.RedirectionSavedState:
34113411
os.write(new_stdout.fileno(), b"\n")
34123412
cmd_pipe_proc_reader._wait_for_exit(0.2)
34133413

3414-
# Check if the pipe process already exited
3414+
# Check if the pipe process already exited. Pass on what it wrote to a pipe, such as why it failed.
34153415
if proc.returncode is not None:
3416-
if cmd_pipe_proc_reader is not None:
3417-
cmd_pipe_proc_reader.wait()
3416+
cmd_pipe_proc_reader.wait()
34183417
raise RedirectionError(f"Pipe process exited with code {proc.returncode} before command could run")
34193418
redir_saved_state.redirecting = True
3420-
if cmd_pipe_proc_reader is None:
3421-
cmd_pipe_proc_reader = utils.ProcReader(proc, self.stdout, sys.stderr)
34223419

34233420
if terminal_fd is not None:
34243421
import io

‎tests/test_cmd2.py‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -982,6 +982,7 @@ def test_pipe_to_shell_error(redirection_app, mocker, capsys, terminal) -> None:
982982
process = popen.return_value
983983
process.returncode = 127
984984
process.wait.return_value = 127
985+
reader = mocker.patch("cmd2.utils.ProcReader").return_value
985986
if terminal:
986987
terminal_stream = mocker.Mock()
987988
terminal_stream.isatty.return_value = True
@@ -990,7 +991,6 @@ def test_pipe_to_shell_error(redirection_app, mocker, capsys, terminal) -> None:
990991
mocker.patch("os.tcgetpgrp", return_value=os.getpgrp())
991992
mocker.patch("os.getsid", return_value=os.getpgrp())
992993
sigmask = mocker.patch("signal.pthread_sigmask", return_value=set())
993-
reader = mocker.patch("cmd2.utils.ProcReader").return_value
994994
previous_tstp = signal.getsignal(signal.SIGTSTP)
995995

996996
def start_pipe(*args, **kwargs):
@@ -1011,10 +1011,11 @@ def start_pipe(*args, **kwargs):
10111011
assert not out
10121012
assert "Pipe process exited with code 127 before command could run" in " ".join(err)
10131013
assert capsys.readouterr().out == ""
1014+
# The pipe process is reaped, and what it wrote to a pipe passed on.
1015+
reader.wait.assert_called_once_with()
10141016
if terminal:
10151017
assert signal.getsignal(signal.SIGTSTP) == previous_tstp
10161018
reader._wait_for_exit.assert_called_once_with(0.2)
1017-
reader.wait.assert_called_once_with()
10181019
process.wait.assert_not_called()
10191020
# SIGTTOU is blocked only inside ProcReader's lends. Blocking it for the whole pipeline would leak the mask into every
10201021
# child the command starts.
@@ -1024,6 +1025,26 @@ def start_pipe(*args, **kwargs):
10241025
assert popen.call_args.kwargs["stdin"].closed
10251026

10261027

1028+
def test_pipe_to_shell_error_keeps_what_the_pipe_process_wrote(redirection_app, mocker) -> None:
1029+
"""A pipe process that exits before the command runs may say why, as a shell does for a command it cannot find.
1030+
1031+
With its output captured, as in a pyscript, that must reach the captured stderr too.
1032+
"""
1033+
real_popen = subprocess.Popen
1034+
1035+
def popen(*args, **kwargs):
1036+
process = real_popen(*args, **kwargs)
1037+
# Exit before the startup check, however loaded the system is
1038+
process.wait()
1039+
return process
1040+
1041+
mocker.patch("subprocess.Popen", popen)
1042+
out, err = run_cmd(redirection_app, "print_output | echo pipe output && cmd2_test_no_such_command")
1043+
assert out == ["pipe output"]
1044+
assert "cmd2_test_no_such_command" in " ".join(err)
1045+
assert "before command could run" in err[-1]
1046+
1047+
10271048
@pytest.mark.skipif(sys.platform == "win32", reason="POSIX terminal job control")
10281049
@pytest.mark.parametrize("android", [False, True])
10291050
def test_terminal_pipe_start_gate_uses_the_platform_shell(redirection_app, mocker, monkeypatch, android) -> None:

0 commit comments

Comments
 (0)