Fix builtins.tryEval and error catching #1098

Open
opened 2026-01-08 12:00:14 +00:00 by piegames · 2 comments
Member

The introduction of builtins.tryEval in its current form has made a lot of people very angry and been widely regarded as a bad move. The most prominent issues are:

  • It can only catch explicit throws and asserts, and not any other runtime evaluation errors. Notably, it cannot catch type errors, missing attributes or nulls. This means that it is generally fit for the use case of "try to wrap this code which may not evaluate and recover gracefully".
  • It makes maintenance of the evaluator code base a nightmare
    • It turns optimizations on evaluation order into observable semantics and thus breaking changes. For example, given [ my list ] ++ "zero" ++ (throw "error") the type checking of the first two arguments must be eager before the evaluation of the last argument, because changing it to be more lazy would turn the type error (not catchable) into a throw error (catchable)
    • It heavily relies on the C++ exception throwing semantics and the general way the evaluator works, preventing both rewrites in a language like Rust (which uses Results over exceptions for error handling) or bytecode/JIT optimization stages (implementing an exception handling runtime within a VM is decidedly not fun, nor performant)
  • Generally, the question which errors should be able to be caught is one that quickly becomes very nuanced, as it boils down to the fine line between which errors we should consider excpected vs unexpected.

Given the current limitations, no easy improvements can be made. Here are some options:

  • Build some taint tracking on the output of tryEval. Unclear how to deal with that taint though and also this sounds prohibitively expensive
  • If we had a type system we could turn this into monads (lolsob)
  • Design a better tryEval2 (and, unrelatedly, a better toString2) that is better at catching all meaningful errors. Might require langver, and does not mitigate the implementation-side issues
  • Deprecate and remove tryEval altogether, possibly in favor of some top-level CLI error handling facilities (e.g. for the Nixpkgs test suite which currently uses tryEval)
  • Deprecate-rename tryEval to unsafeTryEval and drop all purity and stability guarantees on the output of that function. This allows the Nixpkgs tests still to work while allowing us to evolve the evaluator without having to care too much. Other usage in Nixpkgs, especially in derivations, would be forbidden.
    • Alternatively, remove tryEval from pure evluation contexts

The last solution is the one I currently prefer most and would like to implement going forwards. Some of the solutions are not mutually exclusive, and interim solutions that will be complemented by more proper solutions at a later point in time are a valid option.

The introduction of `builtins.tryEval` in its current form has made a lot of people very angry and been widely regarded as a bad move. The most prominent issues are: - It can only catch explicit `throw`s and `assert`s, and not any other runtime evaluation errors. Notably, it cannot catch type errors, missing attributes or nulls. This means that it is generally fit for the use case of "try to wrap this code which may not evaluate and recover gracefully". - It cannot catch errors from builtins that *feel* catchable, leading to workarounds like https://github.com/andir/npins/blob/3329242072a414c5a25aa8d95a56d8afa013a3fa/src/default.nix#L232-L238 - It makes maintenance of the evaluator code base a nightmare - It turns optimizations on evaluation order into observable semantics and thus breaking changes. For example, given `[ my list ] ++ "zero" ++ (throw "error")` the type checking of the first two arguments *must* be eager before the evaluation of the last argument, because changing it to be more lazy would turn the type error (not catchable) into a throw error (catchable) - It heavily relies on the C++ exception throwing semantics and the general way the evaluator works, preventing both rewrites in a language like Rust (which uses `Result`s over exceptions for error handling) or bytecode/JIT optimization stages (implementing an exception handling runtime within a VM is decidedly *not* fun, nor performant) - Generally, the question which errors *should* be able to be caught is one that quickly becomes very nuanced, as it boils down to the fine line between which errors we should consider excpected vs unexpected. Given the current limitations, no easy improvements can be made. Here are some options: - Build some taint tracking on the output of `tryEval`. Unclear how to deal with that taint though and also this sounds prohibitively expensive - If we had a type system we could turn this into monads (lolsob) - Design a better `tryEval2` (and, unrelatedly, a better `toString2`) that is better at catching all meaningful errors. Might require langver, and does not mitigate the implementation-side issues - Deprecate and remove `tryEval` altogether, possibly in favor of some top-level CLI error handling facilities (e.g. for the Nixpkgs test suite which currently uses `tryEval`) - Deprecate-rename `tryEval` to `unsafeTryEval` and drop all purity and stability guarantees on the output of that function. This allows the Nixpkgs tests still to work while allowing us to evolve the evaluator without having to care too much. Other usage in Nixpkgs, especially in derivations, would be forbidden. - Alternatively, remove `tryEval` from pure evluation contexts The last solution is the one I currently prefer most and would like to implement going forwards. Some of the solutions are not mutually exclusive, and interim solutions that will be complemented by more proper solutions at a later point in time are a valid option.
Owner

Build some taint tracking on the output of tryEval. Unclear how to deal with that taint though and also this sounds prohibitively expensive

Reminds us a little of the "poison context" idea that was floated around a while ago. The most common case for throwing in Nixpkgs is to prevent a derivation from being instantiated without e.g. config.allowUnfree. That doesn't over cases like someHandler.${system} or (throw "unsupported system ${system}"), though.

> Build some taint tracking on the output of tryEval. Unclear how to deal with that taint though and also this sounds prohibitively expensive Reminds us a little of the "poison context" idea that was floated around a while ago. The most common case for `throw`ing in *Nixpkgs* is to prevent a derivation from being instantiated without e.g. `config.allowUnfree`. That doesn't over cases like `someHandler.${system} or (throw "unsupported system ${system}")`, though.
Owner

we're also in favor of deprecate-renaming tryEval as a first step. long-term we should either remove it entirely (and add result-returning variants to all builtins that cannot have pre-checks, like readFile in the linked example), or make it catch all eval errors unsafely including type errors and parse errors (which aren't currently eval errors at all). in fact we should add result-returning variants for all builtins that are fallible due to side effects regardless of what we do about tryEval.

@qyriad wrote in #1098 (comment):

Reminds us a little of the "poison context" idea that was floated around a while ago. The most common case for throwing in Nixpkgs is to prevent a derivation from being instantiated without e.g. config.allowUnfree. That doesn't over cases like someHandler.${system} or (throw "unsupported system ${system}"), though.

poison context underapproximates this due to context discarding and sequencing. we'd argue that the nixpkgs checks would be better approximated with an assertion that can deliver a custom message if the goal is to prevent instantiation. assertions having printed a facsimile of their predicate AST in the past does suggest something like this was the goal at some point, but we can't be sure whether it was this or that there were no source annotations for errors when assert was introduced :/

we're also in favor of deprecate-renaming tryEval as a first step. long-term we should either remove it entirely (and add result-returning variants to all builtins that cannot have pre-checks, like readFile in the linked example), or make it catch *all* eval errors unsafely *including* type errors and parse errors (which aren't currently eval errors at all). in fact we should add result-returning variants for all builtins that are fallible due to side effects regardless of what we do about tryEval. @qyriad wrote in https://git.lix.systems/lix-project/lix/issues/1098#issuecomment-17135: > Reminds us a little of the "poison context" idea that was floated around a while ago. The most common case for `throw`ing in _Nixpkgs_ is to prevent a derivation from being instantiated without e.g. `config.allowUnfree`. That doesn't over cases like `someHandler.${system} or (throw "unsupported system ${system}")`, though. poison context underapproximates this due to context discarding and `seq`uencing. we'd argue that the nixpkgs checks would be better approximated with an assertion that can deliver a custom message if the goal is to prevent *instantiation*. assertions having printed a facsimile of their predicate AST in the past does suggest something like this was the goal at some point, but we can't be sure whether it was this or that there were no source annotations for errors when assert was introduced :/
Sign in to join this conversation.
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lix-project/lix#1098
No description provided.