Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- `primop::RecoverableError` for primop errors that should not be memoized in the thunk, allowing retry on next force. Required by Nix >= 2.34 ([release note](https://nix.dev/manual/nix/2.34/release-notes/rl-2.34.html#c-api-changes)) for recoverable errors to remain recoverable, as Nix 2.34 memoizes errors by default.

## [0.2.0] - 2026-01-13

### Added
Expand Down
30 changes: 15 additions & 15 deletions dev/flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 18 additions & 18 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix-bindings-expr/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ use nix_bindings_util::nix_version::emit_version_cfg;

fn main() {
let nix_version = pkg_config::probe_library("nix-expr-c").unwrap().version;
emit_version_cfg(&nix_version, &["2.26"]);
emit_version_cfg(&nix_version, &["2.26", "2.34.0pre"]);
}
75 changes: 75 additions & 0 deletions nix-bindings-expr/src/eval_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2902,4 +2902,79 @@ mod tests {
})
.unwrap();
}

#[test]
#[cfg(nix_at_least = "2.34.0pre")]
fn eval_state_primop_recoverable_error() {
gc_registering_current_thread(|| {
let store = Store::open(None, []).unwrap();
let mut es = EvalState::new(store, []).unwrap();

let call_count = std::cell::Cell::new(0u32);
let v = es
.new_value_thunk(
"recoverable_test",
Box::new(move |es: &mut EvalState| {
let count = call_count.get();
call_count.set(count + 1);
if count == 0 {
Err(primop::RecoverableError::new("transient failure").into())
} else {
es.new_value_int(42)
}
}),
)
.unwrap();

// First force should fail with the recoverable error
let r = es.force(&v);
assert!(r.is_err());
assert!(
r.unwrap_err().to_string().contains("transient failure"),
"Error message should contain 'transient failure'"
);

// Second force should succeed because the error was recoverable
es.force(&v).unwrap();
let i = es.require_int(&v).unwrap();
assert_eq!(i, 42);
})
.unwrap();
}

#[test]
#[cfg(nix_at_least = "2.34.0pre")]
fn eval_state_primop_recoverable_error_in_chain() {
gc_registering_current_thread(|| {
let store = Store::open(None, []).unwrap();
let mut es = EvalState::new(store, []).unwrap();

let call_count = std::cell::Cell::new(0u32);
let v = es
.new_value_thunk(
"recoverable_chain_test",
Box::new(move |es: &mut EvalState| {
let count = call_count.get();
call_count.set(count + 1);
if count == 0 {
// Wrap RecoverableError in .context(), pushing it down the chain
Err(primop::RecoverableError::new("transient failure").into())
} else {
es.new_value_int(42)
}
}),
)
.unwrap();

// First force should fail
let r = es.force(&v);
assert!(r.is_err());

// Second force should succeed if RecoverableError is found in the chain
es.force(&v).unwrap();
let i = es.require_int(&v).unwrap();
assert_eq!(i, 42);
})
Comment on lines +2947 to +2977
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

git ls-files | grep -E "eval_state\.rs$"

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 115


🏁 Script executed:

wc -l nix-bindings-expr/src/eval_state.rs

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 120


🏁 Script executed:

sed -n '2940,2980p' nix-bindings-expr/src/eval_state.rs

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 1485


🏁 Script executed:

# Let's also search for the RecoverableError definition to understand its structure
rg "struct RecoverableError|impl.*RecoverableError" --type rust -A 10

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 1659


🏁 Script executed:

# Let's look for the test function names to ensure we're looking at the right code
rg "fn eval_state_primop_recoverable" nix-bindings-expr/src/eval_state.rs -A 2

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 383


Test doesn't actually verify error chain behavior.

The closure returns Err(primop::RecoverableError::new(...).into()), which is the same direct conversion as the preceding test. Since RecoverableError implements Error with no source() method, this produces a top-level error with no chain—not the error-chain scenario the test name suggests.

To match the test's intent, introduce a wrapper error that chains RecoverableError via source():

Suggested test adjustment
 #[test]
 #[cfg(nix_at_least = "2.34.0pre")]
 fn eval_state_primop_recoverable_error_in_chain() {
     gc_registering_current_thread(|| {
+        #[derive(Debug)]
+        struct WrappedRecoverable(primop::RecoverableError);
+        impl std::fmt::Display for WrappedRecoverable {
+            fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+                write!(f, "wrapped recoverable")
+            }
+        }
+        impl std::error::Error for WrappedRecoverable {
+            fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+                Some(&self.0)
+            }
+        }
+
         let store = Store::open(None, []).unwrap();
         let mut es = EvalState::new(store, []).unwrap();
@@
                         call_count.set(count + 1);
                         if count == 0 {
-                            // Wrap RecoverableError in .context(), pushing it down the chain
-                            Err(primop::RecoverableError::new("transient failure").into())
+                            Err(Box::new(WrappedRecoverable(
+                                primop::RecoverableError::new("transient failure"),
+                            )))
                         } else {
                             es.new_value_int(42)
                         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@nix-bindings-expr/src/eval_state.rs` around lines 2947 - 2977, The test
eval_state_primop_recoverable_error_in_chain currently returns
Err(primop::RecoverableError::new(...).into()) which produces a top-level error
without a source chain; update the closure passed to new_value_thunk to return a
wrapper error type that implements std::error::Error with source() returning the
primop::RecoverableError (or otherwise use anyhow::Error::context to attach the
RecoverableError as a source), then return that wrapper converted into the
expected Error (i.e. wrapper.into()) so the code path that searches the error
chain will find RecoverableError; locate the closure in new_value_thunk inside
eval_state_primop_recoverable_error_in_chain and replace the direct conversion
with the chained-wrapper conversion while keeping the second branch returning
es.new_value_int(42).

.unwrap();
}
}
50 changes: 48 additions & 2 deletions nix-bindings-expr/src/primop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,35 @@ use std::error::Error;
use std::ffi::{c_int, c_void, CStr, CString};
use std::ptr::{null, null_mut};

/// A primop error that is not memoized in the thunk that triggered it,
/// allowing the thunk to be forced again.
///
/// Since [Nix 2.34](https://nix.dev/manual/nix/2.34/release-notes/rl-2.34.html#c-api-changes),
/// primop errors are memoized by default: once a thunk fails, forcing it
/// again returns the same error. Use `RecoverableError` for errors that
/// are transient, so the caller can retry.
///
/// On Nix < 2.34, all errors are already recoverable, so this type has
/// no additional effect.
///
/// Available since nix-bindings-expr 0.2.1.
#[derive(Debug)]
pub struct RecoverableError(String);

impl RecoverableError {
pub fn new(msg: impl Into<String>) -> Self {
RecoverableError(msg.into())
}
}

impl std::fmt::Display for RecoverableError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.fmt(f)
}
}

impl std::error::Error for RecoverableError {}

/// Metadata for a primop, used with `PrimOp::new`.
pub struct PrimOpMeta<'a, const N: usize> {
/// Name of the primop. Note that primops do not have to be registered as
Expand Down Expand Up @@ -35,7 +64,13 @@ impl Drop for PrimOp<'_> {
}
}
}

impl<'a> PrimOp<'a> {
/// Create a new primop with the given metadata and implementation.
///
/// When `f` returns an `Err`, the error is propagated to the Nix evaluator.
/// To return a [recoverable error](RecoverableError), include it in the
/// error chain (e.g. `Err(RecoverableError::new("...").into())`).
pub fn new<const N: usize>(
eval_state: &'a mut EvalState,
meta: PrimOpMeta<N>,
Expand Down Expand Up @@ -134,13 +169,24 @@ unsafe extern "C" fn function_adapter(
raw::copy_value(context_out, ret, v.raw_ptr());
},
Err(e) => unsafe {
let cstr = CString::new(e.to_string()).unwrap_or_else(|_e| {
let err_str = e.to_string();
let err_code = error_code(e);
let cstr = CString::new(err_str).unwrap_or_else(|_e| {
CString::new("<rust nix-expr application error message contained null byte>")
.unwrap()
});
raw_util::set_err_msg(context_out, raw_util::err_NIX_ERR_UNKNOWN, cstr.as_ptr());
raw_util::set_err_msg(context_out, err_code, cstr.as_ptr());
},
Comment on lines +172 to 179
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify current implementation only checks top-level error type and does not walk source() chain.
rg -n -C3 'fn error_code|downcast_ref::<RecoverableError>|source\(' nix-bindings-expr/src/primop.rs

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 415


🏁 Script executed:

#!/bin/bash
# 1. Search for RecoverableError definition and usage
rg -n 'RecoverableError' nix-bindings-expr/src/ -B2 -A5

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 5570


🏁 Script executed:

#!/bin/bash
# 2. Check how errors are created/wrapped in function_adapter
rg -n 'function_adapter|Box::new.*Error|anyhow::anyhow' nix-bindings-expr/src/primop.rs -B3 -A3

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 426


🏁 Script executed:

#!/bin/bash
# 3. Verify Error::source() method and error chain patterns
rg -n 'Error|source|chain' nix-bindings-expr/src/primop.rs | head -30

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 1008


🏁 Script executed:

#!/bin/bash
# 4. Check the full error_code function and surrounding context
sed -n '170,195p' nix-bindings-expr/src/primop.rs

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 866


🏁 Script executed:

#!/bin/bash
# Check if Error::source() is used anywhere in the codebase
rg -n 'source\(\)' nix-bindings-expr/src/

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 62


🏁 Script executed:

#!/bin/bash
# Look at the test more carefully - specifically the wrapped error case
sed -n '2950,2980p' nix-bindings-expr/src/eval_state.rs

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 1211


🏁 Script executed:

#!/bin/bash
# Check what the .context() method does and what error type chain it creates
rg -n 'use.*anyhow|use.*error|Context' nix-bindings-expr/src/primop.rs | head -20

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 344


🏁 Script executed:

#!/bin/bash
# Check if anyhow is being used and how errors are wrapped
rg -n 'anyhow|chain|context|source' nix-bindings-expr/src/ -i | head -30

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 2952


🏁 Script executed:

#!/bin/bash
# Check the imports and dependencies
cat nix-bindings-expr/Cargo.toml | grep -A 20 '\[dependencies\]'

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 823


🏁 Script executed:

#!/bin/bash
# Look at actual primop implementations to see error patterns
rg -n 'Err\(' nix-bindings-expr/src/ -B1 -A1 | head -50

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 3248


Recoverable-error detection currently misses wrapped errors.

error_code() only checks the top-level error (Line 186), so recoverable errors wrapped in another error type won't be detected and will be reported as err_NIX_ERR_UNKNOWN. This breaks the documented "error chain" behavior and the test expectations for retries on Nix ≥ 2.34.

The test at eval_state.rs:2973 explicitly expects this behavior: "Second force should succeed if RecoverableError is found in the chain," but the current implementation cannot detect wrapped errors.

💡 Proposed fix
-            let err_str = e.to_string();
-            let err_code = error_code(e);
+            let err_code = error_code(e.as_ref());
+            let err_str = e.to_string();
             let cstr = CString::new(err_str).unwrap_or_else(|_e| {
                 CString::new("<rust nix-expr application error message contained null byte>")
                     .unwrap()
             });
             raw_util::set_err_msg(context_out, err_code, cstr.as_ptr());
@@
 #[cfg_attr(not(nix_at_least = "2.34.0pre"), allow(unused))]
-fn error_code(e: Box<dyn Error>) -> raw_util::err {
+fn error_code(e: &(dyn Error + 'static)) -> raw_util::err {
     #[cfg(nix_at_least = "2.34.0pre")]
-    if e.downcast_ref::<RecoverableError>().is_some() {
-        return raw_util::err_NIX_ERR_RECOVERABLE;
+    {
+        if e.downcast_ref::<RecoverableError>().is_some() {
+            return raw_util::err_NIX_ERR_RECOVERABLE;
+        }
+        let mut source = e.source();
+        while let Some(err) = source {
+            if err.downcast_ref::<RecoverableError>().is_some() {
+                return raw_util::err_NIX_ERR_RECOVERABLE;
+            }
+            source = err.source();
+        }
     }
     raw_util::err_NIX_ERR_UNKNOWN
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@nix-bindings-expr/src/primop.rs` around lines 172 - 179, The current error
handling only calls error_code(e) on the top-level error, missing wrapped
RecoverableError instances; update the logic in the catch arm that builds
err_str/err_code (the closure using err_str, err_code, CString::new, and
raw_util::set_err_msg) to walk the error chain (using e.source() /
std::error::Error::chain or similar) and call error_code on each cause until a
non-unknown code (or a RecoverableError) is found, then use that code for
raw_util::set_err_msg; alternatively refactor/extend error_code to inspect the
full error chain and return the first matching code.

}
}

#[cfg_attr(not(nix_at_least = "2.34.0pre"), allow(unused))]
fn error_code(e: Box<dyn Error>) -> raw_util::err {
#[cfg(nix_at_least = "2.34.0pre")]
if e.downcast_ref::<RecoverableError>().is_some() {
return raw_util::err_NIX_ERR_RECOVERABLE;
}
raw_util::err_NIX_ERR_UNKNOWN
}

static FUNCTION_ADAPTER: raw::PrimOpFun = Some(function_adapter);
3 changes: 2 additions & 1 deletion nix-bindings-store/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,8 @@ mod tests {
};

assert!(
err.contains("required system or feature not available"),
err.contains("required system or feature not available")
|| err.contains("platform mismatch"),
"Error should mention system not available, got: {}",
err
);
Expand Down
Loading