From 9e7e927837f205fbd00769a47f39e5946ea931b7 Mon Sep 17 00:00:00 2001 From: Rebecca Turner Date: Sat, 9 Mar 2024 18:28:04 -0800 Subject: [PATCH] Print top-level errors normally in `nix repl` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, errors while printing values in `nix repl` would be printed in `«error: ...»` brackets rather than displayed normally: ``` nix-repl> legacyPackages.aarch64-darwin.pythonPackages.APScheduler «error: Package ‘python-2.7.18.7’ in /nix/store/6s0m1qc31zw3l3kq0q4wd5cp3lqpkq0q-source/pkgs/development/interpreters/python/cpython/2.7/default.nix:335 is marked as insecure, refusing to evaluate.» ``` Now, errors will be displayed normally if they're emitted at the top-level of an expression: ``` nix-repl> legacyPackages.aarch64-darwin.pythonPackages.APScheduler error: … in the condition of the assert statement at /nix/store/6s0m1qc31zw3l3kq0q4wd5cp3lqpkq0q-source/lib/customisation.nix:268:17: 267| in commonAttrs // { 268| drvPath = assert condition; drv.drvPath; | ^ 269| outPath = assert condition; drv.outPath; … in the left operand of the OR (||) operator at /nix/store/6s0m1qc31zw3l3kq0q4wd5cp3lqpkq0q-source/pkgs/development/interpreters/python/passthrufun.nix:28:45: 27| if lib.isDerivation value then 28| lib.extendDerivation (valid value || throw "${name} should use `buildPythonPackage` or `toPythonModule` if it is to be part of the Python packages set.") {} value | ^ 29| else (stack trace truncated; use '--show-trace' to show the full trace) error: Package ‘python-2.7.18.7’ in /nix/store/6s0m1qc31zw3l3kq0q4wd5cp3lqpkq0q-source/pkgs/development/interpreters/python/cpython/2.7/default.nix:335 is marked as insecure, refusing to evaluate. ``` Errors emitted in nested structures (like e.g. when printing `nixpkgs`) will still be printed in brackets. Change-Id: I25aeddf08c017582718cb9772a677bf51b9fc2ad --- .../rl-next/better-errors-in-nix-repl.md | 1 + src/libcmd/repl.cc | 3 +- src/libexpr/print-options.hh | 30 ++++- src/libexpr/print.cc | 120 +++++++++--------- .../repl_characterization/data/errors.test | 27 ++++ .../repl_characterization.cc | 1 + 6 files changed, 120 insertions(+), 62 deletions(-) create mode 100644 tests/functional/repl_characterization/data/errors.test diff --git a/doc/manual/rl-next/better-errors-in-nix-repl.md b/doc/manual/rl-next/better-errors-in-nix-repl.md index 3eefdbcbb..14ead8d0c 100644 --- a/doc/manual/rl-next/better-errors-in-nix-repl.md +++ b/doc/manual/rl-next/better-errors-in-nix-repl.md @@ -1,6 +1,7 @@ --- synopsis: Concise error printing in `nix repl` prs: 9928 +cls: 811 --- Previously, if an element of a list or attribute set threw an error while diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 4aa46dbc0..9811ea30c 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -167,7 +167,8 @@ struct NixRepl .force = true, .derivationPaths = true, .maxDepth = maxDepth, - .prettyIndent = 2 + .prettyIndent = 2, + .errors = ErrorPrintBehavior::ThrowTopLevel, }); } }; diff --git a/src/libexpr/print-options.hh b/src/libexpr/print-options.hh index 6c5e80c61..080ba26b8 100644 --- a/src/libexpr/print-options.hh +++ b/src/libexpr/print-options.hh @@ -8,6 +8,29 @@ namespace nix { +/** + * How errors should be handled when printing values. + */ +enum class ErrorPrintBehavior { + /** + * Print the first line of the error in brackets: `«error: oh no!»` + */ + Print, + /** + * Throw the error to the code that attempted to print the value, instead + * of suppressing it it. + */ + Throw, + /** + * Only throw the error if encountered at the top level of the expression. + * + * This will cause expressions like `builtins.throw "uh oh!"` to throw + * errors, but will print attribute sets and other nested structures + * containing values that error (like `nixpkgs`) normally. + */ + ThrowTopLevel, +}; + /** * Options for printing Nix values. */ @@ -68,6 +91,11 @@ struct PrintOptions */ size_t prettyIndent = 0; + /** + * How to handle errors encountered while printing values. + */ + ErrorPrintBehavior errors = ErrorPrintBehavior::Print; + /** * True if pretty-printing is enabled. */ @@ -86,7 +114,7 @@ static PrintOptions errorPrintOptions = PrintOptions { .maxDepth = 10, .maxAttrs = 10, .maxListItems = 10, - .maxStringLength = 1024 + .maxStringLength = 1024, }; } diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 8d7e2ab34..231bde0a0 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -229,25 +229,21 @@ private: void printDerivation(Value & v) { - try { - Bindings::iterator i = v.attrs->find(state.sDrvPath); - NixStringContext context; - std::string storePath; - if (i != v.attrs->end()) - storePath = state.store->printStorePath(state.coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation")); + Bindings::iterator i = v.attrs->find(state.sDrvPath); + NixStringContext context; + std::string storePath; + if (i != v.attrs->end()) + storePath = state.store->printStorePath(state.coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation")); - if (options.ansiColors) - output << ANSI_GREEN; - output << "«derivation"; - if (!storePath.empty()) { - output << " " << storePath; - } - output << "»"; - if (options.ansiColors) - output << ANSI_NORMAL; - } catch (Error & e) { - printError_(e); + if (options.ansiColors) + output << ANSI_GREEN; + output << "«derivation"; + if (!storePath.empty()) { + output << " " << storePath; } + output << "»"; + if (options.ansiColors) + output << ANSI_NORMAL; } bool shouldPrettyPrintAttrs(AttrVec & v) @@ -472,64 +468,68 @@ private: output.flush(); checkInterrupt(); - if (options.force) { - try { + try { + if (options.force) { state.forceValue(v, v.determinePos(noPos)); - } catch (Error & e) { - printError_(e); - return; } - } - switch (v.type()) { + switch (v.type()) { - case nInt: - printInt(v); - break; + case nInt: + printInt(v); + break; - case nFloat: - printFloat(v); - break; + case nFloat: + printFloat(v); + break; - case nBool: - printBool(v); - break; + case nBool: + printBool(v); + break; - case nString: - printString(v); - break; + case nString: + printString(v); + break; - case nPath: - printPath(v); - break; + case nPath: + printPath(v); + break; - case nNull: - printNull(); - break; + case nNull: + printNull(); + break; - case nAttrs: - printAttrs(v, depth); - break; + case nAttrs: + printAttrs(v, depth); + break; - case nList: - printList(v, depth); - break; + case nList: + printList(v, depth); + break; - case nFunction: - printFunction(v); - break; + case nFunction: + printFunction(v); + break; - case nThunk: - printThunk(v); - break; + case nThunk: + printThunk(v); + break; - case nExternal: - printExternal(v); - break; + case nExternal: + printExternal(v); + break; - default: - printUnknown(); - break; + default: + printUnknown(); + break; + } + } catch (Error & e) { + if (options.errors == ErrorPrintBehavior::Throw + || (options.errors == ErrorPrintBehavior::ThrowTopLevel + && depth == 0)) { + throw; + } + printError_(e); } } diff --git a/tests/functional/repl_characterization/data/errors.test b/tests/functional/repl_characterization/data/errors.test new file mode 100644 index 000000000..47d7e7e13 --- /dev/null +++ b/tests/functional/repl_characterization/data/errors.test @@ -0,0 +1,27 @@ +Errors at the top of an expression are printed normally: + + nix-repl> builtins.throw "Evil puppy detected!!!" + error: + … while calling the 'throw' builtin + at «string»:1:1: + 1| builtins.throw "Evil puppy detected!!!" + | ^ + + error: Evil puppy detected!!! + +Errors in attribute values are printed inline, to make it easier to explore +values like nixpkgs where some parts of the value fail to evaluate: + + nix-repl> { puppy = builtins.throw "This puppy is EVIL!!!"; puppy2 = "This puppy is GOOD :)"; } + { + puppy = «error: This puppy is EVIL!!!»; + puppy2 = "This puppy is GOOD :)"; + } + +Same for list values: + + nix-repl> [ (builtins.throw "This puppy is EVIL!!!") ("This puppy is GOOD :)") ] + [ + «error: This puppy is EVIL!!!» + "This puppy is GOOD :)" + ] diff --git a/tests/functional/repl_characterization/repl_characterization.cc b/tests/functional/repl_characterization/repl_characterization.cc index f84bbdaae..0d3e5352d 100644 --- a/tests/functional/repl_characterization/repl_characterization.cc +++ b/tests/functional/repl_characterization/repl_characterization.cc @@ -186,5 +186,6 @@ REPL_TEST(repl_overlays_destructure_without_formals_ok); REPL_TEST(repl_overlays_error); REPL_TEST(repl_printing); REPL_TEST(stack_vars); +REPL_TEST(errors); }; // namespace nix