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

Show addErrorContext traces by default #7553

Closed
roberth opened this issue Jan 4, 2023 · 3 comments · Fixed by #10305
Closed

Show addErrorContext traces by default #7553

roberth opened this issue Jan 4, 2023 · 3 comments · Fixed by #10305
Labels
error-messages Confusing messages and better diagnostics feature Feature request or proposal idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. language The Nix expression language; parser, interpreter, primops, evaluation, etc

Comments

@roberth
Copy link
Member

roberth commented Jan 4, 2023

Is your feature request related to a problem? Please describe.

Module system stack traces are too long. The low information density makes users believe that they won't learn anything from reading through it, but essential information is in the stack trace, thanks to builtins.addErrorContext.

Describe the solution you'd like

addErrorContext traces are sufficiently valuable and terse that the should always be shown.

Optionally, add --no-trace, to disable that, but I don't think we even want that.

Describe alternatives you've considered

See the following and its alternatives

Additional context

Example

This would have been a 700 line stack trace with --show-trace (or --full-trace if you're confused by the other issue).

error: infinite recursion encountered

       … while evaluating definitions from `/home/user/h/nixpkgs/pkgs/top-level/all-packages.nix':

       … while evaluating the option `services.postgresql.enable':

       … while evaluating definitions from `/home/user/h/nixpkgs/pkgs/top-level/all-packages.nix':

       … while evaluating the option `services.rabbitmq.enable':

       … while evaluating definitions from `/home/user/h/nixpkgs/nixos/modules/services/amqp/rabbitmq.nix':

       … while evaluating the option `services.epmd.enable':

       … while evaluating definitions from `/home/user/h/nixpkgs/nixos/modules/services/networking/epmd.nix':

       … while evaluating the option `assertions':

See the full example in #7552

Priorities

Add 👍 to issues you find important.

@roberth roberth added feature Feature request or proposal error-messages Confusing messages and better diagnostics labels Jan 4, 2023
@roberth roberth mentioned this issue Jan 19, 2023
3 tasks
@roberth
Copy link
Member Author

roberth commented Jan 19, 2023

We don't have "spammy addErrorContext" in NixOS, but we might want to restrict to a certain number of lines in case some other library does have it.

If spamming does happen in the future, we could add measures similar to what logging libraries use: log levels and/or labeling by module. This is completely hypothetical for now.

@roberth roberth added the language The Nix expression language; parser, interpreter, primops, evaluation, etc label Jun 23, 2023
@edolstra edolstra added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Jul 14, 2023
@edolstra
Copy link
Member

Note from the team discussion: it might be nice to be able to associate priorities with messages.

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-07-14-nix-team-meeting-minutes-71/30472/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error-messages Confusing messages and better diagnostics feature Feature request or proposal idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. language The Nix expression language; parser, interpreter, primops, evaluation, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants