From 89158eedb550638e79a7fe9f1ea4abfc06beabe4 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 18 Feb 2026 19:42:25 +0300 Subject: [PATCH 1/6] treewide: Make exceptions cloneable This is needed to make it possible to store exceptions in failed values with each new rethrow getting a fresh copy of the exception object. --- src/libexpr/eval-cache.cc | 2 +- src/libexpr/include/nix/expr/eval-cache.hh | 2 +- src/libexpr/include/nix/expr/eval-error.hh | 14 +++---- src/libexpr/include/nix/expr/value/context.hh | 4 +- src/libexpr/primops.cc | 11 ++--- src/libfetchers/git-utils.cc | 4 +- src/libstore/aws-creds.cc | 2 +- src/libstore/build/goal.cc | 4 +- src/libstore/filetransfer.cc | 6 +-- src/libstore/include/nix/store/aws-creds.hh | 5 ++- .../include/nix/store/build-result.hh | 8 ++-- .../nix/store/build/derivation-builder.hh | 4 +- src/libstore/include/nix/store/build/goal.hh | 2 +- .../include/nix/store/builtins/buildenv.hh | 4 +- .../include/nix/store/filetransfer.hh | 2 +- src/libstore/include/nix/store/realisation.hh | 4 +- src/libstore/include/nix/store/sqlite.hh | 2 +- src/libstore/sqlite.cc | 2 +- src/libstore/ssh.cc | 4 +- src/libstore/unix/build/derivation-builder.cc | 4 +- src/libutil/experimental-features.cc | 2 +- src/libutil/include/nix/util/error.hh | 40 ++++++++++++++----- .../include/nix/util/experimental-features.hh | 2 +- src/libutil/include/nix/util/processes.hh | 4 +- .../include/nix/util/source-accessor.hh | 6 +-- 25 files changed, 82 insertions(+), 62 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 4f6c5fe8e78..b2d510ce031 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -11,7 +11,7 @@ namespace nix::eval_cache { CachedEvalError::CachedEvalError(ref cursor, Symbol attr) - : EvalError(cursor->root->state, "cached failure of attribute '%s'", cursor->getAttrPathStr(attr)) + : CloneableError(cursor->root->state, "cached failure of attribute '%s'", cursor->getAttrPathStr(attr)) , cursor(cursor) , attr(attr) { diff --git a/src/libexpr/include/nix/expr/eval-cache.hh b/src/libexpr/include/nix/expr/eval-cache.hh index 6d82f8c7e35..165b27420c4 100644 --- a/src/libexpr/include/nix/expr/eval-cache.hh +++ b/src/libexpr/include/nix/expr/eval-cache.hh @@ -14,7 +14,7 @@ namespace nix::eval_cache { struct AttrDb; class AttrCursor; -struct CachedEvalError : EvalError +struct CachedEvalError : CloneableError { const ref cursor; const Symbol attr; diff --git a/src/libexpr/include/nix/expr/eval-error.hh b/src/libexpr/include/nix/expr/eval-error.hh index 06a7790d5e4..24717d40bb1 100644 --- a/src/libexpr/include/nix/expr/eval-error.hh +++ b/src/libexpr/include/nix/expr/eval-error.hh @@ -18,7 +18,7 @@ class EvalErrorBuilder; * * Most subclasses should inherit from `EvalError` instead of this class. */ -class EvalBaseError : public Error +class EvalBaseError : public CloneableError { template friend class EvalErrorBuilder; @@ -26,14 +26,14 @@ public: EvalState & state; EvalBaseError(EvalState & state, ErrorInfo && errorInfo) - : Error(errorInfo) + : CloneableError(errorInfo) , state(state) { } template explicit EvalBaseError(EvalState & state, const std::string & formatString, const Args &... formatArgs) - : Error(formatString, formatArgs...) + : CloneableError(formatString, formatArgs...) , state(state) { } @@ -60,23 +60,23 @@ MakeError(InfiniteRecursionError, EvalError); * Inherits from EvalBaseError (not EvalError) because resource exhaustion * should not be cached. */ -struct StackOverflowError : public EvalBaseError +struct StackOverflowError : public CloneableError { StackOverflowError(EvalState & state) - : EvalBaseError(state, "stack overflow; max-call-depth exceeded") + : CloneableError(state, "stack overflow; max-call-depth exceeded") { } }; MakeError(IFDError, EvalBaseError); -struct InvalidPathError : public EvalError +struct InvalidPathError : public CloneableError { public: Path path; InvalidPathError(EvalState & state, const Path & path) - : EvalError(state, "path '%s' is not valid", path) + : CloneableError(state, "path '%s' is not valid", path) { } }; diff --git a/src/libexpr/include/nix/expr/value/context.hh b/src/libexpr/include/nix/expr/value/context.hh index 054516bc268..9884c270504 100644 --- a/src/libexpr/include/nix/expr/value/context.hh +++ b/src/libexpr/include/nix/expr/value/context.hh @@ -9,14 +9,14 @@ namespace nix { -class BadNixStringContextElem : public Error +class BadNixStringContextElem final : public CloneableError { public: std::string_view raw; template BadNixStringContextElem(std::string_view raw_, const Args &... args) - : Error("") + : CloneableError("") { raw = raw_; auto hf = HintFmt(args...); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index b7b80be66a7..db57556149e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1312,11 +1312,12 @@ static void prim_warn(EvalState & state, const PosIdx pos, Value ** args, Value state.forceString(*args[0], pos, "while evaluating the first argument; the message passed to builtins.warn"); { - BaseError msg(std::string{msgStr}); - msg.atPos(state.positions[pos]); - auto info = msg.info(); - info.level = lvlWarn; - info.isFromExpr = true; + ErrorInfo info{ + .level = lvlWarn, + .msg = HintFmt(std::string(msgStr)), + .pos = state.positions[pos], + .isFromExpr = true, + }; logWarning(info); } diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index e3bce13b215..d440c3c7076 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -74,11 +74,11 @@ namespace nix { struct GitSourceAccessor; -struct GitError : public Error +struct GitError final : public CloneableError { template GitError(const git_error & error, Ts &&... args) - : Error("") + : CloneableError("") { auto hf = HintFmt(std::forward(args)...); err.msg = HintFmt("%1%: %2% (libgit2 error code = %3%)", Uncolored(hf.str()), error.message, error.klass); diff --git a/src/libstore/aws-creds.cc b/src/libstore/aws-creds.cc index 9b0ddefdca8..5baac798ced 100644 --- a/src/libstore/aws-creds.cc +++ b/src/libstore/aws-creds.cc @@ -28,7 +28,7 @@ namespace nix { AwsAuthError::AwsAuthError(int errorCode) - : Error("AWS authentication error: '%s' (%d)", aws_error_str(errorCode), errorCode) + : CloneableError("AWS authentication error: '%s' (%d)", aws_error_str(errorCode), errorCode) , errorCode(errorCode) { } diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index d64b869e9e1..ef3501b0091 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -1,11 +1,11 @@ #include "nix/store/build/goal.hh" #include "nix/store/build/worker.hh" -#include "nix/store/globals.hh" +#include "nix/store/worker-settings.hh" namespace nix { TimedOut::TimedOut(time_t maxDuration) - : BuildError(BuildResult::Failure::TimedOut, "timed out after %1% seconds", maxDuration) + : CloneableError(BuildResult::Failure::TimedOut, "timed out after %1% seconds", maxDuration) , maxDuration(maxDuration) { } diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index b8a9a9aa86e..8e055c14177 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -60,12 +60,12 @@ namespace { using curlSList = std::unique_ptr<::curl_slist, decltype([](::curl_slist * list) { ::curl_slist_free_all(list); })>; using curlMulti = std::unique_ptr<::CURLM, decltype([](::CURLM * multi) { ::curl_multi_cleanup(multi); })>; -struct curlMultiError : Error +struct curlMultiError final : CloneableError { ::CURLMcode code; curlMultiError(::CURLMcode code) - : Error{"unexpected curl multi error: %s", ::curl_multi_strerror(code)} + : CloneableError{"unexpected curl multi error: %s", ::curl_multi_strerror(code)} { assert(code != CURLM_OK); } @@ -1212,7 +1212,7 @@ void FileTransfer::download( template FileTransferError::FileTransferError( FileTransfer::Error error, std::optional response, const Args &... args) - : Error(args...) + : CloneableError(args...) , error(error) , response(response) { diff --git a/src/libstore/include/nix/store/aws-creds.hh b/src/libstore/include/nix/store/aws-creds.hh index 30f6592a06f..0751757cb01 100644 --- a/src/libstore/include/nix/store/aws-creds.hh +++ b/src/libstore/include/nix/store/aws-creds.hh @@ -34,12 +34,13 @@ struct AwsCredentials } }; -class AwsAuthError : public Error +class AwsAuthError final : public CloneableError { std::optional errorCode; public: - using Error::Error; + using CloneableError::CloneableError; + AwsAuthError(int errorCode); std::optional getErrorCode() const diff --git a/src/libstore/include/nix/store/build-result.hh b/src/libstore/include/nix/store/build-result.hh index ce63d099960..c664e6e5b6f 100644 --- a/src/libstore/include/nix/store/build-result.hh +++ b/src/libstore/include/nix/store/build-result.hh @@ -58,7 +58,7 @@ enum struct BuildResultFailureStatus : uint8_t { * This is both an exception type (inherits from Error) and serves as * the failure variant in BuildResult::inner. */ -struct BuildError : public Error +struct BuildError : public CloneableError { using Status = BuildResultFailureStatus; using enum Status; @@ -80,7 +80,7 @@ public: */ template BuildError(Status status, const Args &... args) - : Error(args...) + : CloneableError(args...) , status{status} { } @@ -97,7 +97,7 @@ public: * Also used for deserialization. */ BuildError(Args args) - : Error(std::move(args.msg)) + : CloneableError(std::move(args.msg)) , status{args.status} , isNonDeterministic{args.isNonDeterministic} @@ -108,7 +108,7 @@ public: * Default constructor for deserialization. */ BuildError() - : Error("") + : CloneableError("") { } diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index a56cbe3c252..3b8f305a70f 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -20,14 +20,14 @@ namespace nix { * Denotes a build failure that stemmed from the builder exiting with a * failing exist status. */ -struct BuilderFailureError : BuildError +struct BuilderFailureError final : CloneableError { int builderStatus; std::string extraMsgAfter; BuilderFailureError(BuildResult::Failure::Status status, int builderStatus, std::string extraMsgAfter) - : BuildError{ + : CloneableError{ status, /* No message for now, because the caller will make for us, with extra context */ diff --git a/src/libstore/include/nix/store/build/goal.hh b/src/libstore/include/nix/store/build/goal.hh index 6a92d282898..263647fc126 100644 --- a/src/libstore/include/nix/store/build/goal.hh +++ b/src/libstore/include/nix/store/build/goal.hh @@ -10,7 +10,7 @@ namespace nix { -struct TimedOut : BuildError +struct TimedOut final : CloneableError { time_t maxDuration; diff --git a/src/libstore/include/nix/store/builtins/buildenv.hh b/src/libstore/include/nix/store/builtins/buildenv.hh index c152ab00af5..136a1ab5a77 100644 --- a/src/libstore/include/nix/store/builtins/buildenv.hh +++ b/src/libstore/include/nix/store/builtins/buildenv.hh @@ -22,7 +22,7 @@ struct Package } }; -class BuildEnvFileConflictError : public Error +class BuildEnvFileConflictError final : public CloneableError { public: const Path fileA; @@ -30,7 +30,7 @@ public: int priority; BuildEnvFileConflictError(const Path fileA, const Path fileB, int priority) - : Error( + : CloneableError( "Unable to build profile. There is a conflict for the following files:\n" "\n" " %1%\n" diff --git a/src/libstore/include/nix/store/filetransfer.hh b/src/libstore/include/nix/store/filetransfer.hh index 48c1b5a2f47..e34c3692a27 100644 --- a/src/libstore/include/nix/store/filetransfer.hh +++ b/src/libstore/include/nix/store/filetransfer.hh @@ -403,7 +403,7 @@ ref getFileTransfer(); */ ref makeFileTransfer(const FileTransferSettings & settings = fileTransferSettings); -class FileTransferError : public Error +class FileTransferError final : public CloneableError { public: FileTransfer::Error error; diff --git a/src/libstore/include/nix/store/realisation.hh b/src/libstore/include/nix/store/realisation.hh index 55597acb7c0..670f8ce499e 100644 --- a/src/libstore/include/nix/store/realisation.hh +++ b/src/libstore/include/nix/store/realisation.hh @@ -146,7 +146,7 @@ struct RealisedPath auto operator<=>(const RealisedPath &) const = default; }; -class MissingRealisation : public Error +class MissingRealisation final : public CloneableError { public: MissingRealisation(DrvOutput & outputId) @@ -155,7 +155,7 @@ public: } MissingRealisation(std::string_view drv, OutputName outputName) - : Error( + : CloneableError( "cannot operate on output '%s' of the " "unbuilt derivation '%s'", outputName, diff --git a/src/libstore/include/nix/store/sqlite.hh b/src/libstore/include/nix/store/sqlite.hh index 83839d3852c..be14fde1938 100644 --- a/src/libstore/include/nix/store/sqlite.hh +++ b/src/libstore/include/nix/store/sqlite.hh @@ -166,7 +166,7 @@ struct SQLiteTxn ~SQLiteTxn(); }; -struct SQLiteError : Error +struct SQLiteError : CloneableError { std::string path; std::string errMsg; diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 498e2deda0c..16561f3b2b1 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -17,7 +17,7 @@ namespace nix { SQLiteError::SQLiteError( const char * path, const char * errMsg, int errNo, int extendedErrNo, int offset, HintFmt && hf) - : Error("") + : CloneableError("") , path(path) , errMsg(errMsg) , errNo(errNo) diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 1a99083669c..a5014b2c89b 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -18,11 +18,11 @@ static std::string parsePublicHostKey(std::string_view host, std::string_view ss } } -class InvalidSSHAuthority : public Error +class InvalidSSHAuthority final : public CloneableError { public: InvalidSSHAuthority(const ParsedURL::Authority & authority, std::string_view reason) - : Error("invalid SSH authority: '%s': %s", authority.to_string(), reason) + : CloneableError("invalid SSH authority: '%s': %s", authority.to_string(), reason) { } }; diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index 504063da097..981da9cf2d8 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -55,10 +55,10 @@ namespace nix { -struct NotDeterministic : BuildError +struct NotDeterministic final : CloneableError { NotDeterministic(auto &&... args) - : BuildError(BuildResult::Failure::NotDeterministic, args...) + : CloneableError(BuildResult::Failure::NotDeterministic, args...) { isNonDeterministic = true; } diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 20d1a082a66..727365833fe 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -378,7 +378,7 @@ std::set parseFeatures(const StringSet & rawFeatures) } MissingExperimentalFeature::MissingExperimentalFeature(ExperimentalFeature feature, std::string reason) - : Error( + : CloneableError( "experimental Nix feature '%1%' is disabled%2%; add '--extra-experimental-features %1%' to enable it", showExperimentalFeature(feature), Uncolored(optionalBracket(" (", reason, ")"))) diff --git a/src/libutil/include/nix/util/error.hh b/src/libutil/include/nix/util/error.hh index d9babf34d1c..ea18b0a30fb 100644 --- a/src/libutil/include/nix/util/error.hh +++ b/src/libutil/include/nix/util/error.hh @@ -227,13 +227,31 @@ public: { return err; }; + + [[noreturn]] virtual void throwClone() const = 0; +}; + +template +class CloneableError : public Base +{ +public: + using Base::Base; + + /** + * Rethrow a copy of this exception. Useful when the exception can get + * modified when appending traces. + */ + [[noreturn]] void throwClone() const override + { + throw Derived(static_cast(*this)); + } }; -#define MakeError(newClass, superClass) \ - class newClass : public superClass \ - { \ - public: \ - using superClass::superClass; \ +#define MakeError(newClass, superClass) \ + class newClass : public CloneableError \ + { \ + public: \ + using CloneableError::CloneableError; \ } MakeError(Error, BaseError); @@ -245,7 +263,7 @@ MakeError(UnimplementedError, Error); * std::error_code. Use when you want to catch and check an error condition like * no_such_file_or_directory (ENOENT) without ifdefs. */ -class SystemError : public Error +class SystemError : public CloneableError { std::error_code errorCode; std::string errorDetails; @@ -265,7 +283,7 @@ protected: */ template SystemError(Disambig, std::error_code errorCode, std::string_view errorDetails, Args &&... args) - : Error("") + : CloneableError("") , errorCode(errorCode) , errorDetails(errorDetails) { @@ -311,7 +329,7 @@ public: * support is too WIP to justify the code churn, but if it is finished * then a better identifier becomes moe worth it. */ -class SysError : public SystemError +class SysError final : public CloneableError { public: int errNo; @@ -322,7 +340,7 @@ public: */ template SysError(int errNo, Args &&... args) - : SystemError( + : CloneableError( Disambig{}, std::make_error_code(static_cast(errNo)), strerror(errNo), @@ -392,7 +410,7 @@ namespace windows { * Unless you need to catch a specific error number, don't catch this in * portable code. Catch `SystemError` instead. */ -class WinError : public SystemError +class WinError : public CloneableError { public: DWORD lastError; @@ -404,7 +422,7 @@ public: */ template WinError(DWORD lastError, Args &&... args) - : SystemError( + : CloneableError( Disambig{}, std::error_code(lastError, std::system_category()), renderError(lastError), diff --git a/src/libutil/include/nix/util/experimental-features.hh b/src/libutil/include/nix/util/experimental-features.hh index aca14bfbb41..1a4c9b6b574 100644 --- a/src/libutil/include/nix/util/experimental-features.hh +++ b/src/libutil/include/nix/util/experimental-features.hh @@ -80,7 +80,7 @@ std::set parseFeatures(const StringSet &); * An experimental feature was required for some (experimental) * operation, but was not enabled. */ -class MissingExperimentalFeature : public Error +class MissingExperimentalFeature final : public CloneableError { public: /** diff --git a/src/libutil/include/nix/util/processes.hh b/src/libutil/include/nix/util/processes.hh index 44e8c2952d3..3b9cff22ceb 100644 --- a/src/libutil/include/nix/util/processes.hh +++ b/src/libutil/include/nix/util/processes.hh @@ -133,14 +133,14 @@ std::pair runProgram(RunOptions && options); void runProgram2(const RunOptions & options); -class ExecError : public Error +class ExecError final : public CloneableError { public: int status; template ExecError(int status, const Args &... args) - : Error(args...) + : CloneableError(args...) , status(status) { } diff --git a/src/libutil/include/nix/util/source-accessor.hh b/src/libutil/include/nix/util/source-accessor.hh index e1de9ece7f3..3a72f800528 100644 --- a/src/libutil/include/nix/util/source-accessor.hh +++ b/src/libutil/include/nix/util/source-accessor.hh @@ -231,19 +231,19 @@ ref makeEmptySourceAccessor(); */ MakeError(RestrictedPathError, Error); -struct SymlinkNotAllowed : public Error +struct SymlinkNotAllowed final : public CloneableError { CanonPath path; SymlinkNotAllowed(CanonPath path) - : Error("relative path '%s' points to a symlink, which is not allowed", path.rel()) + : CloneableError("relative path '%s' points to a symlink, which is not allowed", path.rel()) , path(std::move(path)) { } template SymlinkNotAllowed(CanonPath path, const std::string & fs, Args &&... args) - : Error(fs, std::forward(args)...) + : CloneableError(fs, std::forward(args)...) , path(std::move(path)) { } From c33d9e31cc3a2f71346e7b587c04a8c8619d546a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 7 Sep 2025 11:44:15 +0200 Subject: [PATCH 2/6] Introduce a "failed" value type In the multithreaded evaluator, it's possible for multiple threads to wait on the same thunk. If evaluation of the thunk results in an exception, the waiting threads shouldn't try to re-force the thunk. Instead, they should rethrow the same exception, without duplicating any work. Therefore, there is now a new value type `tFailed` that stores an std::exception_ptr. If evaluation of a thunk/app results in an exception, `forceValue()` overwrites the value with a `tFailed`. If `forceValue()` encounters a `tFailed`, it rethrows the exception. So you normally never need to check for failed values (since forcing them causes a rethrow). Co-authored-by: Robert Hensing --- src/libexpr-c/nix_api_value.cc | 2 + src/libexpr-c/nix_api_value.h | 3 +- src/libexpr/eval.cc | 16 +++-- src/libexpr/include/nix/expr/eval-gc.hh | 3 + src/libexpr/include/nix/expr/eval-inline.hh | 17 +++-- src/libexpr/include/nix/expr/value.hh | 65 +++++++++++++++++-- src/libexpr/primops.cc | 1 + src/libexpr/print-ambiguous.cc | 5 ++ src/libexpr/print.cc | 15 +++++ src/libexpr/value-to-json.cc | 1 + src/libexpr/value-to-xml.cc | 4 ++ ...l-okay-tryeval-failed-thunk-reeval.err.exp | 1 + .../eval-okay-tryeval-failed-thunk-reeval.exp | 1 + .../eval-okay-tryeval-failed-thunk-reeval.nix | 7 ++ 14 files changed, 126 insertions(+), 15 deletions(-) create mode 100644 tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.err.exp create mode 100644 tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.exp create mode 100644 tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.nix diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 7fd8233adec..06570abc338 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -194,6 +194,8 @@ ValueType nix_get_type(nix_c_context * context, const nix_value * value) switch (v.type()) { case nThunk: return NIX_TYPE_THUNK; + case nFailed: + return NIX_TYPE_FAILED; case nInt: return NIX_TYPE_INT; case nFloat: diff --git a/src/libexpr-c/nix_api_value.h b/src/libexpr-c/nix_api_value.h index 5bd45da9059..0220bf68e2d 100644 --- a/src/libexpr-c/nix_api_value.h +++ b/src/libexpr-c/nix_api_value.h @@ -100,7 +100,8 @@ typedef enum { /** @brief External value from C++ plugins or C API * @see Externals */ - NIX_TYPE_EXTERNAL + NIX_TYPE_EXTERNAL, + NIX_TYPE_FAILED, } ValueType; // forward declarations diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index ba52e813ba6..defb4d02dbd 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -155,6 +155,8 @@ std::string_view showType(ValueType type, bool withArticle) return WA("a", "float"); case nThunk: return WA("a", "thunk"); + case nFailed: + return WA("an", "error"); } unreachable(); } @@ -2825,8 +2827,11 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st } return; - case nThunk: // Must not be left by forceValue - assert(false); + // Cannot be returned by forceValue(). + case nThunk: + case nFailed: + unreachable(); + default: // Note that we pass compiler flags that should make `default:` unreachable. // Also note that this probably ran after `eqValues`, which implements // the same logic more efficiently (without having to unwind stacks), @@ -2920,8 +2925,11 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v // !!! return v1.fpoint() == v2.fpoint(); - case nThunk: // Must not be left by forceValue - assert(false); + // Cannot be returned by forceValue(). + case nThunk: + case nFailed: + unreachable(); + default: // Note that we pass compiler flags that should make `default:` unreachable. error("eqValues: cannot compare %1% with %2%", showType(v1), showType(v2)) .withTrace(pos, errorCtx) diff --git a/src/libexpr/include/nix/expr/eval-gc.hh b/src/libexpr/include/nix/expr/eval-gc.hh index 813c2920d0e..1b1eb7e627f 100644 --- a/src/libexpr/include/nix/expr/eval-gc.hh +++ b/src/libexpr/include/nix/expr/eval-gc.hh @@ -33,6 +33,9 @@ using gc_allocator = std::allocator; struct gc {}; +struct gc_cleanup +{}; + #endif namespace nix { diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index 561c07f0891..33aa6b9b47f 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/src/libexpr/include/nix/expr/eval-inline.hh @@ -91,18 +91,25 @@ void EvalState::forceValue(Value & v, const PosIdx pos) Expr * expr = v.thunk().expr; try { v.mkBlackhole(); - // checkInterrupt(); if (env) [[likely]] expr->eval(*this, *env, v); else ExprBlackHole::throwInfiniteRecursionError(*this, v); } catch (...) { - v.mkThunk(env, expr); - tryFixupBlackHolePos(v, pos); + if (!env) + tryFixupBlackHolePos(v, pos); + v.mkFailed(); throw; } - } else if (v.isApp()) - callFunction(*v.app().left, *v.app().right, v, pos); + } else if (v.isApp()) { + try { + callFunction(*v.app().left, *v.app().right, v, pos); + } catch (...) { + v.mkFailed(); + throw; + } + } else if (v.isFailed()) + v.failed().rethrow(); } [[gnu::always_inline]] diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 5b317e7acfc..59969079c5c 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -42,6 +42,7 @@ enum InternalType { tBool, tNull, tFloat, + tFailed, tExternal, tPrimOp, tAttrs, @@ -68,6 +69,7 @@ enum InternalType { */ typedef enum { nThunk, + nFailed, nInt, nFloat, nBool, @@ -424,6 +426,30 @@ struct ValueBase size_t size; Value * const * elems; }; + + struct Failed : gc_cleanup + { + std::exception_ptr ex; + + Failed(std::exception_ptr ex) + : ex(std::move(ex)) + { + } + + [[noreturn]] void rethrow() const + { + try { + std::rethrow_exception(ex); + } catch (BaseError & e) { + /* Rethrow the copy of the exception - not the original one. + Stack tracing mechanisms rely on being able to modify the exceptions + they catch by reference. */ + e.throwClone(); + } catch (...) { + throw; + } + } + }; }; template @@ -450,6 +476,7 @@ struct PayloadTypeToInternalType MACRO(PrimOp *, primOp, tPrimOp) \ MACRO(ValueBase::PrimOpApplicationThunk, primOpApp, tPrimOpApp) \ MACRO(ExternalValueBase *, external, tExternal) \ + MACRO(ValueBase::Failed *, failed, tFailed) \ MACRO(NixFloat, fpoint, tFloat) #define NIX_VALUE_PAYLOAD_TYPE(T, FIELD_NAME, DISCRIMINATOR) \ @@ -754,6 +781,11 @@ protected: path.path = std::bit_cast(payload[1]); } + void getStorage(Failed *& failed) const noexcept + { + failed = std::bit_cast(payload[1]); + } + void setStorage(NixInt integer) noexcept { setSingleDWordPayload(integer.value); @@ -803,6 +835,11 @@ protected: { setUntaggablePayload(path.accessor, path.path); } + + void setStorage(Failed * failed) noexcept + { + setSingleDWordPayload(std::bit_cast(failed)); + } }; /** @@ -1054,12 +1091,12 @@ public: inline bool isThunk() const { return isa(); - }; + } inline bool isApp() const { return isa(); - }; + } inline bool isBlackhole() const; @@ -1067,17 +1104,22 @@ public: inline bool isLambda() const { return isa(); - }; + } inline bool isPrimOp() const { return isa(); - }; + } inline bool isPrimOpApp() const { return isa(); - }; + } + + inline bool isFailed() const + { + return isa(); + } /** * Returns the normal type of a Value. This only returns nThunk if @@ -1098,6 +1140,7 @@ public: t[tBool] = nBool; t[tNull] = nNull; t[tFloat] = nFloat; + t[tFailed] = nFailed; t[tExternal] = nExternal; t[tAttrs] = nAttrs; t[tPrimOp] = nFunction; @@ -1235,6 +1278,11 @@ public: setStorage(n); } + inline void mkFailed() noexcept + { + setStorage(new Value::Failed(std::current_exception())); + } + bool isList() const noexcept { return isa(); @@ -1349,6 +1397,13 @@ public: { return getStorage().accessor; } + + Failed & failed() const noexcept + { + auto p = getStorage(); + assert(p); + return *p; + } }; extern ExprBlackHole eBlackHole; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index db57556149e..0cdf0707559 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -562,6 +562,7 @@ static void prim_typeOf(EvalState & state, const PosIdx pos, Value ** args, Valu v.mkStringNoCopy("float"_sds); break; case nThunk: + case nFailed: unreachable(); } } diff --git a/src/libexpr/print-ambiguous.cc b/src/libexpr/print-ambiguous.cc index cb1a1ef2d3a..ed91cad85a4 100644 --- a/src/libexpr/print-ambiguous.cc +++ b/src/libexpr/print-ambiguous.cc @@ -73,6 +73,11 @@ void printAmbiguous(EvalState & state, Value & v, std::ostream & str, std::set"; + break; case nFunction: if (v.isLambda()) { str << ""; diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 066426ec864..f2f62a63698 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -509,6 +509,17 @@ class Printer } } + void printFailed() + { + if (options.ansiColors) + output << ANSI_MAGENTA; + // Historically, a tried and then ignored value (e.g. through tryEval) was + // reverted to the original thunk. + output << "«thunk»"; + if (options.ansiColors) + output << ANSI_NORMAL; + } + void printExternal(Value & v) { v.external()->print(output); @@ -590,6 +601,10 @@ class Printer printFunction(v); break; + case nFailed: + printFailed(); + break; + case nThunk: printThunk(v); break; diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index b2cc482c6e3..4fac29a6671 100644 --- a/src/libexpr/value-to-json.cc +++ b/src/libexpr/value-to-json.cc @@ -98,6 +98,7 @@ json printValueAsJSON( break; case nThunk: + case nFailed: case nFunction: state.error("cannot convert %1% to JSON", showType(v)).atPos(v.determinePos(pos)).debugThrow(); } diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index 2fe24857ce5..974927c7e67 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -171,7 +171,11 @@ static void printValueAsXML( break; case nThunk: + // Historically, a tried and then ignored value (e.g. through tryEval) was + // reverted to the original thunk. + case nFailed: doc.writeEmptyElement("unevaluated"); + break; } } diff --git a/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.err.exp b/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.err.exp new file mode 100644 index 00000000000..ba0da519dcb --- /dev/null +++ b/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.err.exp @@ -0,0 +1 @@ +trace: throwing diff --git a/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.exp b/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.exp new file mode 100644 index 00000000000..be54b4b4e39 --- /dev/null +++ b/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.exp @@ -0,0 +1 @@ +"done" diff --git a/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.nix b/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.nix new file mode 100644 index 00000000000..21298619907 --- /dev/null +++ b/tests/functional/lang/eval-okay-tryeval-failed-thunk-reeval.nix @@ -0,0 +1,7 @@ +# Since Nix 2.34, errors are memoized +let + # This attribute value will only be evaluated once. + foo = builtins.trace "throwing" throw "nope"; +in +# Trigger and catch the error twice. +builtins.seq (builtins.tryEval foo).success builtins.seq (builtins.tryEval foo).success "done" From 17f344cddaf1ddc8e8ae33e3c15cc814d057c4d0 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 9 Sep 2025 08:03:17 +0200 Subject: [PATCH 3/6] libexpr: Add recoverable errors This provides an explicit API for call-fail-retry-succeed evaluation flows, such as currently used in NixOps4. An alternative design would simply reset the `Value` to the original thunk instead of `tFailed` under the condition of catching a `RecoverableEvalError`. That is somewhat simpler, but I believe the presence of `tFailed` is beneficial for possible use in the repl; being able to show the error sooner, without re-evaluation. The hasPos method is required in order to avoid an include loop. --- src/libexpr-c/nix_api_value.cc | 10 +- src/libexpr-tests/nix_api_expr.cc | 102 ++++++++++++++++++++ src/libexpr/eval-error.cc | 1 + src/libexpr/eval.cc | 53 +++++++++- src/libexpr/include/nix/expr/eval-error.hh | 8 ++ src/libexpr/include/nix/expr/eval-inline.hh | 13 +-- src/libexpr/include/nix/expr/eval.hh | 20 ++++ src/libexpr/include/nix/expr/value.hh | 15 ++- src/libutil-c/nix_api_util.h | 7 ++ src/libutil/error.cc | 5 + src/libutil/include/nix/util/error.hh | 2 + 11 files changed, 223 insertions(+), 13 deletions(-) diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 06570abc338..c27b5cf1523 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -1,4 +1,5 @@ #include "nix/expr/attr-set.hh" +#include "nix/expr/eval-error.hh" #include "nix/util/configuration.hh" #include "nix/expr/eval.hh" #include "nix/store/globals.hh" @@ -107,8 +108,13 @@ static void nix_c_primop_wrapper( f(userdata, &ctx, (EvalState *) &state, external_args.data(), vTmpPtr); if (ctx.last_err_code != NIX_OK) { - /* TODO: Throw different errors depending on the error code */ - state.error("Error from custom function: %s", *ctx.last_err).atPos(pos).debugThrow(); + if (ctx.last_err_code == NIX_ERR_RECOVERABLE) { + state.error("Recoverable error from custom function: %s", *ctx.last_err) + .atPos(pos) + .debugThrow(); + } else { + state.error("Error from custom function: %s", *ctx.last_err).atPos(pos).debugThrow(); + } } if (!vTmp.isValid()) { diff --git a/src/libexpr-tests/nix_api_expr.cc b/src/libexpr-tests/nix_api_expr.cc index c7e246c727f..56363038d0c 100644 --- a/src/libexpr-tests/nix_api_expr.cc +++ b/src/libexpr-tests/nix_api_expr.cc @@ -517,4 +517,106 @@ TEST_F(nix_api_expr_test, nix_expr_attrset_update) assert_ctx_ok(); } +// The following is a test case for retryable thunks. This is a requirement +// for the current way in which NixOps4 evaluates its deployment expressions. +// An alternative strategy could be implemented, but unwinding the stack may +// be a more efficient way to deal with many suspensions/resumptions, compared +// to e.g. using a thread or coroutine stack for each suspended dependency. +// This test models the essential bits of a deployment tool that uses such +// a strategy. + +// State for the retryable primop - simulates deployment resource availability +struct DeploymentResourceState +{ + bool vm_created = false; +}; + +static void primop_load_resource_input( + void * user_data, nix_c_context * context, EvalState * state, nix_value ** args, nix_value * ret) +{ + assert(context); + assert(state); + auto * resource_state = static_cast(user_data); + + // Get the resource input name argument + std::string input_name; + if (nix_get_string(context, args[0], OBSERVE_STRING(input_name)) != NIX_OK) + return; + + // Only handle "vm_id" input - throw for anything else + if (input_name != "vm_id") { + std::string error_msg = "unknown resource input: " + input_name; + nix_set_err_msg(context, NIX_ERR_NIX_ERROR, error_msg.c_str()); + return; + } + + if (resource_state->vm_created) { + // VM has been created, return the ID + nix_init_string(context, ret, "vm-12345"); + } else { + // VM not created yet, fail with dependency error + nix_set_err_msg(context, NIX_ERR_RECOVERABLE, "VM not yet created"); + } +} + +TEST_F(nix_api_expr_test, nix_expr_thunk_re_evaluation_after_deployment) +{ + // This test demonstrates NixOps4's requirement: a thunk calling a primop should be + // re-evaluable when deployment resources become available that were not available initially. + + DeploymentResourceState resource_state; + + PrimOp * primop = nix_alloc_primop( + ctx, + primop_load_resource_input, + 1, + "loadResourceInput", + nullptr, + "load a deployment resource input", + &resource_state); + assert_ctx_ok(); + + nix_value * primopValue = nix_alloc_value(ctx, state); + assert_ctx_ok(); + nix_init_primop(ctx, primopValue, primop); + assert_ctx_ok(); + + nix_value * inputName = nix_alloc_value(ctx, state); + assert_ctx_ok(); + nix_init_string(ctx, inputName, "vm_id"); + assert_ctx_ok(); + + // Create a single thunk by using nix_init_apply instead of nix_value_call + // This creates a lazy application that can be forced multiple times + nix_value * thunk = nix_alloc_value(ctx, state); + assert_ctx_ok(); + nix_init_apply(ctx, thunk, primopValue, inputName); + assert_ctx_ok(); + + // First force: VM not created yet, should fail + nix_value_force(ctx, state, thunk); + ASSERT_EQ(NIX_ERR_NIX_ERROR, nix_err_code(ctx)); + ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("VM not yet created")); + + // Clear the error context for the next attempt + nix_c_context_free(ctx); + ctx = nix_c_context_create(); + + // Simulate deployment process: VM gets created + resource_state.vm_created = true; + + // Second force of the SAME thunk: this is where the "failed" value issue appears + // With failed value caching, this should fail because the thunk is marked as permanently failed + // Without failed value caching (or with retryable failures), this should succeed + nix_value_force(ctx, state, thunk); + + // If we get here without error, the thunk was successfully re-evaluated + assert_ctx_ok(); + + std::string result; + nix_get_string(ctx, thunk, OBSERVE_STRING(result)); + assert_ctx_ok(); + ASSERT_STREQ("vm-12345", result.c_str()); +} + } // namespace nixC diff --git a/src/libexpr/eval-error.cc b/src/libexpr/eval-error.cc index 5da804e7ac4..a4a9b65318d 100644 --- a/src/libexpr/eval-error.cc +++ b/src/libexpr/eval-error.cc @@ -114,5 +114,6 @@ template class EvalErrorBuilder; template class EvalErrorBuilder; template class EvalErrorBuilder; template class EvalErrorBuilder; +template class EvalErrorBuilder; } // namespace nix diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index defb4d02dbd..ff913bfa51d 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1,4 +1,5 @@ #include "nix/expr/eval.hh" +#include "nix/expr/eval-error.hh" #include "nix/expr/eval-settings.hh" #include "nix/expr/primops.hh" #include "nix/expr/print-options.hh" @@ -31,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -2180,6 +2182,54 @@ void ExprBlackHole::eval(EvalState & state, [[maybe_unused]] Env & env, Value & // always force this to be separate, otherwise forceValue may inline it and take // a massive perf hit [[gnu::noinline]] +void EvalState::handleEvalExceptionForThunk(Env * env, Expr * expr, Value & v, const PosIdx pos) +{ + if (!env) + tryFixupBlackHolePos(v, pos); + + auto e = std::current_exception(); + Value * recovery = nullptr; + try { + std::rethrow_exception(e); + } catch (const RecoverableEvalError & e) { + recovery = allocValue(); + } catch (...) { + } + if (recovery) { + recovery->mkThunk(env, expr); + } + v.mkFailed(e, recovery); +} + +[[gnu::noinline]] +void EvalState::handleEvalExceptionForApp(Value & v, const Value & savedApp) +{ + auto e = std::current_exception(); + Value * recovery = nullptr; + try { + std::rethrow_exception(e); + } catch (const RecoverableEvalError & e) { + recovery = allocValue(); + } catch (...) { + } + if (recovery) { + *recovery = savedApp; + } + v.mkFailed(e, recovery); +} + +[[gnu::noinline]] +void EvalState::handleEvalFailed(Value & v, const PosIdx pos) +{ + assert(v.isFailed()); + if (auto recoveryValue = v.failed().recoveryValue) { + v = *recoveryValue; + forceValue(v, pos); + } else { + v.failed().rethrow(); + } +} + void EvalState::tryFixupBlackHolePos(Value & v, PosIdx pos) { if (!v.isBlackhole()) @@ -2188,7 +2238,8 @@ void EvalState::tryFixupBlackHolePos(Value & v, PosIdx pos) try { std::rethrow_exception(e); } catch (InfiniteRecursionError & e) { - e.atPos(positions[pos]); + if (!e.hasPos()) + e.atPos(positions[pos]); } catch (...) { } } diff --git a/src/libexpr/include/nix/expr/eval-error.hh b/src/libexpr/include/nix/expr/eval-error.hh index 24717d40bb1..dd679810e8a 100644 --- a/src/libexpr/include/nix/expr/eval-error.hh +++ b/src/libexpr/include/nix/expr/eval-error.hh @@ -70,6 +70,14 @@ struct StackOverflowError : public CloneableError { public: diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index 33aa6b9b47f..451a5e09e20 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/src/libexpr/include/nix/expr/eval-inline.hh @@ -5,6 +5,7 @@ #include "nix/expr/eval.hh" #include "nix/expr/eval-error.hh" #include "nix/expr/eval-settings.hh" +#include namespace nix { @@ -96,20 +97,20 @@ void EvalState::forceValue(Value & v, const PosIdx pos) else ExprBlackHole::throwInfiniteRecursionError(*this, v); } catch (...) { - if (!env) - tryFixupBlackHolePos(v, pos); - v.mkFailed(); + handleEvalExceptionForThunk(env, expr, v, pos); throw; } } else if (v.isApp()) { + Value savedApp = v; try { callFunction(*v.app().left, *v.app().right, v, pos); } catch (...) { - v.mkFailed(); + handleEvalExceptionForApp(v, savedApp); throw; } - } else if (v.isFailed()) - v.failed().rethrow(); + } else if (v.isFailed()) { + handleEvalFailed(v, pos); + } } [[gnu::always_inline]] diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 29acb4547aa..3e3d76e2c7f 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -652,8 +652,28 @@ public: */ inline void forceValue(Value & v, const PosIdx pos); +private: + + /** + * Internal support function for forceValue + * + * This code is factored out so that it's not in the heavily inlined hot path. + */ + void handleEvalExceptionForThunk(Env * env, Expr * expr, Value & v, const PosIdx pos); + + /** + * Internal support function for forceValue + * + * This code is factored out so that it's not in the heavily inlined hot path. + */ + void handleEvalExceptionForApp(Value & v, const Value & savedApp); + + void handleEvalFailed(Value & v, PosIdx pos); + void tryFixupBlackHolePos(Value & v, PosIdx pos); +public: + /** * Force a value, then recursively force list elements and * attributes. diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 59969079c5c..3a478c4bfd2 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -430,9 +431,15 @@ struct ValueBase struct Failed : gc_cleanup { std::exception_ptr ex; + /** + * Optional value for recovering `RecoverableEvalError` + * Must be set iff `ex` is an instance of `RecoverableEvalError`. + */ + Value * recoveryValue; - Failed(std::exception_ptr ex) - : ex(std::move(ex)) + Failed(std::exception_ptr ex, Value * recoveryValue) + : ex(ex) + , recoveryValue(recoveryValue) { } @@ -1278,9 +1285,9 @@ public: setStorage(n); } - inline void mkFailed() noexcept + inline void mkFailed(std::exception_ptr e, Value * recovery) noexcept { - setStorage(new Value::Failed(std::current_exception())); + setStorage(new Value::Failed(e, recovery)); } bool isList() const noexcept diff --git a/src/libutil-c/nix_api_util.h b/src/libutil-c/nix_api_util.h index d301e5743cf..66ea1752221 100644 --- a/src/libutil-c/nix_api_util.h +++ b/src/libutil-c/nix_api_util.h @@ -109,6 +109,13 @@ enum nix_err { */ NIX_ERR_NIX_ERROR = -4, + /** + * @brief A recoverable error occurred. + * + * This is used primarily by C API *consumers* to communicate that a failed + * primop call should be retried on the next evaluation attempt. + */ + NIX_ERR_RECOVERABLE = -5, }; typedef enum nix_err nix_err; diff --git a/src/libutil/error.cc b/src/libutil/error.cc index c42cbd5a13e..75344dab30f 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -41,6 +41,11 @@ const std::string & BaseError::calcWhat() const } } +bool BaseError::hasPos() const +{ + return err.pos.get() && *err.pos.get(); +} + std::optional ErrorInfo::programName = std::nullopt; std::ostream & operator<<(std::ostream & os, const HintFmt & hf) diff --git a/src/libutil/include/nix/util/error.hh b/src/libutil/include/nix/util/error.hh index ea18b0a30fb..ca54523b666 100644 --- a/src/libutil/include/nix/util/error.hh +++ b/src/libutil/include/nix/util/error.hh @@ -191,6 +191,8 @@ public: err.pos = pos; } + bool hasPos() const; + void pushTrace(Trace trace) { err.traces.push_front(trace); From 100e9a44361d148f2e3628c8884626b9c031f568 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 18 Feb 2026 22:26:53 +0300 Subject: [PATCH 4/6] Add tests/f/lang/eval-fail-memoised-error-trace-not-mutated.nix --- ...l-memoised-error-trace-not-mutated.err.exp | 27 +++++++++++++++++++ ...-fail-memoised-error-trace-not-mutated.nix | 10 +++++++ 2 files changed, 37 insertions(+) create mode 100644 tests/functional/lang/eval-fail-memoised-error-trace-not-mutated.err.exp create mode 100644 tests/functional/lang/eval-fail-memoised-error-trace-not-mutated.nix diff --git a/tests/functional/lang/eval-fail-memoised-error-trace-not-mutated.err.exp b/tests/functional/lang/eval-fail-memoised-error-trace-not-mutated.err.exp new file mode 100644 index 00000000000..f8807b6bd9a --- /dev/null +++ b/tests/functional/lang/eval-fail-memoised-error-trace-not-mutated.err.exp @@ -0,0 +1,27 @@ +error: + … while calling the 'seq' builtin + at /pwd/lang/eval-fail-memoised-error-trace-not-mutated.nix:10:1: + 9| # a fresh instance of the exceptions to avoid trace mutation. + 10| builtins.seq (builtins.tryEval b) (builtins.seq (builtins.tryEval c) d) + | ^ + 11| + + … while calling the 'seq' builtin + at /pwd/lang/eval-fail-memoised-error-trace-not-mutated.nix:10:36: + 9| # a fresh instance of the exceptions to avoid trace mutation. + 10| builtins.seq (builtins.tryEval b) (builtins.seq (builtins.tryEval c) d) + | ^ + 11| + + … forcing d + + … forcing b + + … while calling the 'throw' builtin + at /pwd/lang/eval-fail-memoised-error-trace-not-mutated.nix:2:7: + 1| let + 2| a = throw "nope"; + | ^ + 3| b = builtins.addErrorContext "forcing b" a; + + error: nope diff --git a/tests/functional/lang/eval-fail-memoised-error-trace-not-mutated.nix b/tests/functional/lang/eval-fail-memoised-error-trace-not-mutated.nix new file mode 100644 index 00000000000..03db62843a3 --- /dev/null +++ b/tests/functional/lang/eval-fail-memoised-error-trace-not-mutated.nix @@ -0,0 +1,10 @@ +let + a = throw "nope"; + b = builtins.addErrorContext "forcing b" a; + c = builtins.addErrorContext "forcing c" a; + d = builtins.addErrorContext "forcing d" a; +in +# Since nix 2.34 errors are memoised. Trying to eval a failed thunk includes +# the trace from when it was first forced. When forcing a failed value it gets +# a fresh instance of the exceptions to avoid trace mutation. +builtins.seq (builtins.tryEval b) (builtins.seq (builtins.tryEval c) d) From fd4eee9d62459fa9ecf0e92ecd64cba306cbe43c Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 18 Feb 2026 22:53:58 +0300 Subject: [PATCH 5/6] libexpr-tests: Add ValuePrintingTests.vFailed --- src/libexpr-tests/value/print.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/libexpr-tests/value/print.cc b/src/libexpr-tests/value/print.cc index d226062197d..50e78363e3f 100644 --- a/src/libexpr-tests/value/print.cc +++ b/src/libexpr-tests/value/print.cc @@ -188,6 +188,22 @@ TEST_F(ValuePrintingTests, vBlackhole) test(vBlackhole, "«potential infinite recursion»"); } +TEST_F(ValuePrintingTests, vFailed) +{ + Value v; + try { + throw Error("nope"); + } catch (...) { + v.mkFailed(std::current_exception(), nullptr); + } + + // Historically, a tried and then ignored value (e.g. through tryEval) was + // reverted to the original thunk. + + test(v, "«thunk»"); + test(v, ANSI_MAGENTA "«thunk»" ANSI_NORMAL, PrintOptions{.ansiColors = true}); +} + TEST_F(ValuePrintingTests, depthAttrs) { Value vOne; From dd4b73a44dec4945c63a9040540bc62ed3bfc9cc Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Tue, 9 Sep 2025 12:11:50 +0200 Subject: [PATCH 6/6] Add rl-next/c-api-recoverable-errors --- .../rl-next/c-api-recoverable-errors.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 doc/manual/rl-next/c-api-recoverable-errors.md diff --git a/doc/manual/rl-next/c-api-recoverable-errors.md b/doc/manual/rl-next/c-api-recoverable-errors.md new file mode 100644 index 00000000000..462984f529b --- /dev/null +++ b/doc/manual/rl-next/c-api-recoverable-errors.md @@ -0,0 +1,23 @@ +--- +synopsis: "C API: Errors returned from your primops are not treated as recoverable by default" +prs: [15286, 13930] +--- + +Nix 2.34 by default remembers the error in the thunk that triggered it. + +Previously the following sequence of events worked: + +1. Have a thunk that invokes a primop that's defined through the C API +2. The primop returns an error +3. Force the thunk again +4. The primop returns a value +5. The thunk evaluated successfully + +**Resolution** + +C API consumers that rely on this must change their recoverable error calls: + +```diff +-nix_set_err_msg(context, NIX_ERR_*, msg); ++nix_set_err_msg(context, NIX_ERR_RECOVERABLE, msg); +```