From 14bf54bd393f1b48ba519d104abd53434fc18e75 Mon Sep 17 00:00:00 2001 From: Qyriad Date: Sat, 22 Jun 2024 21:59:47 -0600 Subject: [PATCH] trace which part of `foo.bar.baz` errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns errors like: let somepkg.src = throw "invalid foobar"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta … while calling the 'throw' builtin at «string»:2:17: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta error: invalid foobar into errors like: let somepkg.src = throw "invalid foobar"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta … while evaluating 'somepkg.src' to select 'meta' on it at «string»:3:4: 2| somepkg.src = throw "invalid foobar"; 3| in somepkg.src.meta | ^ … while calling the 'throw' builtin at «string»:2:17: 1| let 2| somepkg.src = throw "invalid foobar"; | ^ 3| in somepkg.src.meta error: invalid foobar And for type errors, from: let somepkg.src = "I'm not an attrset"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = "I'm not an attrset"; | ^ 3| in somepkg.src.meta … while selecting an attribute at «string»:3:4: 2| somepkg.src = "I'm not an attrset"; 3| in somepkg.src.meta | ^ error: expected a set but found a string: "I'm not an attrset" into: let somepkg.src = "I'm not an attrset"; in somepkg.src.meta error: … while evaluating the attribute 'src.meta' at «string»:2:3: 1| let 2| somepkg.src = "I'm not an attrset"; | ^ 3| in somepkg.src.meta … while selecting 'meta' on 'somepkg.src' at «string»:3:4: 2| somepkg.src = "I'm not an attrset"; 3| in somepkg.src.meta | ^ error: expected a set but found a string: "I'm not an attrset" For the low price of an enumerate() and a lambda you too can have the incorrect line of code actually show up in the trace! Change-Id: Ic1491c86e33c167891bdac9adad6224784760bd6 --- doc/manual/rl-next/better-attrpath-errors.md | 44 ++++++++++++++++---- src/libexpr/eval.cc | 43 +++++++++++++++++-- 2 files changed, 75 insertions(+), 12 deletions(-) diff --git a/doc/manual/rl-next/better-attrpath-errors.md b/doc/manual/rl-next/better-attrpath-errors.md index d06bfc43b..e8c7d9a96 100644 --- a/doc/manual/rl-next/better-attrpath-errors.md +++ b/doc/manual/rl-next/better-attrpath-errors.md @@ -1,20 +1,18 @@ --- -synopsis: "Trace when the `foo` part of a `foo.bar.baz` expression errors" -cls: 1505 +synopsis: "Trace which part of a `foo.bar.baz` expression errors" +cls: 1505, 1506 credits: Qyriad category: Improvements --- -Previously, if an expression like `linux_4_9.meta.description` errored in the `linux_4_9` part, it wouldn't show you that that's the part of the expression that failed to evaluate, or even that that line of code is what caused evaluation of the failing expression. +Previously, if an attribute path selection expression like `linux_4_9.meta.description` it wouldn't show you which one of those parts in the attribute path, or even that that line of code is what caused evaluation of the failing expression. The previous error looks like this: ``` -let - inherit (pkgs.linuxKernel.kernels) linux_4_9; -in linux_4_9.meta.description +pkgs.linuxKernel.kernels.linux_4_9.meta.description error: - … while evaluating the attribute 'linux_4_9' + … while evaluating the attribute 'linuxKernel.kernels.linux_4_9.meta.description' at /nix/store/dk2rpyb6ndvfbf19bkb2plcz5y3k8i5v-source/pkgs/top-level/linux-kernels.nix:278:5: 277| } // lib.optionalAttrs config.allowAliases { 278| linux_4_9 = throw "linux 4.9 was removed because it will reach its end of life within 22.11"; @@ -33,6 +31,36 @@ error: Now, the error will look like this: +``` +pkgs.linuxKernel.kernels.linux_4_9.meta.description + +error: + … while evaluating the attribute 'linuxKernel.kernels.linux_4_9.meta.description' + at /nix/store/dk2rpyb6ndvfbf19bkb2plcz5y3k8i5v-source/pkgs/top-level/linux-kernels.nix:278:5: + 277| } // lib.optionalAttrs config.allowAliases { + 278| linux_4_9 = throw "linux 4.9 was removed because it will reach its end of life within 22.11"; + | ^ + 279| linux_4_14 = throw "linux 4.14 was removed because it will reach its end of life within 23.11"; + + … while evaluating 'pkgs.linuxKernel.kernels.linux_4_9' to select 'meta' on it + at «string»:1:1: + 1| pkgs.linuxKernel.kernels.linux_4_9.meta.description + | ^ + + … caused by explicit throw + at /nix/store/dk2rpyb6ndvfbf19bkb2plcz5y3k8i5v-source/pkgs/top-level/linux-kernels.nix:278:17: + 277| } // lib.optionalAttrs config.allowAliases { + 278| linux_4_9 = throw "linux 4.9 was removed because it will reach its end of life within 22.11"; + | ^ + 279| linux_4_14 = throw "linux 4.14 was removed because it will reach its end of life within 23.11"; + + error: linux 4.9 was removed because it will reach its end of life within 22.11 +``` + +Not only does the line of code that referenced the failing attribute show up in the trace, it also tells you that it was specifically the `linux_4_9` part that failed. + +This includes if the failing part is a top-level binding: + ``` let inherit (pkgs.linuxKernel.kernels) linux_4_9; @@ -60,5 +88,3 @@ error: error: linux 4.9 was removed because it will reach its end of life within 22.11 ``` - -Not only does the line of code that referenced the failing binding show up in the trace, it also tells you that it was specifically the `linux_4_9` part that failed. diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index b01867a09..56581cc19 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1458,12 +1458,46 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) showAttrPath(state, env, attrPath)) : nullptr; - for (auto const & currentAttrName : attrPath) { + for (auto const & [partIdx, currentAttrName] : enumerate(attrPath)) { state.nrLookups++; Symbol const name = getName(currentAttrName, state, env); - state.forceValue(*vCurrent, pos); + // For formatting errors, which should be done only when needed. + auto partsSoFar = [&]() -> std::string { + std::stringstream ss; + // We start with the base thing this ExprSelect is selecting on. + assert(this->e != nullptr); + this->e->show(state.symbols, ss); + + // Then grab each part of the attr path up to this one. + assert(partIdx < attrPath.size()); + std::span const parts( + attrPath.begin(), + attrPath.begin() + partIdx + ); + + // And convert them to strings and join them. + for (auto const & part : parts) { + auto const partName = getName(part, state, env); + ss << "." << state.symbols[partName]; + } + + return ss.str(); + }; + + try { + state.forceValue(*vCurrent, pos); + } catch (Error & e) { + state.addErrorTrace( + e, + getPos(), + "while evaluating '%s' to select '%s' on it", + partsSoFar(), + state.symbols[name] + ); + throw; + } if (vCurrent->type() != nAttrs) { @@ -1479,7 +1513,10 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) "expected a set but found %s: %s", showType(*vCurrent), ValuePrinter(state, *vCurrent, errorPrintOptions) - ).withTrace(pos, "while selecting an attribute").debugThrow(); + ).addTrace( + pos, + HintFmt("while selecting '%s' on '%s'", state.symbols[name], partsSoFar()) + ).debugThrow(); } // Now that we know this is actually an attrset, try to find an attr