Skip to content

fix: handle broken pipes during the final stdout flush - #132

Merged
mkaz merged 2 commits into
mkaz:mainfrom
GreedyC:codex/114-handle-closed-pipes
Oct 26, 2026
Merged

mkaz merged 2 commits into
mkaz:mainfrom
GreedyC:codex/114-handle-closed-pipes

Conversation

@GreedyC

@GreedyC GreedyC commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #114.

The existing BrokenPipeError handler catches drawing failures, but buffered output can still fail when Python flushes stdout during interpreter shutdown. A closed pipe then prints an ignored exception and exits with code 120.

Run the complete CLI, including argument parsing and early exits, inside the protected block and flush stdout in a finally block. On a broken pipe, redirect its descriptor to os.devnull, close the extra descriptor, and exit with code 1. This follows the Python documentation's SIGPIPE guidance, and preserves normal output.

Validation

  • Four real subprocess/pipe regressions cover small and large charts, --version, and --help. The two additional early-exit cases fail against the initial chart-only fix and pass after moving the full CLI into the protected block. Initial chart regressions: a small buffered chart and a large chart. Both fail against the old production code with code 120 and stderr; both pass with exit 1 and empty stderr after the fix.
  • Full suite: 31 passed on Python 3.14 and the minimum supported Python 3.9.
  • A normal CLI run still succeeds and prints its chart.
  • Mypy: no issues in eight source files. Source and wheel builds pass.
  • Repository-wide Ruff currently reports 65 existing diagnostics; the filename/code/message multiset is identical with the production fix reverted. The new tests pass their own Ruff check. This is not a clean repository-wide lint result.
  • git diff --check passes; Copilot pre-review found no introduced defect.

Validated on macOS arm64; Windows execution was not tested. No signal-handler changes or dependencies are introduced.

@mkaz
mkaz merged commit 744f76f into mkaz:main Oct 26, 2026
1 check failed
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.

Piping to head produces BrokenPipeError

2 participants