fix(panic): swallows broken pipe errors

We were unwrapping an `Err` from the `write!` calls, of which things
like `BrokenPipe` can happen when the write end of a pipe is closed.
Because we were exit'ing the process anyways at the time the `panic` was
occurring, we are now just swallowing the error.

Changing this API to bubble up the result would be a
breaking change.

Another approach would be to try and write to stdout, if that fails due
to a broken pipe then use stderr. However, that would change the
semantics in what could be argued is a breaking change. Simply dropping
the error, can always be changed to this "use stderr if stdout is
closed" approach later if desired.

For a great explanation of the types of errors see the README in
`calm_io`:

https://github.com/myrrlyn/calm_io/blob/a42845575a04cd8b65e92c19d104627f5fcad3d7/README.md

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "clap"
-version = "2.33.3"
+version = "2.33.4"
 authors = ["Kevin K. <[email protected]>"]
 exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"]
 repository = "https://github.com/clap-rs/clap"

src/errors.rs

@@ -391,14 +391,29 @@ impl Error {
         }
     }
 
-    /// Prints the error to `stderr` and exits with a status of `1`
+    /// Prints the error message and exits. If `Error::use_stderr` evaluates to `true`, the message
+    /// will be written to `stderr` and exits with a status of `1`. Otherwise, `stdout` is used
+    /// with a status of `0`.
     pub fn exit(&self) -> ! {
         if self.use_stderr() {
-            wlnerr!("{}", self.message);
+            wlnerr!(@nopanic "{}", self.message);
             process::exit(1);
         }
-        let out = io::stdout();
-        writeln!(&mut out.lock(), "{}", self.message).expect("Error writing Error to stdout");
+        // We are deliberately dropping errors here. We could match on the error kind, and only
+        // drop things such as `std::io::ErrorKind::BrokenPipe`, however nothing is being bubbled
+        // up or reported back to the caller and we will be exit'ing the process anyways.
+        // Additionally, changing this API to bubble up the result would be a breaking change.
+        //
+        // Another approach could be to try and write to stdout, if that fails due to a broken pipe
+        // then use stderr. However, that would change the semantics in what could be argued is a
+        // breaking change. Simply dropping the error, can always be changed to this "use stderr if
+        // stdout is closed" approach later if desired.
+        //
+        // A good explanation of the types of errors are SIGPIPE where the read side of the pipe
+        // closes before the write side. See the README in `calm_io` for a good explanation:
+        //
+        // https://github.com/myrrlyn/calm_io/blob/a42845575a04cd8b65e92c19d104627f5fcad3d7/README.md
+        let _ = writeln!(&mut io::stdout().lock(), "{}", self.message);
         process::exit(0);
     }
 

src/macros.rs

@@ -839,6 +839,10 @@ macro_rules! impl_settings {
 
 // Convenience for writing to stderr thanks to https://github.com/BurntSushi
 macro_rules! wlnerr(
+    (@nopanic $($arg:tt)*) => ({
+        use std::io::{Write, stderr};
+        let _ = writeln!(&mut stderr().lock(), $($arg)*);
+    });
     ($($arg:tt)*) => ({
         use std::io::{Write, stderr};
         writeln!(&mut stderr(), $($arg)*).ok();