fix(tvix/eval): fix b/281 by adding Value::Catchable r/6650

This commit makes catchable errors a variant of Value.

The main downside of this approach is that we lose the ability to
use Rust's `?` syntax for propagating catchable errors.

Change-Id: Ibe89438d8a70dcec29e016df692b5bf88a5cad13
Reviewed-on: https://cl.tvl.fyi/c/depot/+/9289
Reviewed-by: tazjin <tazjin@tvl.su>
Autosubmit: Adam Joseph <adam@westernsemico.com>
Tested-by: BuildkiteCI

3 files changed, 65 insertions, 14 deletions

diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs
index e4840cc88c..15a709730c 100644
--- a/tvix/eval/src/value/attrs.rs
+++ b/tvix/eval/src/value/attrs.rs
@@ -393,7 +393,7 @@ impl NixAttrs {
             // /another/ set with a __toString attr.
             let s = generators::request_string_coerce(co, result, kind).await;
 
-            return Some(s);
+            return Some(s.ok()?);
         }
 
         None
diff --git a/tvix/eval/src/value/json.rs b/tvix/eval/src/value/json.rs
index 38496f8f35..1290cce14e 100644
--- a/tvix/eval/src/value/json.rs
+++ b/tvix/eval/src/value/json.rs
@@ -4,15 +4,18 @@
 /// as there is internal Nix logic that must happen within the
 /// serialisation methods.
 use super::{CoercionKind, Value};
+use crate::errors::{CatchableErrorKind, ErrorKind};
 use crate::generators::{self, GenCo};
-use crate::ErrorKind;
 
 use serde_json::value::to_value;
 use serde_json::Value as Json; // name clash with *our* `Value`
 use serde_json::{Map, Number};
 
 impl Value {
-    pub(crate) async fn to_json(self, co: &GenCo) -> Result<Json, ErrorKind> {
+    pub(crate) async fn to_json(
+        self,
+        co: &GenCo,
+    ) -> Result<Result<Json, CatchableErrorKind>, ErrorKind> {
         let self_forced = generators::request_force(co, self).await;
 
         let value = match self_forced {
@@ -42,14 +45,14 @@ impl Value {
                 // serialise to the string-coerced version of the result of
                 // calling that.
                 if let Some(s) = attrs.try_to_string(co, CoercionKind::Weak).await {
-                    return Ok(Json::String(s.as_str().to_string()));
+                    return Ok(Ok(Json::String(s.as_str().to_string())));
                 }
 
                 // Attribute sets with an `outPath` attribute
                 // serialise to a JSON serialisation of that inner
                 // value (regardless of what it is!).
                 if let Some(out_path) = attrs.select("outPath") {
-                    return Ok(generators::request_to_json(co, out_path.clone()).await);
+                    return Ok(Ok(generators::request_to_json(co, out_path.clone()).await));
                 }
 
                 let mut out = Map::with_capacity(attrs.len());
@@ -63,6 +66,8 @@ impl Value {
                 Json::Object(out)
             }
 
+            Value::Catchable(c) => return Ok(Err(c)),
+
             val @ Value::Closure(_)
             | val @ Value::Thunk(_)
             | val @ Value::Builtin(_)
@@ -76,12 +81,15 @@ impl Value {
             }
         };
 
-        Ok(value)
+        Ok(Ok(value))
     }
 
     /// Generator version of the above, which wraps responses in
     /// Value::Json.
     pub(crate) async fn to_json_generator(self, co: GenCo) -> Result<Value, ErrorKind> {
-        Ok(Value::Json(self.to_json(&co).await?))
+        match self.to_json(&co).await? {
+            Err(cek) => Ok(Value::Catchable(cek)),
+            Ok(json) => Ok(Value::Json(json)),
+        }
     }
 }
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 254bbbc09a..f5b373e3c4 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -20,7 +20,7 @@ mod path;
 mod string;
 mod thunk;
 
-use crate::errors::ErrorKind;
+use crate::errors::{CatchableErrorKind, ErrorKind};
 use crate::opcode::StackIdx;
 use crate::spans::LightSpan;
 use crate::vm::generators::{self, GenCo};
@@ -81,6 +81,24 @@ pub enum Value {
 
     #[serde(skip)]
     FinaliseRequest(bool),
+
+    #[serde(skip)]
+    Catchable(CatchableErrorKind),
+}
+
+impl From<CatchableErrorKind> for Value {
+    fn from(c: CatchableErrorKind) -> Value {
+        Value::Catchable(c)
+    }
+}
+
+impl<V> From<Result<V, CatchableErrorKind>> for Value
+where
+    Value: From<V>,
+{
+    fn from(v: Result<V, CatchableErrorKind>) -> Value {
+        v.map_or_else(|cek| Value::Catchable(cek), |v| v.into())
+    }
 }
 
 lazy_static! {
@@ -222,18 +240,28 @@ impl Value {
 
             Value::List(list) => {
                 for val in list {
-                    generators::request_deep_force(&co, val.clone(), thunk_set.clone()).await;
+                    if let c @ Value::Catchable(_) =
+                        generators::request_deep_force(&co, val.clone(), thunk_set.clone()).await
+                    {
+                        return Ok(c);
+                    }
                 }
             }
 
             Value::Attrs(attrs) => {
                 for (_, val) in attrs.iter() {
-                    generators::request_deep_force(&co, val.clone(), thunk_set.clone()).await;
+                    if let c @ Value::Catchable(_) =
+                        generators::request_deep_force(&co, val.clone(), thunk_set.clone()).await
+                    {
+                        return Ok(c);
+                    }
                 }
             }
 
             Value::Thunk(_) => panic!("Tvix bug: force_value() returned a thunk"),
 
+            Value::Catchable(_) => return Ok(value),
+
             Value::AttrNotFound
             | Value::Blueprint(_)
             | Value::DeferredUpvalue(_)
@@ -279,8 +307,12 @@ impl Value {
                 }
 
                 if let Some(out_path) = attrs.select("outPath") {
-                    let s = generators::request_string_coerce(&co, out_path.clone(), kind).await;
-                    return Ok(Value::String(s));
+                    return match generators::request_string_coerce(&co, out_path.clone(), kind)
+                        .await
+                    {
+                        Ok(s) => Ok(Value::String(s)),
+                        Err(c) => Ok(Value::Catchable(c)),
+                    };
                 }
 
                 Err(ErrorKind::NotCoercibleToString { from: "set", kind })
@@ -308,8 +340,10 @@ impl Value {
                         out.push(' ');
                     }
 
-                    let s = generators::request_string_coerce(&co, elem, kind).await;
-                    out.push_str(s.as_str());
+                    match generators::request_string_coerce(&co, elem, kind).await {
+                        Ok(s) => out.push_str(s.as_str()),
+                        Err(c) => return Ok(Value::Catchable(c)),
+                    }
                 }
 
                 Ok(Value::String(out.into()))
@@ -328,6 +362,8 @@ impl Value {
                 kind,
             }),
 
+            (c @ Value::Catchable(_), _) => return Ok(c),
+
             (Value::AttrNotFound, _)
             | (Value::Blueprint(_), _)
             | (Value::DeferredUpvalue(_), _)
@@ -384,6 +420,8 @@ impl Value {
 
         let result = match (a, b) {
             // Trivial comparisons
+            (c @ Value::Catchable(_), _) => return Ok(c),
+            (_, c @ Value::Catchable(_)) => return Ok(c),
             (Value::Null, Value::Null) => true,
             (Value::Bool(b1), Value::Bool(b2)) => b1 == b2,
             (Value::String(s1), Value::String(s2)) => s1 == s2,
@@ -526,6 +564,7 @@ impl Value {
             Value::UnresolvedPath(_) => "internal[unresolved_path]",
             Value::Json(_) => "internal[json]",
             Value::FinaliseRequest(_) => "internal[finaliser_sentinel]",
+            Value::Catchable(_) => "internal[catchable]",
         }
     }
 
@@ -533,6 +572,7 @@ impl Value {
     gen_cast!(as_int, i64, "int", Value::Integer(x), *x);
     gen_cast!(as_float, f64, "float", Value::Float(x), *x);
     gen_cast!(to_str, NixString, "string", Value::String(s), s.clone());
+    gen_cast!(to_path, Box<PathBuf>, "path", Value::Path(p), p.clone());
     gen_cast!(to_attrs, Box<NixAttrs>, "set", Value::Attrs(a), a.clone());
     gen_cast!(to_list, NixList, "list", Value::List(l), l.clone());
     gen_cast!(
@@ -660,6 +700,8 @@ impl Value {
             // TODO: handle suspended thunks with a different explanation instead of panicking
             Value::Thunk(t) => t.value().explain(),
 
+            Value::Catchable(_) => "a catchable failure".into(),
+
             Value::AttrNotFound
             | Value::Blueprint(_)
             | Value::DeferredUpvalue(_)
@@ -785,6 +827,7 @@ impl TotalDisplay for Value {
             // Delegate thunk display to the type, as it must handle
             // the case of already evaluated or cyclic thunks.
             Value::Thunk(t) => t.total_fmt(f, set),
+            Value::Catchable(_) => panic!("total_fmt() called on a CatchableErrorKind"),
         }
     }
 }