From 889b8d90f32b8a2ab8aa5b1d7ce1e3ade5a1ec17 Mon Sep 17 00:00:00 2001 From: Vignesh Subbiah Date: Tue, 9 Jun 2026 18:19:06 -0700 Subject: [PATCH] fix(claude-code hook): fail open on closed stdout instead of logging Broken pipe (WEB-4745) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- claude-code/hooks/test_hook_io.py | 80 +++++++++++++++++++++++++++++++ claude-code/hooks/unbound.py | 61 ++++++++++++++++++----- 2 files changed, 130 insertions(+), 11 deletions(-) create mode 100644 claude-code/hooks/test_hook_io.py diff --git a/claude-code/hooks/test_hook_io.py b/claude-code/hooks/test_hook_io.py new file mode 100644 index 00000000..6693b72e --- /dev/null +++ b/claude-code/hooks/test_hook_io.py @@ -0,0 +1,80 @@ +import json +import os +import subprocess +import sys +import unittest +from pathlib import Path + +HOOK = Path(__file__).resolve().parent / "unbound.py" + + +class TestHookBrokenPipe(unittest.TestCase): + """The hook must fail open when Claude Code closes its stdout early. + + Claude closes the hook's pipe as soon as it has the response it needs (or + when it times the hook out). Writing to that closed pipe used to raise + BrokenPipeError, which (a) flooded error.log with "Exception in main: + Broken pipe" and (b) made the interpreter print "Exception ignored ... + BrokenPipeError" and exit non-zero at shutdown. See WEB-4745. + + These run the real hook as a subprocess so they exercise the actual + stdout/stderr/exit-code behavior Claude Code sees. + """ + + EVENT = json.dumps({ + "hook_event_name": "PreToolUse", + "tool_name": "Bash", + "tool_input": {"command": "echo hi"}, + "session_id": "test-broken-pipe", + }).encode() + + def _run(self, close_stdout): + proc = subprocess.Popen( + [sys.executable, str(HOOK)], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + # Point error.log at a throwaway dir so the test never touches the + # developer's real ~/.claude/hooks/error.log. + env={**os.environ, "HOME": str(Path(self._tmp))}, + ) + if close_stdout: + proc.stdout.close() + try: + proc.stdin.write(self.EVENT) + proc.stdin.close() + except BrokenPipeError: + pass + rc = proc.wait(timeout=30) + err = proc.stderr.read().decode() + out = b"" if close_stdout else proc.stdout.read() + proc.stderr.close() + return rc, out, err + + def setUp(self): + import tempfile + self._tmpdir = tempfile.TemporaryDirectory() + self._tmp = self._tmpdir.name + self.addCleanup(self._tmpdir.cleanup) + + def test_closed_stdout_exits_cleanly_with_no_traceback(self): + rc, _out, err = self._run(close_stdout=True) + self.assertEqual(rc, 0, f"expected clean exit on closed pipe, got {rc}") + self.assertNotIn("BrokenPipeError", err) + self.assertNotIn("Traceback", err) + + def test_closed_stdout_does_not_log_broken_pipe(self): + self._run(close_stdout=True) + log = Path(self._tmp) / ".claude" / "hooks" / "error.log" + contents = log.read_text() if log.exists() else "" + self.assertNotIn("Broken pipe", contents) + + def test_open_stdout_still_emits_valid_json(self): + rc, out, _err = self._run(close_stdout=False) + self.assertEqual(rc, 0) + parsed = json.loads(out.decode()) + self.assertIn("suppressOutput", parsed) + + +if __name__ == "__main__": + unittest.main() diff --git a/claude-code/hooks/unbound.py b/claude-code/hooks/unbound.py index 58f9f15c..0b8c1f00 100644 --- a/claude-code/hooks/unbound.py +++ b/claude-code/hooks/unbound.py @@ -131,6 +131,24 @@ def log_error(message: str, category: str = 'general'): report_error_to_gateway(message, category, _cached_api_key) +def _emit(payload) -> None: + """Write a hook response to stdout, failing open if Claude Code has already + closed the pipe. + + Claude closes the hook's stdout as soon as it has the response it needs (or + when it moves on / times the hook out). A subsequent write then raises + BrokenPipeError. That is not an actionable error — the response is simply + moot — so we swallow it silently instead of logging noise. Previously these + writes used bare print() and the BrokenPipeError surfaced as the dominant + line in error.log (see WEB-4745).""" + try: + text = payload if isinstance(payload, str) else json.dumps(payload) + sys.stdout.write(text + "\n") + sys.stdout.flush() + except (BrokenPipeError, OSError): + pass + + def _read_policy_cache_raw() -> Optional[Dict]: """Read and JSON-parse the policy cache file. Returns None on missing/corrupt.""" try: @@ -1512,15 +1530,15 @@ def main(): try: input_data = sys.stdin.read().strip() - + if not input_data: - print('{"suppressOutput": true}', flush=True) + _emit('{"suppressOutput": true}') return try: event = json.loads(input_data) except json.JSONDecodeError: - print('{"suppressOutput": true}', flush=True) + _emit('{"suppressOutput": true}') return hook_event_name = event.get('hook_event_name') @@ -1531,7 +1549,7 @@ def main(): _device_serial() # warm the (slow) serial probe + cache once per session _check_self_update() _dispatch_discovery() - print("{}") + _emit("{}") return session_id = event.get('session_id') @@ -1539,7 +1557,7 @@ def main(): if hook_event_name == 'PreToolUse': response = process_pre_tool_use(event, api_key) response["suppressOutput"] = True - print(json.dumps(response), flush=True) + _emit(response) return # Handle UserPromptSubmit - check policy before processing @@ -1554,7 +1572,7 @@ def main(): 'event': event }) response["suppressOutput"] = True - print(json.dumps(response), flush=True) + _emit(response) return # If allowed, continue to log the event (output printed at end) @@ -1573,13 +1591,34 @@ def main(): cleanup_old_logs() - print('{"suppressOutput": true}', flush=True) - + _emit('{"suppressOutput": true}') + + 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 except Exception as e: - # Still return empty JSON object to Claude Code to indicate completion + # Still return empty JSON object to Claude Code to indicate completion. + # _emit() is pipe-safe, so a dead pipe here won't trigger a second fault. log_error(f"Exception in main: {str(e)}", 'general') - print('{"suppressOutput": true}', flush=True) + _emit('{"suppressOutput": true}') if __name__ == '__main__': - main() \ No newline at end of file + # Fail open on a closed stdout. Claude Code may close the hook's pipe before + # we finish writing; _emit() already swallows BrokenPipeError on each write, + # but the interpreter also flushes stdout at shutdown, which would re-raise + # against the dead pipe and print "Exception ignored ... BrokenPipeError" + # plus exit non-zero. Redirect stdout to devnull so that final flush is a + # no-op and the hook exits cleanly. (Standard CPython SIGPIPE recipe.) See WEB-4745. + try: + main() + sys.stdout.flush() + except BrokenPipeError: + pass + finally: + try: + os.dup2(os.open(os.devnull, os.O_WRONLY), sys.stdout.fileno()) + except OSError: + pass \ No newline at end of file