Ignore EPIPE in CLI (#746)

wfchandler · web-flow · commit f472bce5970b · 2024-07-17T10:24:03.000-04:00
Piping command output to a process that performs a partial read before
closing the pipe, such as `head`, will cause an `EPIPE` to be raised on
the next write attempt. The standard `(e)print(ln)!` macros will panic
on any errors when writing, including `EPIPE`. One option to handle this
is to switch to `write(ln)!`, but this will inject new requirements to
handle `Result` where used, which can be onerous.

Create wrappers around the print macros to ignore `EPIPE`, and replace
calls to the originals with them. Update `main` to ignore any `EPIPE`s
returned from `writeln!` calls in subcommands.

We deliberately do not make this change to the `auth login/logout`
subcommands as these are mutating the config. Failure to notify the user
of the changes is fatal.

Before:

  $ oxide system networking switch-port-settings show | head -1
    switch0/qsfp0
    thread 'tokio-runtime-worker' panicked at library/std/src/io/stdio.rs:1021:9:
    failed printing to stdout: Broken pipe (os error 32)
    note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
    thread 'main' panicked at cli/src/main.rs:102:10:
    called `Result::unwrap()` on an `Err` value: JoinError::Panic(Id(15), ...)

After:

  $ oxide system networking switch-port-settings show | head -1
    switch0/qsfp0
diff --git a/cli/src/cmd_api.rs b/cli/src/cmd_api.rs
@@ -16,6 +16,8 @@ use futures::{StreamExt, TryStreamExt};
 use oxide::Client;
 use serde::Deserialize;
 
+use crate::{print_nopipe, println_nopipe};
+
 /// Makes an authenticated HTTP request to the Oxide API and prints the response.
 ///
 /// The endpoint argument should be a path of a Oxide API endpoint.
@@ -183,18 +185,18 @@ impl crate::AuthenticatedCmd for CmdApi {
         if !resp.status().is_success() {
             let status = resp.status();
             let v = resp.json::<serde_json::Value>().await?;
-            println!(
+            println_nopipe!(
                 "error; status code: {} {}",
                 status.as_u16(),
                 status.canonical_reason().unwrap_or_default()
             );
-            println!("{}", serde_json::to_string_pretty(&v).unwrap());
+            println_nopipe!("{}", serde_json::to_string_pretty(&v).unwrap());
             return Err(anyhow!("error"));
         }
 
         // Print the response headers if requested.
         if self.include {
-            println!("{:?} {}", resp.version(), resp.status());
+            println_nopipe!("{:?} {}", resp.version(), resp.status());
             print_headers(resp.headers())?;
         }
 
@@ -205,7 +207,7 @@ impl crate::AuthenticatedCmd for CmdApi {
         if !self.paginate {
             // Print the response body.
             let result = resp.json::<serde_json::Value>().await?;
-            println!("{}", serde_json::to_string_pretty(&result)?);
+            println_nopipe!("{}", serde_json::to_string_pretty(&result)?);
 
             Ok(())
         } else {
@@ -239,7 +241,7 @@ impl crate::AuthenticatedCmd for CmdApi {
                 Result::Ok(Some((items, next_page)))
             });
 
-            print!("[");
+            print_nopipe!("[");
 
             let result = first
                 .chain(rest)
@@ -252,19 +254,19 @@ impl crate::AuthenticatedCmd for CmdApi {
                         let items_core = &value_out[2..len - 2];
 
                         if comma_needed {
-                            print!(",");
+                            print_nopipe!(",");
                         }
-                        println!();
-                        print!("{}", items_core);
+                        println_nopipe!();
+                        print_nopipe!("{}", items_core);
                     }
                     Ok(true)
                 })
                 .await;
-            println!();
-            println!("]");
+            println_nopipe!();
+            println_nopipe!("]");
 
             if let Err(e) = &result {
-                println!("An error occurred during a paginated query:\n{}", e);
+                println_nopipe!("An error occurred during a paginated query:\n{}", e);
             }
             result.map(|_| ())
         }
@@ -387,7 +389,7 @@ fn print_headers(headers: &reqwest::header::HeaderMap) -> Result<()> {
     tw.flush()?;
 
     let table = String::from_utf8(tw.into_inner()?)?;
-    println!("{}", table);
+    println_nopipe!("{}", table);
 
     Ok(())
 }
diff --git a/cli/src/cmd_auth.rs b/cli/src/cmd_auth.rs
@@ -19,7 +19,7 @@ use toml_edit::{Item, Table};
 use uuid::Uuid;
 
 use crate::context::Context;
-use crate::{AsHost, RunnableCmd};
+use crate::{println_nopipe, AsHost, RunnableCmd};
 
 /// Login, logout, and get the status of your authentication.
 ///
@@ -441,11 +441,11 @@ impl CmdAuthStatus {
             match client.current_user_view().send().await {
                 Ok(user) => {
                     log::debug!("success response for {} (env): {:?}", host_env, user);
-                    println!("Logged in to {} as {}", host_env, user.id)
+                    println_nopipe!("Logged in to {} as {}", host_env, user.id)
                 }
                 Err(e) => {
                     log::debug!("error response for {} (env): {}", host_env, e);
-                    println!("{}: {}", host_env, Self::error_msg(&e))
+                    println_nopipe!("{}: {}", host_env, Self::error_msg(&e))
                 }
             };
         } else {
@@ -467,9 +467,11 @@ impl CmdAuthStatus {
                     }
                 };
 
-                println!(
+                println_nopipe!(
                     "Profile \"{}\" ({}) status: {}",
-                    profile_name, profile_info.host, status
+                    profile_name,
+                    profile_info.host,
+                    status
                 );
             }
         }
diff --git a/cli/src/cmd_disk.rs b/cli/src/cmd_disk.rs
@@ -4,6 +4,8 @@
 
 // Copyright 2024 Oxide Computer Company
 
+use crate::{eprintln_nopipe, println_nopipe};
+
 use anyhow::bail;
 use anyhow::Result;
 use async_trait::async_trait;
@@ -190,9 +192,10 @@ impl CmdDiskImport {
             .await;
 
         if let Err(e) = response {
-            eprintln!(
+            eprintln_nopipe!(
                 "trying to unwind, deleting {:?} failed with {:?}",
-                self.disk, e
+                self.disk,
+                e
             );
             return Err(e.into());
         }
@@ -210,9 +213,10 @@ impl CmdDiskImport {
 
         // If this fails, then the disk will remain in state "import-ready"
         if let Err(e) = response {
-            eprintln!(
+            eprintln_nopipe!(
                 "trying to unwind, finalizing {:?} failed with {:?}",
-                self.disk, e
+                self.disk,
+                e
             );
             return Err(e.into());
         }
@@ -231,9 +235,10 @@ impl CmdDiskImport {
         // If this fails, then the disk will remain in state
         // "importing-from-bulk-writes"
         if let Err(e) = response {
-            eprintln!(
+            eprintln_nopipe!(
                 "trying to unwind, stopping the bulk write process for {:?} failed with {:?}",
-                self.disk, e
+                self.disk,
+                e
             );
             return Err(e.into());
         }
@@ -334,7 +339,7 @@ impl crate::AuthenticatedCmd for CmdDiskImport {
             .await;
 
         if let Err(e) = start_bulk_write_response {
-            eprintln!("starting the bulk write process failed with {:?}", e);
+            eprintln_nopipe!("starting the bulk write process failed with {:?}", e);
 
             // If this fails, the disk is in state import-ready. Finalize it so
             // it can be deleted.
@@ -402,7 +407,7 @@ impl crate::AuthenticatedCmd for CmdDiskImport {
             let n = match file.by_ref().take(CHUNK_SIZE).read_to_end(&mut chunk) {
                 Ok(n) => n,
                 Err(e) => {
-                    eprintln!(
+                    eprintln_nopipe!(
                         "reading from {} failed with {:?}",
                         self.path.to_string_lossy(),
                         e,
@@ -420,7 +425,7 @@ impl crate::AuthenticatedCmd for CmdDiskImport {
                 let encoded = base64::engine::general_purpose::STANDARD.encode(&chunk[0..n]);
 
                 if let Err(e) = senders[i % UPLOAD_TASKS].send((offset, encoded, n)).await {
-                    eprintln!("sending chunk to thread failed with {:?}", e);
+                    eprintln_nopipe!("sending chunk to thread failed with {:?}", e);
                     break Err(e.into());
                 }
             } else {
@@ -456,7 +461,7 @@ impl crate::AuthenticatedCmd for CmdDiskImport {
 
         if results.iter().any(|x| x.is_err()) {
             // If any of the upload threads returned an error, unwind the disk.
-            eprintln!("one of the upload threads failed");
+            eprintln_nopipe!("one of the upload threads failed");
             self.unwind_disk_bulk_write_stop(client).await?;
             self.unwind_disk_finalize(client).await?;
             self.unwind_disk_delete(client).await?;
@@ -475,7 +480,7 @@ impl crate::AuthenticatedCmd for CmdDiskImport {
             .await;
 
         if let Err(e) = stop_bulk_write_response {
-            eprintln!("stopping the bulk write process failed with {:?}", e);
+            eprintln_nopipe!("stopping the bulk write process failed with {:?}", e);
 
             // Attempt to unwind the disk, although it will probably fail - the
             // first step is to stop the bulk write process!
@@ -498,7 +503,7 @@ impl crate::AuthenticatedCmd for CmdDiskImport {
         let finalize_response = request.send().await;
 
         if let Err(e) = finalize_response {
-            eprintln!("finalizing the disk failed with {:?}", e);
+            eprintln_nopipe!("finalizing the disk failed with {:?}", e);
 
             // Attempt to unwind the disk, although it will probably fail - the
             // first step is to finalize the disk!
@@ -541,7 +546,7 @@ impl crate::AuthenticatedCmd for CmdDiskImport {
                 .await?;
         }
 
-        println!("done!");
+        println_nopipe!("done!");
 
         Ok(())
     }
diff --git a/cli/src/cmd_net.rs b/cli/src/cmd_net.rs
@@ -28,6 +28,8 @@ use std::io::Write;
 use tabwriter::TabWriter;
 use uuid::Uuid;
 
+use crate::println_nopipe;
+
 // We do not yet support port breakouts, but the API is phrased in terms of
 // ports that can be broken out. The constant phy0 represents the first port
 // in a breakout.
@@ -875,14 +877,14 @@ impl AuthenticatedCmd for CmdPortConfig {
                     .map(|x| (x.id, x.name))
                     .collect();
 
-                println!(
+                println_nopipe!(
                     "{}{}{}",
                     p.switch_location.to_string().blue(),
                     "/".dimmed(),
                     p.port_name.blue(),
                 );
 
-                println!(
+                println_nopipe!(
                     "{}",
                     "=".repeat(p.port_name.len() + p.switch_location.to_string().len() + 1)
                         .dimmed()
@@ -900,7 +902,7 @@ impl AuthenticatedCmd for CmdPortConfig {
                     writeln!(&mut tw, "{:?}\t{:?}\t{:?}", l.autoneg, l.fec, l.speed,)?;
                 }
                 tw.flush()?;
-                println!();
+                println_nopipe!();
 
                 writeln!(
                     &mut tw,
@@ -923,7 +925,7 @@ impl AuthenticatedCmd for CmdPortConfig {
                     writeln!(&mut tw, "{}\t{}\t{:?}", addr, *alb.0.name, a.vlan_id)?;
                 }
                 tw.flush()?;
-                println!();
+                println_nopipe!();
 
                 writeln!(
                     &mut tw,
@@ -987,7 +989,7 @@ impl AuthenticatedCmd for CmdPortConfig {
                     )?;
                 }
                 tw.flush()?;
-                println!();
+                println_nopipe!();
 
                 // Uncomment to see full payload
                 //println!("");
@@ -1017,13 +1019,13 @@ impl AuthenticatedCmd for CmdBgpStatus {
             .iter()
             .partition(|x| x.switch == SwitchLocation::Switch0);
 
-        println!("{}", "switch0".dimmed());
-        println!("{}", "=======".dimmed());
+        println_nopipe!("{}", "switch0".dimmed());
+        println_nopipe!("{}", "=======".dimmed());
         show_status(&sw0)?;
-        println!();
+        println_nopipe!();
 
-        println!("{}", "switch1".dimmed());
-        println!("{}", "=======".dimmed());
+        println_nopipe!("{}", "switch1".dimmed());
+        println_nopipe!("{}", "=======".dimmed());
         show_status(&sw1)?;
 
         Ok(())
@@ -1078,12 +1080,12 @@ impl AuthenticatedCmd for CmdPortStatus {
         sw0.sort_by_key(|x| x.port_name.as_str());
         sw1.sort_by_key(|x| x.port_name.as_str());
 
-        println!("{}", "switch0".dimmed());
-        println!("{}", "=======".dimmed());
+        println_nopipe!("{}", "switch0".dimmed());
+        println_nopipe!("{}", "=======".dimmed());
         self.show_switch(client, "switch0", &sw0).await?;
 
-        println!("{}", "switch1".dimmed());
-        println!("{}", "=======".dimmed());
+        println_nopipe!("{}", "switch1".dimmed());
+        println_nopipe!("{}", "=======".dimmed());
         self.show_switch(client, "switch1", &sw1).await?;
 
         Ok(())
@@ -1228,9 +1230,9 @@ impl CmdPortStatus {
         }
 
         ltw.flush()?;
-        println!();
+        println_nopipe!();
         mtw.flush()?;
-        println!();
+        println_nopipe!();
 
         Ok(())
     }
diff --git a/cli/src/main.rs b/cli/src/main.rs
diff --git a/cli/src/print.rs b/cli/src/print.rs
