Print human errors in traces by default

This helps get nice pretty stack traces which terminate when we get into the
weeds of errors but still present relevant contextual information when Cargo
isn't run with --verbose

src/cargo/lib.rs

@@ -151,7 +151,7 @@ pub fn shell(verbose: bool) -> MultiShell {
 // `output` print variant error strings to either stderr or stdout.
 // For fatal errors, print to stderr;
 // and for others, e.g. docopt version info, print to stdout.
-fn output(caption: Option<String>, detail: Option<String>,
+fn output(caption: Option<&str>, detail: Option<String>,
           shell: &mut MultiShell, fatal: bool) {
     let std_shell = if fatal {shell.err()} else {shell.out()};
     if let Some(caption) = caption {
@@ -167,46 +167,51 @@ pub fn handle_error(err: CliError, shell: &mut MultiShell) {
     log!(4, "handle_error; err={:?}", err);
 
     let CliError { error, exit_code, unknown } = err;
-    let verbose = shell.get_verbose();
     let fatal = exit_code != 0; // exit_code == 0 is non-fatal error
 
+
+    let desc = error.description();
     if unknown {
-        output(Some("An unknown error occurred".to_string()), None, shell, fatal);
-    } else if error.to_string().len() > 0 {
-        output(Some(error.to_string()), None, shell, fatal);
+        output(Some("An unknown error occurred"), None, shell, fatal);
+    } else if desc.len() > 0 {
+        output(Some(desc), None, shell, fatal);
     }
-
-    if error.cause().is_some() || unknown {
-        if !verbose {
-            output(None,
-                   Some("\nTo learn more, run the command again with --verbose.".to_string()),
-                   shell, fatal);
-        }
+    if shell.get_verbose() {
+        output(None, error.detail(), shell, fatal);
     }
-
-    if verbose {
-        if unknown {
-            output(Some(error.to_string()), None, shell, fatal);
-        }
-        if let Some(detail) = error.detail() {
-            output(None, Some(detail), shell, fatal);
-        }
-        if let Some(err) = error.cause() {
-            let _ = handle_cause(err, shell);
-        }
+    if !handle_cause(&*error, shell) {
+        let _ = shell.err().say("\nTo learn more, run the command again \
+                                 with --verbose.".to_string(), BLACK);
     }
 
     std::os::set_exit_status(exit_code as isize);
 }
 
-fn handle_cause(mut err: &Error, shell: &mut MultiShell) {
+fn handle_cause(mut cargo_err: &CargoError, shell: &mut MultiShell) -> bool {
+    let verbose = shell.get_verbose();
+    let mut err;
     loop {
-        let _ = shell.err().say("\nCaused by:", BLACK);
-        let _ = shell.err().say(format!("  {}", err.description()), BLACK);
+        cargo_err = match cargo_err.cargo_cause() {
+            Some(cause) => cause,
+            None => { err = cargo_err.cause(); break }
+        };
+        if !verbose && !cargo_err.is_human() { return false }
+        print(cargo_err.description(), cargo_err.detail(), shell);
+    }
+    loop {
+        let cause = match err { Some(err) => err, None => return true };
+        if !verbose { return false }
+        print(cause.description(), cause.detail(), shell);
+        err = cause.cause();
+    }
 
-        match err.cause() {
-            Some(e) => err = e,
-            None => break,
+    fn print(desc: &str, detail: Option<String>, shell: &mut MultiShell) {
+        let _ = shell.err().say("\nCaused by:", BLACK);
+        let _ = shell.err().say(format!("  {}", desc), BLACK);
+        if shell.get_verbose() {
+            if let Some(detail) = detail {
+                let _ = shell.err().say(detail, BLACK);
+            }
         }
     }
 }

src/cargo/util/errors.rs

@@ -19,6 +19,7 @@ pub type CargoResult<T> = Result<T, Box<CargoError>>;
 
 pub trait CargoError: Error {
     fn is_human(&self) -> bool { false }
+    fn cargo_cause(&self) -> Option<&CargoError>{ None }
 }
 
 impl fmt::String for Box<CargoError> {
@@ -41,6 +42,7 @@ impl Error for Box<CargoError> {
 
 impl CargoError for Box<CargoError> {
     fn is_human(&self) -> bool { (**self).is_human() }
+    fn cargo_cause(&self) -> Option<&CargoError> { (**self).cargo_cause() }
 }
 
 // =============================================================================
@@ -53,7 +55,7 @@ pub trait ChainError<T> {
 
 struct ChainedError<E> {
     error: E,
-    cause: Box<Error>,
+    cause: Box<CargoError>,
 }
 
 impl<'a, T, F> ChainError<T> for F where F: FnOnce() -> CargoResult<T> {
@@ -63,7 +65,7 @@ impl<'a, T, F> ChainError<T> for F where F: FnOnce() -> CargoResult<T> {
     }
 }
 
-impl<T, E: Error> ChainError<T> for Result<T, E> {
+impl<T, E: CargoError> ChainError<T> for Result<T, E> {
     fn chain_error<E2, C>(self, callback: C) -> CargoResult<T>
                          where E2: CargoError, C: FnOnce() -> E2 {
         self.map_err(move |err| {
@@ -88,11 +90,11 @@ impl<T> ChainError<T> for Option<T> {
 impl<E: Error> Error for ChainedError<E> {
     fn description(&self) -> &str { self.error.description() }
     fn detail(&self) -> Option<String> { self.error.detail() }
-    fn cause(&self) -> Option<&Error> { Some(&*self.cause) }
 }
 
 impl<E: CargoError> CargoError for ChainedError<E> {
     fn is_human(&self) -> bool { self.error.is_human() }
+    fn cargo_cause(&self) -> Option<&CargoError> { Some(&*self.cause) }
 }
 
 // =============================================================================
@@ -170,8 +172,9 @@ impl<E: Error> Error for Human<E> {
     fn cause(&self) -> Option<&Error> { self.0.cause() }
 }
 
-impl<E: Error> CargoError for Human<E> {
+impl<E: CargoError> CargoError for Human<E> {
     fn is_human(&self) -> bool { true }
+    fn cargo_cause(&self) -> Option<&CargoError> { self.0.cargo_cause() }
 }
 
 // =============================================================================
@@ -233,7 +236,7 @@ from_error! {
     toml::DecodeError,
 }
 
-impl<E: Error> FromError<Human<E>> for Box<CargoError> {
+impl<E: CargoError> FromError<Human<E>> for Box<CargoError> {
     fn from_error(t: Human<E>) -> Box<CargoError> { Box::new(t) }
 }
 
@@ -247,6 +250,7 @@ impl CargoError for CliError {}
 impl CargoError for toml::Error {}
 impl CargoError for toml::DecodeError {}
 impl CargoError for url::ParseError {}
+impl CargoError for str::Utf8Error {}
 
 // =============================================================================
 // Construction helpers

src/cargo/util/toml.rs

@@ -104,9 +104,11 @@ pub fn to_manifest(contents: &[u8],
     }));
     let root = try!(parse(contents, &manifest));
     let mut d = toml::Decoder::new(toml::Value::Table(root));
-    let toml_manifest: TomlManifest = try!(Decodable::decode(&mut d));
+    let manifest: TomlManifest = try!(Decodable::decode(&mut d).map_err(|e| {
+        human(e.to_string())
+    }));
 
-    let pair = try!(toml_manifest.to_manifest(source_id, &layout, config));
+    let pair = try!(manifest.to_manifest(source_id, &layout, config));
     let (mut manifest, paths) = pair;
     match d.toml {
         Some(ref toml) => add_unused_keys(&mut manifest, toml, "".to_string()),

tests/test_cargo_compile.rs

@@ -45,9 +45,9 @@ test!(cargo_compile_with_invalid_manifest {
         .with_status(101)
         .with_stderr("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-No `package` or `project` section found.
+Caused by:
+  No `package` or `project` section found.
 "))
 });
 
@@ -63,7 +63,9 @@ test!(cargo_compile_with_invalid_manifest2 {
         .with_status(101)
         .with_stderr("\
 failed to parse manifest at `[..]`
-could not parse input as TOML
+
+Caused by:
+  could not parse input as TOML
 Cargo.toml:3:19-3:20 expected a value
 
 "))
@@ -85,7 +87,9 @@ test!(cargo_compile_with_invalid_manifest3 {
         .with_status(101)
         .with_stderr("\
 failed to parse manifest at `[..]`
-could not parse input as TOML\n\
+
+Caused by:
+  could not parse input as TOML\n\
 src[..]Cargo.toml:1:5-1:6 expected a value\n\n"))
 });
 
@@ -103,9 +107,9 @@ test!(cargo_compile_with_invalid_version {
                 .with_status(101)
                 .with_stderr("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-cannot parse '1.0' as a semver for the key `project.version`
+Caused by:
+  cannot parse '1.0' as a semver for the key `project.version`
 "))
 
 });
@@ -784,7 +788,9 @@ test!(missing_lib_and_bin {
                 execs().with_status(101)
                        .with_stderr("\
 failed to parse manifest at `[..]Cargo.toml`
-either a [lib] or [[bin]] section must be present\n"));
+
+Caused by:
+  either a [lib] or [[bin]] section must be present\n"));
 });
 
 test!(lto_build {

tests/test_cargo_compile_custom_build.rs

@@ -859,7 +859,9 @@ test!(build_script_only {
                 execs().with_status(101)
                        .with_stderr("\
 failed to parse manifest at `[..]`
-either a [lib] or [[bin]] section must be present"));
+
+Caused by:
+  either a [lib] or [[bin]] section must be present"));
 });
 
 test!(shared_dep_with_a_build_script {

tests/test_cargo_compile_git_deps.rs

@@ -447,9 +447,9 @@ test!(cargo_compile_with_short_ssh_git {
         .with_stdout("")
         .with_stderr(format!("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-invalid url `{}`: relative URL without a base
+Caused by:
+  invalid url `{}`: relative URL without a base
 ", url)));
 });
 

tests/test_cargo_features.rs

@@ -24,9 +24,9 @@ test!(invalid1 {
     assert_that(p.cargo_process("build"),
                 execs().with_status(101).with_stderr(format!("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-Feature `bar` includes `baz` which is neither a dependency nor another feature
+Caused by:
+  Feature `bar` includes `baz` which is neither a dependency nor another feature
 ").as_slice()));
 });
 
@@ -49,9 +49,9 @@ test!(invalid2 {
     assert_that(p.cargo_process("build"),
                 execs().with_status(101).with_stderr(format!("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-Features and dependencies cannot have the same name: `bar`
+Caused by:
+  Features and dependencies cannot have the same name: `bar`
 ").as_slice()));
 });
 
@@ -74,9 +74,9 @@ test!(invalid3 {
     assert_that(p.cargo_process("build"),
                 execs().with_status(101).with_stderr(format!("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-Feature `bar` depends on `baz` which is not an optional dependency.
+Caused by:
+  Feature `bar` depends on `baz` which is not an optional dependency.
 Consider adding `optional = true` to the dependency
 ").as_slice()));
 });
@@ -137,9 +137,9 @@ test!(invalid5 {
     assert_that(p.cargo_process("build"),
                 execs().with_status(101).with_stderr(format!("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-Dev-dependencies are not allowed to be optional: `bar`
+Caused by:
+  Dev-dependencies are not allowed to be optional: `bar`
 ").as_slice()));
 });
 
@@ -159,9 +159,9 @@ test!(invalid6 {
     assert_that(p.cargo_process("build").arg("--features").arg("foo"),
                 execs().with_status(101).with_stderr(format!("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-Feature `foo` requires `bar` which is not an optional dependency
+Caused by:
+  Feature `foo` requires `bar` which is not an optional dependency
 ").as_slice()));
 });
 
@@ -182,9 +182,9 @@ test!(invalid7 {
     assert_that(p.cargo_process("build").arg("--features").arg("foo"),
                 execs().with_status(101).with_stderr(format!("\
 failed to parse manifest at `[..]`
-Cargo.toml is not a valid manifest
 
-Feature `foo` requires `bar` which is not an optional dependency
+Caused by:
+  Feature `foo` requires `bar` which is not an optional dependency
 ").as_slice()));
 });