Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
cli: Handle broken pipe (SIGPIPE) gracefully
Fintan Halpenny committed 1 month ago
commit 07ddc37aff7775da8685fe03496a9873c594c744
parent 9ed9882acd809e69aa4110979901c93590283679
13 files changed +96 -36
modified crates/radicle-cli/src/commands/cob.rs
@@ -80,7 +80,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
                    oid
                }
            };
-
            println!("{oid}");
+
            term::print(oid);
        }
        Migrate => {
            let mut db = profile.cobs_db_mut()?;
@@ -100,7 +100,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
                FilteredTypeName::from(type_name).as_ref(),
            )?;
            for cob in cobs {
-
                println!("{}", cob.id);
+
                term::print(cob.id);
            }
        }
        Log {
@@ -215,7 +215,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
                }
            };

-
            println!("{oid}");
+
            term::print(oid);
        }
    }
    Ok(())
modified crates/radicle-cli/src/commands/cob/args.rs
@@ -400,7 +400,10 @@ mod test {
            .chain(ARGS.iter())
            .collect::<Vec<_>>(),
        );
-
        println!("{args:?}");
+
        #[allow(clippy::print_stdout)]
+
        {
+
            println!("{args:?}");
+
        }
        assert!(args.is_ok())
    }

modified crates/radicle-cli/src/commands/debug.rs
@@ -60,7 +60,7 @@ fn debug(profile: Option<&Profile>) -> anyhow::Result<()> {
        warnings: collect_warnings(profile),
    };

-
    println!("{}", serde_json::to_string_pretty(&debug).unwrap());
+
    term::print(serde_json::to_string_pretty(&debug).unwrap());

    Ok(())
}
modified crates/radicle-cli/src/commands/id.rs
@@ -378,7 +378,7 @@ fn print(
    profile: &Profile,
) -> anyhow::Result<()> {
    print_meta(revision, previous, profile)?;
-
    println!();
+
    term::blank();
    print_diff(revision.parent.as_ref(), &revision.id, repo)?;

    Ok(())
@@ -519,7 +519,7 @@ fn print_diff(

    if let Some(modified) = diff.modified().next() {
        let diff = modified.diff.to_unified_string()?;
-
        print!("{diff}");
+
        term::print_inline(diff);
    } else {
        term::print(term::format::italic("No changes."));
    }
modified crates/radicle-cli/src/commands/inspect.rs
@@ -70,7 +70,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
                let refs = RefsAt::new(&repo, remote)?;
                let sigrefs = SignedRefs::load_at(refs.at, remote, &repo);

-
                println!(
+
                term::print(format_args!(
                    "{:<48} {} {}",
                    term::format::tertiary(remote.to_human()),
                    term::format::secondary(refs.at),
@@ -105,7 +105,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
                            term::format::negative("missing".to_string())
                        }
                    }
-
                );
+
                ));
            }
        }
        Target::Policy => {
@@ -113,19 +113,19 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
            let seed = policies.seed_policy(&rid)?;
            match seed.policy {
                SeedingPolicy::Allow { scope } => {
-
                    println!(
+
                    term::print(format_args!(
                        "Repository {} is {} with scope {}",
                        term::format::tertiary(&rid),
                        term::format::positive("being seeded"),
                        term::format::dim(format!("`{scope}`"))
-
                    );
+
                    ));
                }
                SeedingPolicy::Block => {
-
                    println!(
+
                    term::print(format_args!(
                        "Repository {} is {}",
                        term::format::tertiary(&rid),
                        term::format::negative("not being seeded"),
-
                    );
+
                    ));
                }
            }
        }
@@ -134,19 +134,19 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
            let aliases = profile.aliases();
            for did in doc.delegates().iter() {
                if let Some(alias) = aliases.alias(did) {
-
                    println!(
+
                    term::print(format_args!(
                        "{} {}",
                        term::format::tertiary(&did),
                        term::format::parens(term::format::dim(alias))
-
                    );
+
                    ));
                } else {
-
                    println!("{}", term::format::tertiary(&did));
+
                    term::print(term::format::tertiary(&did));
                }
            }
        }
        Target::Visibility => {
            let (_, doc) = repo(rid, storage)?;
-
            println!("{}", term::format::visibility(doc.visibility()));
+
            term::print(term::format::visibility(doc.visibility()));
        }
        Target::History => {
            let (repo, _) = repo(rid, storage)?;
@@ -179,22 +179,22 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
                .with_timezone(&timezone)
                .to_rfc2822();

-
                println!(
+
                term::print(format_args!(
                    "{} {}",
                    term::format::yellow("commit"),
                    term::format::yellow(oid),
-
                );
+
                ));
                if let Ok(parent) = tip.parent_id(0) {
-
                    println!("parent {parent}");
+
                    term::print(format_args!("parent {parent}"));
                }
-
                println!("blob   {}", revision.blob);
-
                println!("date   {time}");
-
                println!();
+
                term::print(format_args!("blob   {}", revision.blob));
+
                term::print(format_args!("date   {time}"));
+
                term::blank();

                if let Some(msg) = tip.message() {
                    for line in msg.lines() {
                        if line.is_empty() {
-
                            println!();
+
                            term::blank();
                        } else {
                            term::indented(term::format::dim(line));
                        }
@@ -202,10 +202,10 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
                    term::blank();
                }
                for line in json::to_pretty(&doc, Path::new("radicle.json"))? {
-
                    println!(" {line}");
+
                    term::print(format_args!(" {line}"));
                }

-
                println!();
+
                term::blank();
            }
        }
        Target::RepoId => {
@@ -234,7 +234,7 @@ fn refs(repo: &radicle::storage::git::Repository) -> anyhow::Result<()> {
        }
    }

-
    print!("{}", tree(refs));
+
    term::print_inline(tree(refs));

    Ok(())
}
modified crates/radicle-cli/src/commands/node.rs
@@ -88,7 +88,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {
        Command::Inventory { nid } => {
            let nid = nid.as_ref().unwrap_or(profile.id());
            for rid in profile.routing()?.get_inventory(nid)? {
-
                println!("{}", term::format::tertiary(rid));
+
                term::print(term::format::tertiary(rid));
            }
        }
        Command::Status {
modified crates/radicle-cli/src/commands/node/control.rs
@@ -170,7 +170,7 @@ pub fn logs(lines: usize, follow: Option<time::Duration>, profile: &Profile) ->
    }
    tail.reverse();

-
    print!("{}", term::format::dim(String::from_utf8_lossy(&tail)));
+
    term::print_inline(term::format::dim(String::from_utf8_lossy(&tail)));

    if let Some(timeout) = follow {
        file.seek(SeekFrom::End(0))?;
@@ -184,7 +184,7 @@ pub fn logs(lines: usize, follow: Option<time::Duration>, profile: &Profile) ->
            if len == 0 {
                thread::sleep(time::Duration::from_millis(250));
            } else {
-
                print!("{}", term::format::dim(line));
+
                term::print_inline(term::format::dim(line));
            }
        }
    }
@@ -414,7 +414,7 @@ pub fn config(node: &Node) -> anyhow::Result<()> {
    let cfg = node.config()?;
    let cfg = serde_json::to_string_pretty(&cfg)?;

-
    println!("{cfg}");
+
    term::print(cfg);

    Ok(())
}
modified crates/radicle-cli/src/commands/node/events.rs
@@ -2,6 +2,8 @@ use std::time;

use radicle::node::{Event, Handle};

+
use crate::terminal as term;
+

pub fn run<H>(node: H, count: usize, timeout: time::Duration) -> anyhow::Result<()>
where
    H: Handle<Event = Result<Event, <H as Handle>::Error>>,
@@ -11,7 +13,7 @@ where
        let event = event?;
        let obj = serde_json::to_string(&event)?;

-
        println!("{obj}");
+
        term::print(&obj);

        // Only output up to `count` events.
        if i + 1 >= count {
modified crates/radicle-cli/src/commands/node/routing.rs
@@ -44,6 +44,6 @@ fn print_table(entries: impl IntoIterator<Item = (RepoId, NodeId)>) {

fn print_json(entries: impl IntoIterator<Item = (RepoId, NodeId)>) {
    for (rid, nid) in entries {
-
        println!("{}", serde_json::json!({ "rid": rid, "nid": nid }));
+
        term::print(serde_json::json!({ "rid": rid, "nid": nid }));
    }
}
modified crates/radicle-cli/src/commands/path.rs
@@ -9,7 +9,7 @@ pub use args::Args;
pub fn run(_args: Args, _ctx: impl term::Context) -> anyhow::Result<()> {
    let home = profile::home()?;

-
    println!("{}", home.path().display());
+
    term::print(home.path().display());

    Ok(())
}
modified crates/radicle-cli/src/lib.rs
@@ -1,6 +1,10 @@
#![allow(clippy::collapsible_if)]
#![allow(clippy::or_fun_call)]
#![allow(clippy::too_many_arguments)]
+
// Prevent use of `println!` and `print!` which panic on broken pipes.
+
// Use `term::print()` or `term::print_inline()` instead.
+
// See: https://github.com/rust-lang/rust/issues/62569
+
#![deny(clippy::print_stdout)]
pub mod commands;
pub mod git;
pub mod node;
modified crates/radicle-cli/src/main.rs
@@ -118,6 +118,24 @@ fn main() {
    .homepage(env!("CARGO_PKG_HOMEPAGE"))
    .support("Open a support request at https://radicle.zulipchat.com/ or file an issue via Radicle itself, or e-mail to team@radicle.xyz"));

+
    // Install a panic hook that intercepts broken pipe panics and exits
+
    // cleanly. This is a backstop for any `println!` calls (in our code or
+
    // dependencies like clap) that we haven't converted to `term::print()`.
+
    //
+
    // The Rust stdlib's `println!` panics with "failed printing to stdout:
+
    // Broken pipe" when writing to a closed pipe. We chain our hook in front
+
    // of human_panic's hook so that non-BrokenPipe panics are still handled
+
    // by human_panic.
+
    //
+
    // See: <https://github.com/rust-lang/rust/issues/62569>
+
    let default_hook = std::panic::take_hook();
+
    std::panic::set_hook(Box::new(move |info| {
+
        if is_broken_pipe_panic(info) {
+
            process::exit(0);
+
        }
+
        default_hook(info);
+
    }));
+

    if let Some(lvl) = radicle::logger::env_level() {
        let logger = Box::new(radicle::logger::Logger::new(lvl));
        log::set_boxed_logger(logger).expect("no other logger should have been set already");
@@ -146,12 +164,47 @@ fn run(command: Command, ctx: impl term::Context) -> ! {
    match run_command(command, ctx) {
        Ok(()) => process::exit(0),
        Err(err) => {
+
            // If the error is a broken pipe, exit silently. This happens when
+
            // output is piped to a command that exits early, e.g. `rad config | head`.
+
            //
+
            // Rust ignores SIGPIPE by default (since 1.62), so broken pipes
+
            // surface as errors rather than terminating the process. For a CLI
+
            // tool, the expected Unix behaviour is to exit cleanly.
+
            //
+
            // See: <https://github.com/rust-lang/rust/issues/62569>
+
            if is_broken_pipe(&err) {
+
                process::exit(0);
+
            }
            term::fail(&err);
            process::exit(1);
        }
    }
}

+
/// Check if an error (possibly wrapped in `anyhow`) is a broken pipe.
+
fn is_broken_pipe(err: &anyhow::Error) -> bool {
+
    for cause in err.chain() {
+
        if let Some(io_err) = cause.downcast_ref::<io::Error>() {
+
            if io_err.kind() == ErrorKind::BrokenPipe {
+
                return true;
+
            }
+
        }
+
    }
+
    false
+
}
+

+
/// Check if a panic was caused by a broken pipe write.
+
///
+
/// The Rust stdlib panics with a `String` payload containing "Broken pipe"
+
/// when `println!` or `print!` encounters a closed stdout. This is stable
+
/// behaviour across all Unix platforms (the message comes from the OS error
+
/// description for `EPIPE`).
+
fn is_broken_pipe_panic(info: &std::panic::PanicHookInfo<'_>) -> bool {
+
    info.payload()
+
        .downcast_ref::<String>()
+
        .is_some_and(|msg| msg.contains("Broken pipe"))
+
}
+

fn run_command(command: Command, ctx: impl term::Context) -> Result<(), anyhow::Error> {
    match command {
        Command::Auth(args) => auth::run(args, ctx),
modified crates/radicle-cli/tests/commands/sigpipe.rs
@@ -28,7 +28,6 @@ mod unix {

        use crate::util::environment::Environment;

-
        #[ignore = "test fails"]
        #[test]
        fn rad_config_broken_pipe() {
            let mut environment = Environment::new();
@@ -109,7 +108,6 @@ mod unix {
        }

        /// `rad self` uses `Element::print()` for table output.
-
        #[ignore = "test fails"]
        #[test]
        fn rad_self() {
            let mut environment = Environment::new();