More clear message on "no space left on device" error #4128

Activity

1. alexcrichton

   added

   A-diagnosticsArea: Error and warning messages generated by Cargo itself.Area: Error and warning messages generated by Cargo itself.

   on Jun 6, 2017
2. hcpl commented on Jun 6, 2017

   

   hcpl

   on Jun 6, 2017

   Author

   More actions

   The part responsible for this: https://github.com/rust-lang/cargo/blob/master/src/cargo/lib.rs#L185-L200:

   let hide = unknown && shell.get_verbose() != Verbose;
   
   if let Some(error) = error {
       let _ignored_result = if hide {
           shell.error("An unknown error occurred")
       } else if fatal {
           shell.error(&error)
       } else {
           shell.say(&error, BLACK)
       };
   
       if !handle_cause(error, shell) || hide {
           let _ = shell.err().say("\nTo learn more, run the command again \
                                    with --verbose.".to_string(), BLACK);
       }
   }

   Ways to improve the error reporting:

   • Remove "unknown" from "An unknown error occurred", since the current message can be understood as "Something Cargo doesn't have any understanding about has happened", while in fact Cargo is able to handle this.
   • Introduce the "error message priority system" - high-priority errors are reported regardless of --verbose, only low are hidden by default.
3. alexcrichton commented on Jun 6, 2017

   

   alexcrichton

   on Jun 6, 2017

   Member

   More actions

   Ah so to be clear here there's a few bugs.

   • First and foremost Cargo needs to give a much better error message here more than just not hiding it by default. Cargo typically does this by attaching context to an error as it bubbles up the stack and gets propagated outwards. In the source code you'll find lots of calls to chain_err to support this. Wherever this error is originating from (I'm not sure where) we need a chain_err to add more context to what operation Cargo is even doing. It seems surprising that on a fresh build Cargo is still creating files!
   • Next, Cargo needs to handle "unknown errors" better. I wrote up more about that here
4. hcpl commented on Jun 6, 2017

   

   hcpl

   on Jun 6, 2017

   Author

   More actions

   I want to clarify some things.

   • As I understand, cargo has 3 categories of errors: "show by default", "show if verbose" and "internal" and these 3 scenarios are handled by CargoError::is_human and CargoErrorKind::Internal(...).
   • For any given error kind, "humanness" is a constant property, but is it internal or not, depends on situation.
   • Internal only wraps a CargoError to indicate that the error is not meant to be expected and doesn't add anything.

   If that's true, I think Internal shouldn't be an enum variant but an internal: bool field in CargoError, since it doesn't carry more meaning than a boolean field.
5. alexcrichton commented on Jun 7, 2017

   

   alexcrichton

   on Jun 7, 2017

   Member

   More actions

   That's sorta the way it is, Cargo basically only has two kinds of errors though ("internal" is the same as "show if verbose" above). When printing an error Cargo will walk the stack of errors and print everything so long as it's not tagged internal. Then once it hits an internal error it either prints out "pass --verbose for more info" if the flag isn't passed or otherwise keeps going and prints everything out if --verbose was passed in.

   And yeah I'd be fine changing the struct layout of errors!
6. jluner commented on Jun 8, 2017

   

   jluner

   on Jun 8, 2017

   Contributor

   More actions

   Internal is an "implementation detail" (in the ugly sense) - the need to withhold error information in non-verbose situations had to be reflected somehow, but with error-chain defining the Error struct, there's no way to attach additional information unconditionally. Internal also loses information in some cases - if you take an externally defined error like those from std::io or git, wrapping it directly in Internal messes with your ability to see its chain using the iterator from error-chain. I didn't particularly like it ending up that way, but I think the real fix is @alexcrichton 's rethink of cargo error handling generally.
7. ehuss commented on May 20, 2020

   

   ehuss

   on May 20, 2020

   Contributor

   More actions

   I think this is mostly resolved by now. Errors are generally never hidden (#7896), and a lot more context has been added over the years (like recently #8232).
8. ehuss

   closed this as completedon May 20, 2020