Skip to content

[WIP] Add a new method Session.run_capture() which allows to capture stdout and stderr separately, even while showing the result - #1124

Draft
felixfontein wants to merge 5 commits into
wntrblm:mainfrom
felixfontein:capture
Draft

Conversation

@felixfontein

Copy link
Copy Markdown
Contributor

Session.run_capture()'s interface is similar to Session.run(), except:

  • It always returns tuple[str, str, int] (unless the command isn't run, then it returns None) with return code, stdout, and stderr;
  • If silent=True is specified and the program exits abnormally, stderr is shown; if silent=False (default), all stdout and stderr is shown in near real-time (similar to tee, you catch the output while the user can still see it);
  • There is a success_all: bool parameter which, when set to True, does not fail on any return code; that allows the caller to completely handle the return code, without having to provide an exhaustive list of possible return codes for success_codes.

The implementation of silent=False was somewhat tricky. I found https://stackoverflow.com/questions/5045771/python-how-to-prevent-subprocesses-from-receiving-ctrl-c-control-c-sigint and the resulting https://github.com/pycontribs/subprocess-tee/blob/main/src/subprocess_tee/__init__.py implementation, but that didn't work as I expected in particular with respect to KeyboardInterrupt. I had to add a SIGINT signal handler and cancel tasks myself. (By default SIGINT kills the event loop, and not even finally blocks in tasks are handled... You can only catch KeyboardInterrupt completely outside the loop.run_until_complete() call.)

I also considered not using asyncio, but selecting on stdout/stderr, i.e. basically reimplementing Popen.communicate(). But then I looked closer in how Popen.communicate() is implemented in the standard library, and decided it's way too complex to try to do it this way, especially when it also has to work on Windows.

This is a work in progress since it doesn't have tests yet. Also I only did a few tests on Linux (I don't have access to Windows, but can do some manual testing on macOS later). Before I spent even more time I'm also curious on whether this has a chance of being merged :)

Fixes #1053.

@henryiii

henryiii commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Edit: ahh, sorry, too many tabs open, got comments mixed up. I tried something new here, I had Claude Fable plan out what it would do, but base it on this PR, so it has two comments you can look at. Should I push them here or make a PR to your fork so you can let me know what you think?

@felixfontein

Copy link
Copy Markdown
Contributor Author

I guess both is fine for me; whatever you prefer. Pushing here has the advantage of all discussion happening here (and not also in PRs in other repos.)

Comment thread nox/popen.py Outdated
Read a stream chunk by chunk, append it to ``out_buffer``, and write it to ``out_stream``.
"""
while True:
chunk = await stream.readline()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stream.readline() will fail if the line is too long. See ansible-community/antsibull-core@d910be2.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I've re-implemented that here in 3c6064f. The main difference to your implementation is that it has a lot of comments that explain what exactly is happening, without having to resort to the docs or asyncio.streams' sources.

@felixfontein
felixfontein force-pushed the capture branch 3 times, most recently from e19afd9 to a909bd2 Compare July 18, 2026 11:41
@felixfontein

Copy link
Copy Markdown
Contributor Author

I tried to debug the failing tests a bit on a Macbook; there the tests pass if the tests are run sequentially (without --numprocesses=auto), but some always fail when things are run in parallel. The "crashed" workers are due to KeyboardInterrupted escaping from tests; adding try/except KeyboardInterrupt as exc: raise ValueError("Caught KeyboardInterrupt") from exc around all of them avoided the "crashed" workers, but still had the other problems.

I don't have access to Windows, so no idea how to work around the NotImplementedError there...

felixfontein and others added 5 commits August 16, 2026 09:22
… and stderr separately, even while showing the result.
Co-authored-by: Maxwell G <maxwell@gtmx.me>
This fixes issues with the tests (no more "unclosed event loops"
errors), and probably allows tee_popen() to run in parallel.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Allow session.run(quiet=True) to return stdout and stderr separately

3 participants