Skip to content

fix: exit 0 when a downstream reader closes stdout early#1811

Closed
varuntej07 wants to merge 1 commit into
Graphify-Labs:v8from
varuntej07:fix-broken-pipe-exit
Closed

fix: exit 0 when a downstream reader closes stdout early#1811
varuntej07 wants to merge 1 commit into
Graphify-Labs:v8from
varuntej07:fix-broken-pipe-exit

Conversation

@varuntej07

Copy link
Copy Markdown
Contributor

Problem

Piping graphify into a reader that stops early (head, PowerShell
Select-Object -First N, a pager that quits) closes stdout before we finish
writing. The interpreter's shutdown flush then hits the closed pipe and crashes:

Exception ignored on flushing sys.stdout:
OSError: [Errno 22] Invalid argument

The process exits nonzero (120 on POSIX). A downstream head closing the pipe
is normal, so graphify should exit 0 the way foo | head does.

Fix

Wrap the CLI entry point: catch BrokenPipeError and OSError with EINVAL or
EPIPE (Windows reports a closed pipe as EINVAL), redirect stdout to devnull
so the shutdown flush stays quiet, and exit 0. Unrelated OSErrors still
propagate, so real failures such as a missing graph.json are not masked.

Verification

A reader that reads one line then closes the pipe:

before: returncode=120, "Exception ignored on flushing sys.stdout"
after:  returncode=0,   clean

Adds tests/test_broken_pipe_exit.py (5 tests covering the exit-code contract,
including that non-pipe errors still propagate). Existing CLI suites pass.

@safishamsi

Copy link
Copy Markdown
Collaborator

Thanks @varuntej07 — this shipped as part of the #1807 fix in 0.9.14. Your explicit in-guard sys.stdout.flush() was the important nuance my first pass missed (block-buffered piped stdout only flushing at shutdown, outside the guard), so I adopted it in a46eee4 and credited you. Also handled the Windows OSError(EINVAL) form per your note. Really appreciate the well-reasoned PR.

@safishamsi safishamsi closed this Jul 12, 2026
@varuntej07

Copy link
Copy Markdown
Contributor Author

Thanks for the review and the credit @safishamsi. Glad the explicit flush helped close that edge case cleanly. Appreciate you building Graphify, keep pushing! More tasks, fewer tokens!!

safishamsi added a commit that referenced this pull request Jul 13, 2026
#1831 — `graphify export graphml` crashed on any dict/list-valued
attribute (per-node metadata dict, graph-level hyperedges list) because
nx.write_graphml only accepts scalars; a real ~2,300-node graph failed
every export and left a 0-byte .graphml behind. to_graphml now coerces
None->"" and JSON-serializes non-scalars across graph/node/edge scopes
(int/float/bool/str pass through), and writes atomically via a temp file
so a failed export can't leave a partial file. Closes #1830.

#1807 followup — adopt @varuntej07's explicit in-guard sys.stdout.flush()
from #1811: piped stdout is block-buffered, so a small fully-buffered
output would only flush at interpreter shutdown (outside the guard),
where a closed-pipe reader escapes as a noisy shutdown error and nonzero
exit. Flushing inside the try closes that gap. Closes #1811.

Reported by @hofmockel (#1831) and @varuntej07 (#1807/#1811).

Co-Authored-By: hofmockel <hofmockel@users.noreply.github.com>
Co-Authored-By: varuntej07 <varuntej07@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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