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

Print the value in error: cannot coerce messages #9754

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

9999years
Copy link
Contributor

Motivation

This extends the error: cannot coerce a <TYPE> to a string message to print the value that could not be coerced. This helps with debugging by making it easier to track down where the value is being produced from, especially in errors with deep or unhelpful stack traces.

Context

Take two of #9553 now that #9606 is merged.
See #561.

Here's an example of a mistake I made that would have been much easier to fix with this patch. I encountered this error while converting a NixOS configuration that relied on builtins.currentSystem to work in a flake. There were several overlays which pinned packages to specific versions, like this:

let
  nixos_master = import (
    builtins.fetchGit {
      url = "https://github.com/NixOS/nixpkgs.git";
      ref = "master";
      rev = "0c0d57d79de06b36286b8c7551dc173e372b86ad";
    }
  ) {};
in
self: super: {
  opentelemetry-collector-contrib = nixos_master.opentelemetry-collector-contrib; # version 0.78.0
}

My first try added an inherit (config.nixpkgs) localSystem; to the import arguments, but that gave me an unhelpful error:

$ nix repl nixpkgs
nix-repl> nixpkgs = legacyPackages.x86_64-linux.path
nix-repl> system = lib.systems.elaborate "x86_64-linux"
nix-repl> import nixpkgs { inherit system; }
error:
       … while evaluating a branch condition

         at /nix/store/7g8wbm1z6948x0qma4hzkkb9a3xys76w-source/pkgs/stdenv/booter.nix:64:9:

           63|       go = pred: n:
           64|         if n == len
             |         ^
           65|         then rnul pred

       … while calling the 'length' builtin

         at /nix/store/7g8wbm1z6948x0qma4hzkkb9a3xys76w-source/pkgs/stdenv/booter.nix:62:13:

           61|     let
           62|       len = builtins.length list;
             |             ^
           63|       go = pred: n:

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

       error: cannot coerce a set to a string

Note that the stack trace and error message don't make it clear which set couldn't be coerced to a string, or where the coercion failed. It's fairly easy to figure out what's gone wrong here, but in a repository with 14kloc it was significantly more difficult.

With this patch, the error message makes it clear what's happened (a system attrset has been used in place of a system string):

error: cannot coerce a set to a string: { aesSupport = «thunk»; avx2Support = «thunk»; avx512Support = «thunk»; avxSupport = «thunk»; canExecute = «thunk»; config = «thunk»; darwinArch = «thunk»; darwinMinVersion = «thunk»; darwinMinVersionVariable = «thunk»; darwinPlatform = «thunk»; «94 attributes elided»}

Priorities

Add 👍 to pull requests you find important.

@9999years 9999years requested a review from edolstra as a code owner January 12, 2024 18:31
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Jan 12, 2024
@9999years 9999years force-pushed the print-value-when-coercion-fails branch 2 times, most recently from d57b43f to 71ec998 Compare January 12, 2024 18:37
@roberth
Copy link
Member

roberth commented Jan 13, 2024

It seems that some comments from this review apply here as well.

@Ma27
Copy link
Member

Ma27 commented Jan 15, 2024

@9999years do you also have patches for the error message "attempt to call something which is not a function" lying around? Otherwise I'd be happy to write something for that as well :)

@9999years
Copy link
Contributor Author

@Ma27

do you also have patches for the error message "attempt to call something which is not a function"

No but that sounds great. Go for it!

@9999years 9999years force-pushed the print-value-when-coercion-fails branch from 71ec998 to 489f190 Compare January 20, 2024 01:06
Ma27 added a commit to Ma27/nix that referenced this pull request Jan 20, 2024
Low-hanging fruit in the spirit of NixOS#9753 and NixOS#9754 (means 9999years did
all the hard work already).

This basically prints out what was attempted to be called as function,
i.e.

  map (import <nixpkgs> {}) [ 1 2 3 ]

now gives the following error message:

    error:
           … while calling the 'map' builtin
             at «string»:1:1:
                1| map (import <nixpkgs> {}) [ 1 2 3 ]
                 | ^

           … while evaluating the first argument passed to builtins.map

           error: expected a function but found a set: { _type = "pkgs"; AAAAAASomeThingsFailToEvaluate = «thunk»; AMB-plugins = «thunk»; ArchiSteamFarm = «thunk»; BeatSaberModManager = «thunk»; CHOWTapeModel = «thunk»; ChowCentaur = «thunk»; ChowKick = «thunk»; ChowPhaser = «thunk»; CoinMP = «thunk»;  «18783 attributes elided»}
Ma27 added a commit to Ma27/nix that referenced this pull request Jan 22, 2024
Low-hanging fruit in the spirit of NixOS#9753 and NixOS#9754 (means 9999years did
all the hard work already).

This basically prints out what was attempted to be called as function,
i.e.

  map (import <nixpkgs> {}) [ 1 2 3 ]

now gives the following error message:

    error:
           … while calling the 'map' builtin
             at «string»:1:1:
                1| map (import <nixpkgs> {}) [ 1 2 3 ]
                 | ^

           … while evaluating the first argument passed to builtins.map

           error: expected a function but found a set: { _type = "pkgs"; AAAAAASomeThingsFailToEvaluate = «thunk»; AMB-plugins = «thunk»; ArchiSteamFarm = «thunk»; BeatSaberModManager = «thunk»; CHOWTapeModel = «thunk»; ChowCentaur = «thunk»; ChowKick = «thunk»; ChowPhaser = «thunk»; CoinMP = «thunk»;  «18783 attributes elided»}
@9999years 9999years force-pushed the print-value-when-coercion-fails branch from 489f190 to 0e822c0 Compare January 22, 2024 22:44
This extends the `error: cannot coerce a TYPE to a string` message
to print the value that could not be coerced. This helps with debugging
by making it easier to track down where the value is being produced
from, especially in errors with deep or unhelpful stack traces.
@9999years 9999years force-pushed the print-value-when-coercion-fails branch from 0e822c0 to 83bb494 Compare January 23, 2024 23:15
@9999years 9999years requested a review from roberth January 23, 2024 23:43
@roberth roberth merged commit 5b7bfd2 into NixOS:master Jan 24, 2024
8 checks passed
@roberth
Copy link
Member

roberth commented Jan 24, 2024

Thanks again @9999years for another nice improvement.

@9999years 9999years deleted the print-value-when-coercion-fails branch January 24, 2024 17:21
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-01-22-nix-team-meeting-minutes-117/38838/1

9999years added a commit to 9999years/nixpkgs that referenced this pull request Mar 7, 2024
I recently encountered this error attempting to build a derivation:

```
error:
       … while calling the 'derivationStrict' builtin

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

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

       … while evaluating attribute 'nativeBuildInputs' of derivation 'infra-shell'

         at /nix/store/ppm74s0slima7385piksmdcnvcawgs1x-source/pkgs/stdenv/generic/make-derivation.nix:397:7:

          396|       depsBuildBuild              = elemAt (elemAt dependencies 0) 0;
          397|       nativeBuildInputs           = elemAt (elemAt dependencies 0) 1;
             |       ^
          398|       depsBuildTarget             = elemAt (elemAt dependencies 0) 2;

       error: Dependency is not of a valid type: element 23 of nativeBuildInputs for infra-shell
```

This isn't very helpful; it doesn't tell me what the type of the
dependency is. I'm also not directly constructing `stdenv.mkDerivation {
nativeBuildInputs = [...]; }`, so it's not obvious what "element 23"
would be (the derivation is constructed through several layers of
helpers, like many derivations in nixpkgs are).

Ultimately, element 23 was a function, which surfaces another issue:

```
nix-repl> throw "Dependency is not of a valid type: ${a: a}"
error:
       … while evaluating a path segment

         at «string»:1:43:

            1| throw "Dependency is not of a valid type: ${a: a}"
             |                                           ^

       error: cannot coerce a function to a string
```

(`builtins.toString` produces similar results.) The Nix language doesn't
provide a safe value printing function, so we can't really construct a
good error message here.

The good news is that we can remove this error checking code from
nixpkgs entirely without losing usability. Here's the error message on
Nix 2.19.3:

```
error:
       … while calling the 'derivationStrict' builtin

         at /derivation-internal.nix:9:12:

            8|
            9|   strict = derivationStrict drvAttrs;
             |            ^
           10|

       … while evaluating derivation 'my-derivation'
         whose name attribute is located at /Users/wiggles/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:347:7

       … while evaluating attribute 'nativeBuildInputs' of derivation 'my-derivation'

         at /Users/wiggles/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:391:7:

          390|       depsBuildBuild              = elemAt (elemAt dependencies 0) 0;
          391|       nativeBuildInputs           = elemAt (elemAt dependencies 0) 1;
             |       ^
          392|       depsBuildTarget             = elemAt (elemAt dependencies 0) 2;

       error: cannot coerce a function to a string
```

Thanks to PRs like NixOS/nix#9754, these error
messages will improve in coming Nix releases:

```
       … while evaluating attribute 'nativeBuildInputs' of derivation 'my-derivation'
         at /Users/wiggles/nixpkgs/pkgs/stdenv/generic/make-derivation.nix:391:7:
          390|       depsBuildBuild              = elemAt (elemAt dependencies 0) 0;
          391|       nativeBuildInputs           = elemAt (elemAt dependencies 0) 1;
             |       ^
          392|       depsBuildTarget             = elemAt (elemAt dependencies 0) 2;

       … while evaluating one element of the list

       error: cannot coerce a function to a string: «lambda @ /Users/wiggles/nixpkgs/xxx-derivation.nix:7:28»
```
tebowy pushed a commit to tebowy/nix that referenced this pull request Jul 11, 2024
…n-fails

Print the value in `error: cannot coerce` messages

(cherry picked from commit 5b7bfd2)

===

test taken from 6e8d598; it was added
previously (and not backported because its pr was a mostly-revert), but
it's useful to have around.

Change-Id: Icbd14b55e3610ce7b774667bf14b82e6dc717982
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
with-tests Issues related to testing. PRs with tests have some priority
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants