Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
REVIEW: Polish changes to `main.rs`
Lorenz Leutgeb committed 24 days ago
commit 665df61be9d1dd7c54cf6bc812c795d87c1be930
parent 613ec91ac750d79ecfd89c813d6a5236baf4a934
1 file changed +42 -36
modified crates/radicle-cli/src/main.rs
@@ -118,21 +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()`.
+
    // Install a panic hook that intercepts panics caused by broken pipes and exits
+
    // cleanly. This is a backstop for any uses of `println!` (in our code or
+
    // dependencies like `clap`) that were not 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.
+
    // `println!` panics with "failed printing to stdout: Broken pipe" when
+
    // failing to write to a closed standard output. We chain our hook in front
+
    // of `human_panic`'s hook so that panics not caused by broken pipes 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| {
-
        handle_broken_pipe(info);
-
        default_hook(info);
-
    }));
+
    // See also <https://github.com/rust-lang/rust/issues/62569>.
+
    #[cfg(unix)]
+
    {
+
        let default_hook = std::panic::take_hook();
+
        std::panic::set_hook(Box::new(move |info| {
+
            handle_broken_pipe(info);
+
            default_hook(info);
+
        }));
+
    }

    if let Some(lvl) = radicle::logger::env_level() {
        let logger = Box::new(radicle::logger::Logger::new(lvl));
@@ -162,14 +165,16 @@ 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`.
+
            // If the error is a broken pipe, exit cleanly. This happens when
+
            // output is piped to a command that exits before reading all our
+
            // output, 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.
+
            // Rust ignores `SIGPIPE` by default (since 1.62), so broken pipes
+
            // and instead returns `io::ErrorKind::BrokenPipe` errors on writes.
+
            // We want to catch these and exit cleanly.
            //
-
            // See: <https://github.com/rust-lang/rust/issues/62569>
+
            // See <https://github.com/rust-lang/rust/issues/62569>.
+
            #[cfg(unix)]
            if is_broken_pipe(&err) {
                process::exit(0);
            }
@@ -179,13 +184,14 @@ fn run(command: Command, ctx: impl term::Context) -> ! {
    }
}

-
/// Handle an [`ErrorKind::BrokenPipe`] during a panic, and exit the process
-
/// with exit code 0.
+
/// Handle an error of kind [`ErrorKind::BrokenPipe`] during a panic, and
+
/// exit the process with exit code 0.
///
/// # Debug
///
-
/// If compiled with `debug_assertions` on, then the panic is written to
+
/// If compiled with `debug_assertions` enabled, then the panic is written to
/// [`std::io::stderr`].
+
#[cfg(unix)]
fn handle_broken_pipe(info: &std::panic::PanicHookInfo<'_>) {
    if is_broken_pipe_panic(info) {
        if cfg!(debug_assertions) {
@@ -211,24 +217,24 @@ fn handle_broken_pipe(info: &std::panic::PanicHookInfo<'_>) {
    }
}

-
/// Check if an error (possibly wrapped in `anyhow`) is a broken pipe.
+
/// Check if any error in the [`anyhow::Error::chain`] of `err` is of kind
+
/// [`ErrorKind::BrokenPipe`].
+
#[cfg(unix)]
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
+
    err.chain()
+
        .filter_map(|cause| cause.downcast_ref::<io::Error>())
+
        .any(|io_err| io_err.kind() == ErrorKind::BrokenPipe)
}

-
/// Check if a panic was caused by a broken pipe write.
+
/// Check whether a panic was caused by writing to a broken pipe.
+
///
+
/// The standard library panics with a [`String`] payload containing
+
/// "Broken pipe" when [`println!`] or [`print!`] fail to write because standard
+
/// output is closed. This is stable behaviour across all Unix platforms, since
+
/// it is adopted from the description of `EPIPE` in [`errno.h` in POSIX.1-2024].
///
-
/// 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`).
+
/// [`errno.h` in POSIX.1-2024]: https://pubs.opengroup.org/onlinepubs/9799919799.2024edition/basedefs/errno.h.html
+
#[cfg(unix)]
fn is_broken_pipe_panic(info: &std::panic::PanicHookInfo<'_>) -> bool {
    info.payload()
        .downcast_ref::<String>()