From 05f42519b53575ad3235b5e0a0cd7d71f04076a5 Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Sat, 9 Sep 2023 22:02:56 -0700 Subject: fix(tvix/eval): fix b/281 by adding Value::Catchable 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 Autosubmit: Adam Joseph Tested-by: BuildkiteCI --- tvix/eval/src/value/attrs.rs | 2 +- tvix/eval/src/value/json.rs | 20 +++++++++++----- tvix/eval/src/value/mod.rs | 57 ++++++++++++++++++++++++++++++++++++++------ 3 files changed, 65 insertions(+), 14 deletions(-) (limited to 'tvix/eval/src/value') diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index e4840cc88c13..15a709730c1c 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 38496f8f35f8..1290cce14e48 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 { + pub(crate) async fn to_json( + self, + co: &GenCo, + ) -> Result, 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 { - 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 254bbbc09a04..f5b373e3c4ef 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 for Value { + fn from(c: CatchableErrorKind) -> Value { + Value::Catchable(c) + } +} + +impl From> for Value +where + Value: From, +{ + fn from(v: Result) -> 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, "path", Value::Path(p), p.clone()); gen_cast!(to_attrs, Box, "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"), } } } -- cgit 1.4.1