Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
hcpl opened this issue Jun 6, 2017 · 6 comments
Closed

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

hcpl opened this issue Jun 6, 2017 · 6 comments
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself.

Comments

@hcpl
Copy link

hcpl commented Jun 6, 2017

Currently, cargo build outputs:

    Finished dev [unoptimized + debuginfo] target(s) in 0.2 secs
error: An unknown error occurred

To learn more, run the command again with --verbose.

and cargo build --verbose:

       Fresh rustc-serialize v0.3.24
       ...
    Finished dev [unoptimized + debuginfo] target(s) in 0.3 secs
error: No space left on device (os error 28)

Seems like the (os error 28) message could've been printed in non-verbose mode too.

@alexcrichton alexcrichton added the A-diagnostics Area: Error and warning messages generated by Cargo itself. label Jun 6, 2017
@hcpl
Copy link
Author

hcpl commented Jun 6, 2017

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.

@alexcrichton
Copy link
Member

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

@hcpl
Copy link
Author

hcpl commented Jun 6, 2017

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.

@alexcrichton
Copy link
Member

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!

@jluner
Copy link
Contributor

jluner commented Jun 8, 2017

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.

@ehuss
Copy link
Contributor

ehuss commented May 20, 2020

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).

@ehuss ehuss closed this as completed May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Error and warning messages generated by Cargo itself.
Projects
None yet
Development

No branches or pull requests

4 participants