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