Skip to content

fix(audit): exit silently on broken pipe#337

Merged
fohte merged 3 commits into
mainfrom
fohte/fix-audit-broken-pipe
May 4, 2026
Merged

fix(audit): exit silently on broken pipe#337
fohte merged 3 commits into
mainfrom
fohte/fix-audit-broken-pipe

Conversation

@fohte
Copy link
Copy Markdown
Owner

@fohte fohte commented May 4, 2026

Purpose

  • Piping runok audit --json | head -1 (or similar consumers that close stdout early) makes runok panic with failed printing to stdout: Broken pipe (os error 32), so a Rust backtrace is printed to stderr next to the JSON line

Reproduction steps

  1. With at least one audit log entry recorded, run runok audit --json | head -1
  2. One JSON line is printed, then the following panic backtrace lands on stderr
    thread 'main' panicked at library/std/src/io/stdio.rs:1165:9:
    failed printing to stdout: Broken pipe (os error 32)
    

Approach

  • On Unix, restore the default SIGPIPE handler (SIG_DFL) at the start of main so the process exits silently on EPIPE
    • The Rust standard library installs SIG_IGN for SIGPIPE at startup, which turns pipe writes into io::Error and makes println! panic on the failure
# Before: backtrace is printed to stderr
$ runok audit --json | head -1
{"timestamp":...}
thread 'main' panicked at library/std/src/io/stdio.rs:1165:9:
failed printing to stdout: Broken pipe (os error 32)

# After: exits silently
$ runok audit --json | head -1
{"timestamp":...}
Design decisions

How to handle EPIPE

Decision Design Pros Cons
Chosen Restore SIGPIPE to SIG_DFL at the start of main A single syscall covers every stdout/stderr writer in the binary Requires an unsafe libc call
Rejected Inspect the return value of writeln! per call site and swallow BrokenPipe Cross-platform and needs no unsafe The same handling has to be duplicated at every output site, and any future output path that forgets it brings the panic back

fohte added 2 commits May 4, 2026 15:16
intent(audit): make `runok audit --json | head -1` and similar pipelines
  match Unix conventions (e.g. `yes | head`) instead of surfacing a Rust
  panic with `failed printing to stdout: Broken pipe (os error 32)`.
decision(main): restore SIG_DFL for SIGPIPE at process startup so EPIPE
  terminates the process quietly. Covers every stdout/stderr writer at
  once (audit --json, config-schema, future commands) with one syscall.
rejected(audit): catching `ErrorKind::BrokenPipe` per call site — would
  need to be repeated for every `println!` in the binary and would not
  protect future output paths added later.
constraint(main): only applied on `cfg(unix)`; Windows has no SIGPIPE.
learned(rust): the standard library installs SIG_IGN for SIGPIPE at
  startup, which converts pipe writes into EPIPE — the `println!` macro
  then panics on the resulting `io::Error`. The upstream Rust issue
  tracking this is still open, so the fix has to live in the
  application.
…mber

intent(docs): satisfy the project rule that next.md entries must carry a
  resolvable PR link by the time the entry merges.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.73%. Comparing base (23cc3e7) to head (a78e4c7).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #337      +/-   ##
==========================================
- Coverage   88.74%   88.73%   -0.02%     
==========================================
  Files          53       53              
  Lines       12163    12168       +5     
==========================================
+ Hits        10794    10797       +3     
- Misses       1369     1371       +2     
Flag Coverage Δ
Linux 88.55% <100.00%> (-0.02%) ⬇️
macOS 89.91% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes a panic that occurs when runok audit --json is piped into a consumer that closes stdout early, such as head. The fix involves restoring the default SIGPIPE handler on Unix systems to ensure the process terminates silently on EPIPE. The changes include a new regression test and an update to the release notes. Feedback includes replacing a placeholder link in the documentation and optimizing string construction in the test suite by using String::with_capacity and writeln! to avoid inefficient repeated allocations.

Comment thread docs/src/content/docs/releases/next.md
Comment thread tests/e2e/audit.rs Outdated
…ed buffer

intent(audit): keep test setup cheap by avoiding 5000 short-lived `format!`
  allocations when assembling the ~5 MiB JSONL payload used to overflow the
  pipe buffer.
@fohte fohte merged commit 1feddf1 into main May 4, 2026
10 checks passed
@fohte fohte deleted the fohte/fix-audit-broken-pipe branch May 4, 2026 07:41
@fohte-bot fohte-bot Bot mentioned this pull request May 4, 2026
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.

1 participant