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

Improve error formatting #4467

Merged
merged 4 commits into from Jan 25, 2021
Merged

Improve error formatting #4467

merged 4 commits into from Jan 25, 2021

Conversation

edolstra
Copy link
Member

@edolstra edolstra commented Jan 21, 2021

Changes:

  • The divider lines are gone. These were in practice a bit confusing, in particular with --show-trace or --keep-going, since then there were multiple lines, suggesting a start/end which wasn't the case.

  • Instead, multi-line error messages are now indented to align with the prefix (e.g. error: ).

  • The description field is gone since we weren't really using it.

  • hint is renamed to msg since it really wasn't a hint.

  • The error is now printed before the location info.

  • The name field is no longer printed since most of the time it wasn't very useful since it was just the name of the exception (like EvalError). Ideally in the future this would be a unique, easily googleable error ID (like rustc).

  • trace: is now just . This assumes error contexts start with something like "while doing X".

  • Error locations are printed in a more standard format (filename:line:column).

Example before:

error: --- AssertionError ---------------------------------------------------------------------------------------- nix
at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

     6|
     7|   x = assert false; 1;
      |       ^
     8|

assertion 'false' failed
----------------------------------------------------- show-trace -----------------------------------------------------
trace: while evaluating the attribute 'x' of the derivation 'hello-2.10'
at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix

   191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
   192|           name = "${attrs.pname}-${attrs.version}";
      |           ^
   193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {

Example after:

error: assertion 'false' failed

       at /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix:7:7:

            6|
            7|   x = assert false; 1;
             |       ^
            8|

       … while evaluating the attribute 'x' of the derivation 'hello-2.10'

       at /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:192:11:

          191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
          192|           name = "${attrs.pname}-${attrs.version}";
             |           ^
          193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {

@garbas

@edolstra edolstra changed the title Error formatting Improve error formatting Jan 21, 2021
Changes:

* The divider lines are gone. These were in practice a bit confusing,
  in particular with --show-trace or --keep-going, since then there
  were multiple lines, suggesting a start/end which wasn't the case.

* Instead, multi-line error messages are now indented to align with
  the prefix (e.g. "error: ").

* The 'description' field is gone since we weren't really using it.

* 'hint' is renamed to 'msg' since it really wasn't a hint.

* The error is now printed *before* the location info.

* The 'name' field is no longer printed since most of the time it
  wasn't very useful since it was just the name of the exception (like
  EvalError). Ideally in the future this would be a unique, easily
  googleable error ID (like rustc).

* "trace:" is now just "…". This assumes error contexts start with
  something like "while doing X".

Example before:

  error: --- AssertionError ---------------------------------------------------------------------------------------- nix
  at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

       6|
       7|   x = assert false; 1;
        |       ^
       8|

  assertion 'false' failed
  ----------------------------------------------------- show-trace -----------------------------------------------------
  trace: while evaluating the attribute 'x' of the derivation 'hello-2.10'
  at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix

     191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
     192|           name = "${attrs.pname}-${attrs.version}";
        |           ^
     193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {

Example after:

  error: assertion 'false' failed

         at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

              6|
              7|   x = assert false; 1;
               |       ^
              8|

         … while evaluating the attribute 'x' of the derivation 'hello-2.10'

         at: (192:11) in file: /home/eelco/Dev/nixpkgs/pkgs/stdenv/generic/make-derivation.nix

            191|         // (lib.optionalAttrs (!(attrs ? name) && attrs ? pname && attrs ? version)) {
            192|           name = "${attrs.pname}-${attrs.version}";
               |           ^
            193|         } // (lib.optionalAttrs (stdenv.hostPlatform != stdenv.buildPlatform && !dontAddHostSuffix && (attrs ? name || (attrs ? pname && attrs ? version)))) {
It's now

  at /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix:7:7:

instead of

  at: (7:7) in file: /home/eelco/Dev/nixpkgs/pkgs/applications/misc/hello/default.nix

The new format is more standard and clickable.
@domenkozar
Copy link
Member

cc @bburdette

@edolstra edolstra merged commit 488a826 into NixOS:master Jan 25, 2021
@edolstra edolstra deleted the error-formatting branch January 25, 2021 11:51
@bburdette
Copy link
Contributor

I for one will miss the divider, although the show-trace one was maybe a bit much. What about program name? I like seeing that in gists of errors and whatnot, takes one question off the table right away. In a future time where the only command is nix it won't be relevant, but I feel like its nice to have for now.

Probably hintfmt should be renamed too? But not sure what the name should be. msgfmt is pretty generic. Something like colorfmt would be more accurate.

@edolstra
Copy link
Member Author

Yeah the program name is useful (especially to see whether the error came from the daemon) but it won't work anymore once everything is called "nix". We probably should add something to the trace to indicate that the error came from the daemon.

colorfmt or colorize would be a good name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants