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, 75 insertions, 43 deletions

diff --git a/tvix/eval/src/builtins/impure.rs b/tvix/eval/src/builtins/impure.rs
index 26d1ba4945..5324fd5653 100644
--- a/tvix/eval/src/builtins/impure.rs
+++ b/tvix/eval/src/builtins/impure.rs
@@ -27,40 +27,47 @@ mod impure_builtins {
 
     #[builtin("pathExists")]
     async fn builtin_path_exists(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
-        let path = coerce_value_to_path(&co, path).await?;
-        Ok(generators::request_path_exists(&co, path).await)
+        match coerce_value_to_path(&co, path).await? {
+            Err(cek) => Ok(Value::Catchable(cek)),
+            Ok(path) => Ok(generators::request_path_exists(&co, path).await),
+        }
     }
 
     #[builtin("readDir")]
     async fn builtin_read_dir(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
-        let path = coerce_value_to_path(&co, path).await?;
+        match coerce_value_to_path(&co, path).await? {
+            Err(cek) => Ok(Value::Catchable(cek)),
+            Ok(path) => {
+                let dir = generators::request_read_dir(&co, path).await;
+                let res = dir.into_iter().map(|(name, ftype)| {
+                    (
+                        // TODO: propagate Vec<u8> or bytes::Bytes into NixString.
+                        NixString::from(
+                            String::from_utf8(name.to_vec()).expect("parsing file name as string"),
+                        ),
+                        Value::String(
+                            match ftype {
+                                FileType::Directory => "directory",
+                                FileType::Regular => "regular",
+                                FileType::Symlink => "symlink",
+                                FileType::Unknown => "unknown",
+                            }
+                            .into(),
+                        ),
+                    )
+                });
 
-        let dir = generators::request_read_dir(&co, path).await;
-        let res = dir.into_iter().map(|(name, ftype)| {
-            (
-                // TODO: propagate Vec<u8> or bytes::Bytes into NixString.
-                NixString::from(
-                    String::from_utf8(name.to_vec()).expect("parsing file name as string"),
-                ),
-                Value::String(
-                    match ftype {
-                        FileType::Directory => "directory",
-                        FileType::Regular => "regular",
-                        FileType::Symlink => "symlink",
-                        FileType::Unknown => "unknown",
-                    }
-                    .into(),
-                ),
-            )
-        });
-
-        Ok(Value::attrs(NixAttrs::from_iter(res)))
+                Ok(Value::attrs(NixAttrs::from_iter(res)))
+            }
+        }
     }
 
     #[builtin("readFile")]
     async fn builtin_read_file(co: GenCo, path: Value) -> Result<Value, ErrorKind> {
-        let path = coerce_value_to_path(&co, path).await?;
-        Ok(generators::request_read_to_string(&co, path).await)
+        match coerce_value_to_path(&co, path).await? {
+            Err(cek) => Ok(Value::Catchable(cek)),
+            Ok(path) => Ok(generators::request_read_to_string(&co, path).await),
+        }
     }
 }
 
diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index 322f917108..13355fa1c5 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -40,18 +40,25 @@ pub const CURRENT_PLATFORM: &str = env!("TVIX_CURRENT_SYSTEM");
 /// builtin. This coercion can _never_ be performed in a Nix program
 /// without using builtins (i.e. the trick `path: /. + path` to
 /// convert from a string to a path wouldn't hit this code).
-pub async fn coerce_value_to_path(co: &GenCo, v: Value) -> Result<PathBuf, ErrorKind> {
+pub async fn coerce_value_to_path(
+    co: &GenCo,
+    v: Value,
+) -> Result<Result<PathBuf, CatchableErrorKind>, ErrorKind> {
     let value = generators::request_force(co, v).await;
     if let Value::Path(p) = value {
-        return Ok(*p);
+        return Ok(Ok(*p));
     }
 
-    let vs = generators::request_string_coerce(co, value, CoercionKind::Weak).await;
-    let path = PathBuf::from(vs.as_str());
-    if path.is_absolute() {
-        Ok(path)
-    } else {
-        Err(ErrorKind::NotAnAbsolutePath(path))
+    match generators::request_string_coerce(co, value, CoercionKind::Weak).await {
+        Ok(vs) => {
+            let path = PathBuf::from(vs.as_str());
+            if path.is_absolute() {
+                Ok(Ok(path))
+            } else {
+                Err(ErrorKind::NotAnAbsolutePath(path))
+            }
+        }
+        Err(cek) => Ok(Err(cek)),
     }
 }
 
@@ -218,8 +225,10 @@ mod pure_builtins {
             if i != 0 {
                 res.push_str(&separator);
             }
-            let s = generators::request_string_coerce(&co, val, CoercionKind::Weak).await;
-            res.push_str(s.as_str());
+            match generators::request_string_coerce(&co, val, CoercionKind::Weak).await {
+                Ok(s) => res.push_str(s.as_str()),
+                Err(c) => return Ok(Value::Catchable(c)),
+            }
         }
         Ok(res.into())
     }
@@ -313,6 +322,9 @@ mod pure_builtins {
             // and our tests for foldl'.
             nul = generators::request_call_with(&co, op.clone(), [nul, val]).await;
             nul = generators::request_force(&co, nul).await;
+            if let c @ Value::Catchable(_) = nul {
+                return Ok(c);
+            }
         }
 
         Ok(nul)
@@ -340,9 +352,13 @@ mod pure_builtins {
 
     #[builtin("toJSON")]
     async fn builtin_to_json(co: GenCo, val: Value) -> Result<Value, ErrorKind> {
-        let json_value = val.to_json(&co).await?;
-        let json_str = serde_json::to_string(&json_value)?;
-        Ok(json_str.into())
+        match val.to_json(&co).await? {
+            Err(cek) => Ok(Value::Catchable(cek)),
+            Ok(json_value) => {
+                let json_str = serde_json::to_string(&json_value)?;
+                Ok(json_str.into())
+            }
+        }
     }
 
     #[builtin("fromTOML")]
@@ -893,7 +909,7 @@ mod pure_builtins {
 
     #[builtin("throw")]
     async fn builtin_throw(co: GenCo, message: Value) -> Result<Value, ErrorKind> {
-        Err(ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(
+        Ok(Value::Catchable(CatchableErrorKind::Throw(
             message.to_str()?.to_string(),
         )))
     }
@@ -929,15 +945,20 @@ mod pure_builtins {
 
     #[builtin("toPath")]
     async fn builtin_to_path(co: GenCo, s: Value) -> Result<Value, ErrorKind> {
-        let path: Value = crate::value::canon_path(coerce_value_to_path(&co, s).await?).into();
-        Ok(path.coerce_to_string(co, CoercionKind::Weak).await?)
+        match coerce_value_to_path(&co, s).await? {
+            Err(cek) => return Ok(Value::Catchable(cek)),
+            Ok(path) => {
+                let path: Value = crate::value::canon_path(path).into();
+                Ok(path.coerce_to_string(co, CoercionKind::Weak).await?)
+            }
+        }
     }
 
     #[builtin("tryEval")]
     async fn builtin_try_eval(co: GenCo, #[lazy] e: Value) -> Result<Value, ErrorKind> {
         let res = match generators::request_try_force(&co, e).await {
-            Some(value) => [("value", value), ("success", true.into())],
-            None => [("value", false.into()), ("success", false.into())],
+            Value::Catchable(_) => [("value", false.into()), ("success", false.into())],
+            value => [("value", value), ("success", true.into())],
         };
 
         Ok(Value::attrs(NixAttrs::from_iter(res.into_iter())))
diff --git a/tvix/eval/src/builtins/to_xml.rs b/tvix/eval/src/builtins/to_xml.rs
index 9fb6bb5f51..6d486d3565 100644
--- a/tvix/eval/src/builtins/to_xml.rs
+++ b/tvix/eval/src/builtins/to_xml.rs
@@ -134,6 +134,10 @@ fn value_variant_to_xml<W: Write>(w: &mut EventWriter<W>, value: &Value) -> Resu
                 metadata: Some(Rc::new(value.clone())),
             })
         }
+
+        Value::Catchable(_) => {
+            panic!("tvix bug: value_to_xml() called on a value which had not been deep-forced")
+        }
     }?;
 
     Ok(())