fix(claude-code hook): fail open on closed stdout instead of logging Broken pipe (WEB-4745)#122
fix(claude-code hook): fail open on closed stdout instead of logging Broken pipe (WEB-4745)#122vigneshsubbiah16 wants to merge 1 commit into
Conversation
…Broken pipe (WEB-4745) Claude Code closes the hook's stdout as soon as it has the response it needs (or when it times the hook out). The hook's bare print() calls then raised BrokenPipeError, which dominated error.log with "Exception in main: Broken pipe" — burying the failures we actually care about (API timeouts) — and the except handler re-print()ed on the same dead pipe, risking a double fault. Separately, the interpreter's shutdown-time stdout flush re-hit the dead pipe, printing "Exception ignored ... BrokenPipeError" and exiting non-zero. - Add pipe-safe _emit(); route all main() stdout writes through it. - Catch BrokenPipeError in main() (no noisy log, no re-print). - Apply the standard CPython SIGPIPE recipe at the entry point so the shutdown flush is a no-op and the hook exits cleanly. - Add test_hook_io.py: subprocess-level tests asserting clean exit, no traceback, no Broken-pipe log line, and that the normal path still emits valid JSON. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| except BrokenPipeError: | ||
| # Claude Code closed our stdout before we finished — nothing to report | ||
| # and nothing to write. Swallow it so it doesn't become error.log noise | ||
| # (and so we don't re-raise by trying to write again). See WEB-4745. | ||
| pass |
There was a problem hiding this comment.
Overly-broad
BrokenPipeError handler silently swallows non-stdout pipe failures
The except BrokenPipeError: pass wraps the entire main() body, not just the _emit() calls. If any processing code inside the try block — such as a subprocess pipe used in process_pre_tool_use (the PR description references a curl-based HTTP call in that path) — raises BrokenPipeError, the error is silently discarded with no log entry. The intent is clearly to ignore stdout pipe closure, but the handler is not scoped to stdout. A subprocess pipe failure mid-request would be treated identically to a closed stdout and leave no diagnostic trace in error.log.
| rc = proc.wait(timeout=30) | ||
| err = proc.stderr.read().decode() | ||
| out = b"" if close_stdout else proc.stdout.read() | ||
| proc.stderr.close() |
There was a problem hiding this comment.
Potential deadlock when reading
stderr after proc.wait() with stdout closed
proc.wait() blocks until the subprocess exits. If the subprocess writes enough to stderr to fill the OS pipe buffer (~64 KB) before it has consumed all of stdin, both sides will block: the subprocess trying to write stderr, the test waiting for the subprocess to exit. In this hook the risk is low, but the safer pattern is proc.communicate() (with stdin pre-supplied) or draining stderr concurrently via a thread, rather than sequentially waiting then reading.
What
The Claude Code hook (
claude-code/hooks/unbound.py) was flooding~/.claude/hooks/error.logwithException in main: [Errno 32] Broken pipe(~18 of ~25 lines on a dogfood machine over two weeks). This makes the one log we tell support to read useless for spotting the failures that matter (API timeouts) — directly undermining the self-diagnosing goal of the Coding Telemetry Reliability project.Closes WEB-4745.
Root cause
Claude Code closes the hook's stdout as soon as it has the response it needs (or when it times the hook out). The hook's bare
print(..., flush=True)calls then raiseBrokenPipeError:except Exceptioninmain()→ logged asException in main: Broken pipe(noise).print()ed again on the same dead pipe → potential double fault.Exception ignored ... BrokenPipeErroron stderr + non-zero exit.A closed pipe is not an actionable error — the response is simply moot. The hook should fail open silently.
Changes
_emit()helper and route everymain()stdout write through it (BrokenPipeError/OSError→ silent no-op).except BrokenPipeErrorinmain()so a dead pipe is neither logged nor re-printed.test_hook_io.py: subprocess-level tests (outermost layer) asserting, on a closed stdout, clean exit 0 / no traceback / noBroken pipelog line, and that the normal path still emits valid JSON.Test plan
python3 -m pytest claude-code/hooks/test_hook_io.py→ 3 passed.test_identity.py::...test_keys_limited_to_identity_fields) is pre-existing onmainand unrelated to this change (build_account_identityreturningdevice_serial/user_email).PreToolUseJSON and fails open.Scope note
This is the safe, behavior-preserving slice. The related security findings — API key in curl argv / leaked into
error.log(WEB-4748, WEB-4734) — and the 20s synchronous-call stall (WEB-4746) are not included here; they change the HTTP mechanism / enforcement timing and should land separately with live-gateway validation.🤖 Generated with Claude Code
Greptile Summary
This PR fixes
BrokenPipeErrornoise that dominated~/.claude/hooks/error.logwhen Claude Code closed the hook's stdout early. It introduces a_emit()helper that silently absorbs pipe errors on every write, adds an explicitexcept BrokenPipeErrorinmain(), and applies the standard CPython SIGPIPE recipe at the entry point so the interpreter's shutdown flush is also a no-op._emit()helper (unbound.py): wraps everysys.stdout.write+ flush in a(BrokenPipeError, OSError)guard, replacing all bareprint()calls throughoutmain().main()returns,os.dup2(os.devnull, stdout_fd)is called in afinallyblock so the interpreter's own shutdown flush lands on/dev/nullrather than a dead pipe.test_hook_io.py: three new subprocess-level tests assert clean exit (rc=0), no traceback on stderr, noBroken pipeinerror.log, and valid JSON emission on the normal path.Confidence Score: 4/5
Safe to merge — the change is narrowly scoped to swallowing stdout pipe errors and adds no new logic to the hook's processing path.
The fix is correct and the tests cover the advertised scenarios. Two minor concerns: the
except BrokenPipeError: passblock inmain()wraps the entire processing body, so a pipe failure inside a subprocess called byprocess_pre_tool_usewould be silently dropped rather than logged; and the test'sproc.wait()/proc.stderr.read()ordering is susceptible to a deadlock if stderr output ever grows large. Neither is a current defect for this hook in practice.Both changed files are small and self-contained.
unbound.py's newexcept BrokenPipeErrorhandler is worth a second look to confirm it cannot mask real subprocess pipe failures inprocess_pre_tool_use.Important Files Changed
_emit()pipe-safe helper and replaces all bareprint()calls; addsexcept BrokenPipeErrorin main() and applies CPython SIGPIPE recipe at entry point — correct approach, but the BrokenPipeError handler scope is wider than stdout alone.proc.wait()beforeproc.stderr.read()is susceptible to a theoretical deadlock.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Claude Code spawns hook] --> B[main reads stdin] B --> C{Parse JSON event} C -- invalid --> D[_emit suppressOutput] C -- PreToolUse --> E[process_pre_tool_use] E --> F[_emit response] C -- SessionStart --> G[warm caches and dispatch] G --> H[_emit empty object] C -- other --> I[append_to_audit_log] I --> J[_emit suppressOutput] D --> K{stdout open?} F --> K H --> K J --> K K -- yes --> L[write and flush succeeds] K -- no --> M[BrokenPipeError swallowed silently] B -- BrokenPipeError --> N[except BrokenPipeError pass] B -- other Exception --> O[log_error and _emit fallback] L --> P[main returns] M --> P N --> P O --> P P --> Q[sys.stdout.flush in __main__] Q -- BrokenPipeError --> R[except BrokenPipeError pass] Q -- ok --> S[finally: dup2 stdout to devnull] R --> S S --> T[interpreter shutdown flush to devnull exit 0]Reviews (1): Last reviewed commit: "fix(claude-code hook): fail open on clos..." | Re-trigger Greptile