Skip to content

Fix BrokenPipeError traceback when piping CLI output - #7354

Open
mvanhorn wants to merge 1 commit into
cylc:8.6.xfrom
mvanhorn:fix/7247-brokenpipeerror
Open

Fix BrokenPipeError traceback when piping CLI output#7354
mvanhorn wants to merge 1 commit into
cylc:8.6.xfrom
mvanhorn:fix/7247-brokenpipeerror

Conversation

@mvanhorn

Copy link
Copy Markdown

Summary

Adds a BrokenPipeError handler to the central CLI decorator (cli_function in cylc/flow/terminal.py) so that piping a Cylc command into a consumer that closes the pipe early exits quietly instead of dumping a Python traceback.

Fixes #7247

Why this matters

As reported in #7247, running something like:

cylc config <flow> | less    # then quit less
cylc config <flow> | grep -q '\w'

prints an ugly BrokenPipeError: [Errno 32] Broken pipe traceback to stderr. It is harmless (the command still works) but looks broken, and it has existed since at least 8.3.5. The reporter confirmed it reproduces every time with grep -q '\w'. The root cause is that the central CLI wrapper, which already converts "known" errors into clean exits, did not handle BrokenPipeError raised when writing to a closed stdout pipe.

Putting the fix in cli_function means every Cylc subcommand benefits, not just cylc config.

Changes

  • cylc/flow/terminal.py: add except BrokenPipeError to the cli_function wrapper. It follows the standard-library-recommended pattern of redirecting the remaining stdout to os.devnull (to avoid a second BrokenPipeError when Python flushes buffered stdout at interpreter shutdown) and then exits with status 1. The redirect is guarded so it degrades gracefully if stdout is not backed by a real file descriptor. A broken pipe is never user-actionable, so the traceback is suppressed at all verbosity levels (distinct from the "unknown error" path, which intentionally shows a traceback).
  • tests/unit/test_terminal.py: add test_cli_broken_pipe verifying the wrapper converts BrokenPipeError into a clean SystemExit(1) with no traceback on stderr, at both normal and debug verbosity.
  • changes.d/7247.fix.md: towncrier changelog fragment.
  • CONTRIBUTING.md: add contributor name (CLA).

Testing

  • pytest tests/unit/test_terminal.py — all 16 cases pass (the 8 existing test_cli cases for CylcError/ParsecError/Exception/SystemExit are unaffected, plus the 2 new broken-pipe cases).
  • flake8 cylc/flow/terminal.py tests/unit/test_terminal.py — clean.
  • End-to-end: with a command that writes a large stream through cli_function piped into a reader that closes early, the unpatched code prints a multi-line Traceback ... BrokenPipeError to stderr; the patched code exits cleanly with no traceback.

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present). — N/A, no dependency changes.
  • Tests are included (or explain why this is not needed).
  • Changelog entry included (changes.d/7247.fix.md).
  • CONTRIBUTING.md updated if it describes a relevant change to development workflow. — N/A.

@MetRonnie

Copy link
Copy Markdown
Member

Thanks for you contribution. Unfortunately we've got quite the backlog so it may take us a while to get round to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants