refactor(tvix/eval): factor CatchableErrorKind out of ErrorKind r/6649

This commit creates a separate enum for "catchable" errors (the kind
that `builtins.tryEval` can detect).

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

5 files changed, 57 insertions, 29 deletions

diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index d701fa08a0..322f917108 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -17,7 +17,7 @@ use crate::vm::generators::{self, GenCo};
 use crate::warnings::WarningKind;
 use crate::{
     self as tvix_eval,
-    errors::ErrorKind,
+    errors::{CatchableErrorKind, ErrorKind},
     value::{CoercionKind, NixAttrs, NixList, NixString, SharedThunkSet, Thunk, Value},
 };
 
@@ -893,7 +893,9 @@ mod pure_builtins {
 
     #[builtin("throw")]
     async fn builtin_throw(co: GenCo, message: Value) -> Result<Value, ErrorKind> {
-        Err(ErrorKind::Throw(message.to_str()?.to_string()))
+        Err(ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(
+            message.to_str()?.to_string(),
+        )))
     }
 
     #[builtin("toString")]
diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 3ab0e4d933..5c2825507f 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -27,7 +27,7 @@ use std::rc::{Rc, Weak};
 use std::sync::Arc;
 
 use crate::chunk::Chunk;
-use crate::errors::{Error, ErrorKind, EvalResult};
+use crate::errors::{CatchableErrorKind, Error, ErrorKind, EvalResult};
 use crate::observer::CompilerObserver;
 use crate::opcode::{CodeIdx, ConstantIdx, Count, JumpOffset, OpCode, UpvalueIdx};
 use crate::spans::LightSpan;
@@ -398,7 +398,9 @@ impl Compiler<'_> {
             if raw_path.len() == 2 {
                 return self.emit_error(
                     node,
-                    ErrorKind::NixPathResolution("Empty <> path not allowed".into()),
+                    ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(
+                        "Empty <> path not allowed".into(),
+                    )),
                 );
             }
             let path = &raw_path[1..(raw_path.len() - 1)];
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index 76f55d6812..54def8d334 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -16,12 +16,19 @@ use crate::spans::ToSpan;
 use crate::value::{CoercionKind, NixString};
 use crate::{SourceCode, Value};
 
+/// "CatchableErrorKind" errors -- those which can be detected by `builtins.tryEval`.
+#[derive(Clone, Debug)]
+pub enum CatchableErrorKind {
+    Throw(String),
+    AssertionFailed,
+    /// Resolving a user-supplied angle brackets path literal failed in some way.
+    NixPathResolution(String),
+}
+
 #[derive(Clone, Debug)]
 pub enum ErrorKind {
     /// These are user-generated errors through builtins.
-    Throw(String),
     Abort(String),
-    AssertionFailed,
 
     DivisionByZero,
 
@@ -55,9 +62,6 @@ pub enum ErrorKind {
         rhs: &'static str,
     },
 
-    /// Resolving a user-supplied angle brackets path literal failed in some way.
-    NixPathResolution(String),
-
     /// Resolving a user-supplied relative or home-relative path literal failed in some way.
     RelativePathResolution(String),
 
@@ -176,6 +180,14 @@ pub enum ErrorKind {
         context: String,
         underlying: Box<ErrorKind>,
     },
+
+    CatchableErrorKind(CatchableErrorKind),
+}
+
+impl From<CatchableErrorKind> for ErrorKind {
+    fn from(c: CatchableErrorKind) -> ErrorKind {
+        ErrorKind::CatchableErrorKind(c)
+    }
 }
 
 impl error::Error for Error {
@@ -235,7 +247,7 @@ impl ErrorKind {
     /// Returns `true` if this error can be caught by `builtins.tryEval`
     pub fn is_catchable(&self) -> bool {
         match self {
-            Self::Throw(_) | Self::AssertionFailed | Self::NixPathResolution(_) => true,
+            Self::CatchableErrorKind(_) => true,
             Self::NativeError { err, .. } | Self::BytecodeError(err) => err.kind.is_catchable(),
             _ => false,
         }
@@ -285,9 +297,13 @@ impl Error {
 impl Display for ErrorKind {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match &self {
-            ErrorKind::Throw(msg) => write!(f, "error thrown: {}", msg),
+            ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(msg)) => {
+                write!(f, "error thrown: {}", msg)
+            }
             ErrorKind::Abort(msg) => write!(f, "evaluation aborted: {}", msg),
-            ErrorKind::AssertionFailed => write!(f, "assertion failed"),
+            ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed) => {
+                write!(f, "assertion failed")
+            }
 
             ErrorKind::DivisionByZero => write!(f, "division by zero"),
 
@@ -324,7 +340,8 @@ impl Display for ErrorKind {
                 write!(f, "can not compare a {} with a {}", lhs, rhs)
             }
 
-            ErrorKind::NixPathResolution(err) | ErrorKind::RelativePathResolution(err) => {
+            ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(err))
+            | ErrorKind::RelativePathResolution(err) => {
                 write!(f, "could not resolve path: {}", err)
             }
 
@@ -724,16 +741,15 @@ impl Error {
         let label = match &self.kind {
             ErrorKind::DuplicateAttrsKey { .. } => "in this attribute set",
             ErrorKind::InvalidAttributeName(_) => "in this attribute set",
-            ErrorKind::NixPathResolution(_) | ErrorKind::RelativePathResolution(_) => {
-                "in this path literal"
-            }
+            ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(_))
+            | ErrorKind::RelativePathResolution(_) => "in this path literal",
             ErrorKind::UnexpectedArgument { .. } => "in this function call",
 
             // The spans for some errors don't have any more descriptive stuff
             // in them, or we don't utilise it yet.
-            ErrorKind::Throw(_)
+            ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(_))
             | ErrorKind::Abort(_)
-            | ErrorKind::AssertionFailed
+            | ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed)
             | ErrorKind::AttributeNotFound { .. }
             | ErrorKind::IndexOutOfBounds { .. }
             | ErrorKind::TailEmptyList
@@ -774,14 +790,14 @@ impl Error {
     /// used to refer users to documentation.
     fn code(&self) -> &'static str {
         match self.kind {
-            ErrorKind::Throw(_) => "E001",
+            ErrorKind::CatchableErrorKind(CatchableErrorKind::Throw(_)) => "E001",
             ErrorKind::Abort(_) => "E002",
-            ErrorKind::AssertionFailed => "E003",
+            ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed) => "E003",
             ErrorKind::InvalidAttributeName { .. } => "E004",
             ErrorKind::AttributeNotFound { .. } => "E005",
             ErrorKind::TypeError { .. } => "E006",
             ErrorKind::Incomparable { .. } => "E007",
-            ErrorKind::NixPathResolution(_) => "E008",
+            ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(_)) => "E008",
             ErrorKind::DynamicKeyInScope(_) => "E009",
             ErrorKind::UnknownStaticVariable => "E010",
             ErrorKind::UnknownDynamicVariable(_) => "E011",
diff --git a/tvix/eval/src/nix_search_path.rs b/tvix/eval/src/nix_search_path.rs
index 79c19752f6..3e553e62bb 100644
--- a/tvix/eval/src/nix_search_path.rs
+++ b/tvix/eval/src/nix_search_path.rs
@@ -3,7 +3,7 @@ use std::convert::Infallible;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
-use crate::errors::ErrorKind;
+use crate::errors::{CatchableErrorKind, ErrorKind};
 use crate::EvalIO;
 
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -134,10 +134,12 @@ impl NixSearchPath {
                 return Ok(p);
             }
         }
-        Err(ErrorKind::NixPathResolution(format!(
-            "path '{}' was not found in the Nix search path",
-            path.display()
-        )))
+        Err(ErrorKind::CatchableErrorKind(
+            CatchableErrorKind::NixPathResolution(format!(
+                "path '{}' was not found in the Nix search path",
+                path.display()
+            )),
+        ))
     }
 }
 
@@ -211,7 +213,10 @@ mod tests {
             let mut io = StdIO {};
             let err = nix_search_path.resolve(&mut io, "nope").unwrap_err();
             assert!(
-                matches!(err, ErrorKind::NixPathResolution(..)),
+                matches!(
+                    err,
+                    ErrorKind::CatchableErrorKind(CatchableErrorKind::NixPathResolution(..))
+                ),
                 "err = {err:?}"
             );
         }
diff --git a/tvix/eval/src/vm/mod.rs b/tvix/eval/src/vm/mod.rs
index d883b25000..07d3725fd9 100644
--- a/tvix/eval/src/vm/mod.rs
+++ b/tvix/eval/src/vm/mod.rs
@@ -21,7 +21,7 @@ use crate::{
     chunk::Chunk,
     cmp_op,
     compiler::GlobalsMap,
-    errors::{Error, ErrorKind, EvalResult},
+    errors::{CatchableErrorKind, Error, ErrorKind, EvalResult},
     io::EvalIO,
     nix_search_path::NixSearchPath,
     observer::RuntimeObserver,
@@ -925,7 +925,10 @@ impl<'o> VM<'o> {
                 }
 
                 OpCode::OpAssertFail => {
-                    frame.error(self, ErrorKind::AssertionFailed)?;
+                    frame.error(
+                        self,
+                        ErrorKind::CatchableErrorKind(CatchableErrorKind::AssertionFailed),
+                    )?;
                 }
 
                 // Data-carrying operands should never be executed,