nix icon indicating copy to clipboard operation
nix copied to clipboard

Show the first few and last few stack frames by default

Open 9999years opened this issue 1 year ago • 7 comments

Is your feature request related to a problem? Please describe. When an error is encountered and --show-trace hasn't been given, only the three outer-most stack frames are displayed. Often, these are not very useful. Here's the full text of an error I was asked to help diagnose at work:

error:
       … while calling the 'derivationStrict' builtin

         at /builtin/derivation.nix:9:12: (source not available)

       … while evaluating derivation 'enter-shell'
         whose name attribute is located at /nix/store/ybwlqh43kz9cpccikrka6h2x44c98yf4-source/pkgs/stdenv/generic/make-derivation.nix:311:7

       … while evaluating attribute 'text' of derivation 'enter-shell'

         at /nix/store/ybwlqh43kz9cpccikrka6h2x44c98yf4-source/pkgs/build-support/trivial-builders/default.nix:148:16:

          147|     runCommand name
          148|       { inherit text executable checkPhase allowSubstitutes preferLocalBuild;
             |                ^
          149|         passAsFile = [ "text" ];

       (stack trace truncated; use '--show-trace' to show the full trace)

It's likely that the inner-most couple stack frames contain the most useful information here, but they're not shown and non-expert Nix users don't understand enough of the information being presented in the first few frames to know they're all generic boilerplate code.

Describe the solution you'd like

Instead of showing the three outer-most stack frames, the first (perhaps?) two and last two should be shown instead.

Additional context

The stack frame printing logic was designed by @layus, who admits that the choice to print the first three frames was arbitrary:

https://github.com/NixOS/nix/blob/09a6e8e7030170611a833612b9f40b9a10778c18/src/libutil/error.cc#L297-L298

While that logic has been more-or-less fine until now, I think adding the last few frames will increase the signal-to-noise ratio of Nix's error reports.

Priorities

Add :+1: to issues you find important.

9999years avatar Feb 07 '24 21:02 9999years

Well, the three inner frame where showed, then someone reversed the order of the frames without changing the logic around selecting the first three, thus displaying the outer most. I find it quite funny that it took so long for someone (else) to notice.

I am all :+1: on this. I would even go as far as not showing the outermost frames. They are often irrelevant I think, but I have no data to sustain it. (The number is arbitrary, but the inner/outer side was not).

Also, we need tests for this. I failed to add the right ones, relying only on the internal full stack, thus not testing the logic picking only a few frames.

layus avatar Feb 07 '24 23:02 layus

See more context about order reversal in https://github.com/NixOS/nix/pull/7334. Also, some people seem to think that the outermost frames are more interesting. But I am not sure we are speaking about the same inner and outer. Having both is probably the best way out of this.

layus avatar Feb 07 '24 23:02 layus

Module system, nixpkgs, and flakes all have useful information at different depths. It's hard to have a single choice here.

tomberek avatar Feb 08 '24 04:02 tomberek

Yes, I think much of the time --show-trace is the best way to get useful data. Still, we can try to make the default display more useful more of the time.

Another idea suggested was to make more of an effort to display user-annotated frames.

9999years avatar Feb 08 '24 17:02 9999years

With flakes, especially a local one, there is a heuristic that any "local files" frames are helpful, likely because that's what they are working on and might be editing. But that feels wrong, to make the output dependent on cwd.

tomberek avatar Feb 09 '24 03:02 tomberek

Another heuristic is whether the frame has addErrorContext. IMO this would be very very important to include for NixOS usability.

lf- avatar Feb 09 '24 03:02 lf-

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

https://discourse.nixos.org/t/tweag-nix-dev-update-54/39990/1

nixos-discourse avatar Feb 19 '24 10:02 nixos-discourse

Another heuristic is whether the frame has addErrorContext. IMO this would be very very important to include for NixOS usability.

The team has approved this one in https://github.com/NixOS/nix/issues/7553, ~so a PR would be welcomed.~ I've taken a stab at it myself.

roberth avatar Mar 22 '24 12:03 roberth

The team has approved this one in #7553, so a PR would be welcomed.

I happen to go on and promise an implementation to someone else on that same day, so now a review would be welcomed.

  • https://github.com/NixOS/nix/pull/10305

(It's implementing a different issue than this one, but I had to update my previous comment. Also it touches the same code.)

roberth avatar Mar 23 '24 23:03 roberth