Improve NodeError infrastructure

editor/src/messages/portfolio/document/node_graph/node_graph_message_handler.rs

@@ -3830,10 +3830,7 @@ impl NodeGraphMessageHandler {
 				.evaluation_errors
 				.iter()
 				.filter(|error| inside(&error.node_path))
-				.map(|error| match error.node_path.len() == depth + 1 {
-					true => (error.node_path[depth], error.message.clone()),
-					false => (error.node_path[depth], "Node graph error within this node".to_string()),
-				})
+				.map(|error| (error.node_path[depth], error.message.clone()))
 				.collect(),
 			false => graph_errors,
 		};

editor/src/node_graph_executor.rs

@@ -48,12 +48,28 @@ pub struct ExecutionResponse {
 pub enum ExecutionResult {
 	Pending,
 	Value(TaggedValue),
-	Fallback(TaggedValue, EvaluationError),
-	Error(EvaluationError),
+	Fallback(TaggedValue, EvaluationErrors),
+	Error(EvaluationErrors),
 	Panic(Box<dyn Any + Send>),
 }
 
-pub type EvaluationError = Box<[(Arc<dyn std::error::Error + Send + Sync>, Box<[NodeId]>)]>;
+#[derive(Debug)]
+pub struct EvaluationErrors(Box<[(Arc<dyn std::error::Error + Send + Sync>, NodeErrorTrace)]>);
+pub type NodeErrorTrace = Box<[NodePaths]>;
+pub type NodePaths = Box<[Box<[NodeId]>]>;
+
+impl EvaluationErrors {
+	fn iter(&self) -> impl Iterator<Item = (&(dyn std::error::Error + Send + Sync), &NodeErrorTrace)> {
+		self.0.iter().map(|(error, trace)| (&**error, trace))
+	}
+}
+
+impl std::fmt::Display for EvaluationErrors {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		let messages: Vec<_> = self.iter().map(|(error, _)| error.to_string()).collect();
+		write!(f, "{}", messages.join("\n"))
+	}
+}
 
 #[derive(serde::Serialize, serde::Deserialize)]
 pub struct CompilationResponse {
@@ -462,13 +478,24 @@ impl NodeGraphExecutor {
 					// A pending graph keeps the last frame, and the errors shown with it
 					if execution_context.export_config.is_none() && !matches!(result, ExecutionResult::Pending) {
 						let errors = match &result {
-							ExecutionResult::Fallback(_, errors) | ExecutionResult::Error(errors) => errors
-								.iter()
-								.map(|(error, node_path)| NodeEvaluationError {
-									node_path: node_path.to_vec(),
-									message: error.to_string(),
-								})
-								.collect(),
+							ExecutionResult::Fallback(_, errors) | ExecutionResult::Error(errors) => {
+								let in_document = |path: &&[NodeId]| path.split_last().is_some_and(|(node, network)| document.network_interface.document_node(node, network).is_some());
+								errors
+									.iter()
+									.flat_map(|(error, trace)| {
+										let paths = trace
+											.iter()
+											.rev()
+											.map(|paths| paths.iter().map(|path| &**path).filter(in_document).collect::<Vec<_>>())
+											.find(|paths| !paths.is_empty())
+											.unwrap_or_default();
+										paths.into_iter().map(|node_path| NodeEvaluationError {
+											node_path: node_path.to_vec(),
+											message: error.to_string(),
+										})
+									})
+									.collect()
+							}
 							ExecutionResult::Pending | ExecutionResult::Value(_) | ExecutionResult::Panic(_) => Vec::new(),
 						};
 						responses.add(NodeGraphMessage::UpdateEvaluationErrors { errors });
@@ -477,7 +504,7 @@ impl NodeGraphExecutor {
 					let node_graph_output = match result {
 						ExecutionResult::Pending => Ok(None),
 						ExecutionResult::Value(value) | ExecutionResult::Fallback(value, _) => Ok(Some(value)),
-						ExecutionResult::Error(errors) => Err(errors.iter().map(|(error, _)| error.to_string()).collect::<Vec<_>>().join("\n")),
+						ExecutionResult::Error(errors) => Err(errors.to_string()),
 						ExecutionResult::Panic(payload) => {
 							let message = payload.downcast_ref::<&str>().copied().or_else(|| payload.downcast_ref::<String>().map(String::as_str));
 							Err(format!("Panicked: {}", message.unwrap_or("no message")))

editor/src/node_graph_executor/runtime.rs

@@ -285,7 +285,7 @@ impl NodeRuntime {
 							ExecutionResult::Fallback(value, self.resolve_errors(&error))
 						}
 						Ok(Ok(Poll::Error(error))) => ExecutionResult::Error(self.resolve_errors(&error)),
-						Ok(Err(error)) => ExecutionResult::Error(Box::new([(Arc::clone(NodeError::new(error).error()), Box::default())])),
+						Ok(Err(error)) => ExecutionResult::Error(EvaluationErrors(Box::new([(Arc::clone(NodeError::new(error).error()), NodeErrorTrace::default())]))),
 						Err(payload) => {
 							let response = ExecutionResponse {
 								execution_id,
@@ -450,23 +450,24 @@ impl NodeRuntime {
 		}
 	}
 
-	fn resolve_errors(&self, error: &NodeError) -> EvaluationError {
+	fn resolve_errors(&self, error: &NodeError) -> EvaluationErrors {
 		let source_map = self.executor.tree().source_map();
+		let paths = |node: NodeId| -> NodePaths {
+			let mut paths: Vec<_> = source_map.iter().filter(|(_, (id, _))| *id == node).map(|(path, _)| path.clone()).collect();
+			paths.sort_unstable();
+			paths.into()
+		};
 		let mut errors = Vec::new();
-		let mut pending = vec![(Some(self.executor.output()), error)];
-		while let Some((node, error)) = pending.pop() {
-			let origin = node.and_then(|node| self.executor.find_origin(node, error));
-			let start = errors.len();
-			if let Some(origin) = origin {
-				errors.extend(source_map.iter().filter(|(_, (node, _))| *node == origin).map(|(path, _)| (Arc::clone(error.error()), path.clone())));
-				errors[start..].sort_unstable_by(|(_, a), (_, b)| a.cmp(b));
-			}
-			if errors.len() == start {
-				errors.push((Arc::clone(error.error()), Box::default()));
-			}
-			pending.extend(error.upstream().iter().rev().map(|upstream| (origin, upstream)));
+		let output = self.executor.output();
+		let mut pending = vec![(vec![paths(output)], output, error)];
+		while let Some((mut trace, node, error)) = pending.pop() {
+			let passed = self.executor.trace_error(node, error);
+			let origin = passed.last().copied().unwrap_or(node);
+			trace.extend(passed.into_iter().map(&paths));
+			pending.extend(error.upstream().iter().rev().map(|upstream| (trace.clone(), origin, upstream)));
+			errors.push((Arc::clone(error.error()), trace.into()));
 		}
-		errors.into_boxed_slice()
+		EvaluationErrors(errors.into())
 	}
 
 	/// Updates state data, giving back the layers whose thumbnails were dropped because they're no longer monitored.

node-graph/interpreted-executor/src/dynamic_executor.rs

@@ -144,17 +144,22 @@ impl DynamicExecutor {
 	// Only really needed due to the runtime not knowing about rank adapters.
 	// Once we reworked that we should be able to move this resolution into the runtime.
 	// Also remember to remove `BorrowTree::nodes.3` at that point
-	pub fn find_origin(&self, mut node: NodeId, error: &NodeError) -> Option<NodeId> {
+	pub fn trace_error(&self, mut node: NodeId, error: &NodeError) -> Box<[NodeId]> {
+		let mut passed = Vec::new();
 		let mut trace = error.trace().iter().rev();
 		while let Some(&index) = trace.next() {
 			// Hack to undo rank promotion while traversing the graph
 			if self.typing_context.promotions(node).is_some_and(|promotions| promotions.iter().any(|(argument, _)| *argument == index)) {
 				trace.next();
 			}
 
-			node = *self.tree.nodes.get(&node).map(|(_, _, inputs)| &**inputs)?.get(index)?;
+			let Some(&input) = self.tree.nodes.get(&node).and_then(|(_, _, inputs)| inputs.get(index)) else {
+				break;
+			};
+			node = input;
+			passed.push(node);
 		}
-		Some(node)
+		passed.into_boxed_slice()
 	}
 }
 

node-graph/interpreted-executor/src/dynamic_executor/test.rs

@@ -877,11 +877,14 @@ fn error_traced_through(indices: &[usize]) -> NodeError {
 fn a_trace_leads_to_the_node_that_raised_the_error() {
 	let executor = DynamicExecutor::new(bounding_box_network(TaggedValue::TypeDefault(item!(Vector)))).unwrap();
 
-	assert_eq!(executor.find_origin(executor.output(), &error_traced_through(&[])), Some(NodeId(1)));
-	assert_eq!(executor.find_origin(executor.output(), &error_traced_through(&[0])), Some(NodeId(0)));
-	assert_eq!(executor.find_origin(executor.output(), &error_traced_through(&[0, 0])), None, "a value node has no inputs");
-	assert_eq!(executor.find_origin(NodeId(0), &error_traced_through(&[])), Some(NodeId(0)));
-	assert_eq!(executor.find_origin(NodeId(0), &error_traced_through(&[0])), None, "an upstream trace starts at its parent's origin");
+	assert!(executor.trace_error(executor.output(), &error_traced_through(&[])).is_empty());
+	assert_eq!(*executor.trace_error(executor.output(), &error_traced_through(&[0])), [NodeId(0)]);
+	assert_eq!(*executor.trace_error(executor.output(), &error_traced_through(&[0, 0])), [NodeId(0)], "a value node has no inputs");
+	assert!(executor.trace_error(NodeId(0), &error_traced_through(&[])).is_empty());
+	assert!(
+		executor.trace_error(NodeId(0), &error_traced_through(&[0])).is_empty(),
+		"an upstream trace starts at its parent's origin"
+	);
 }
 
 #[test]
@@ -896,7 +899,7 @@ fn a_trace_steps_over_the_index_of_a_promotion_adapter() {
 	})
 	.unwrap();
 
-	assert_eq!(executor.find_origin(executor.output(), &error_traced_through(&[0, 0])), Some(NodeId(0)));
+	assert_eq!(*executor.trace_error(executor.output(), &error_traced_through(&[0, 0])), [NodeId(0)]);
 }
 
 #[test]
@@ -923,16 +926,14 @@ fn executor_panics_reach_the_host_with_the_original_payload() {
 
 #[test]
 fn chained_fallbacks_both_reach_the_root() {
-	let value_node = ProtoNode::value(ConstructionArgs::Value(TaggedValue::Number(1.).into()), vec![NodeId(0)]);
-	let fail_node = ProtoNode::value(ConstructionArgs::Value(TaggedValue::Bool(true).into()), vec![NodeId(1)]);
-	let mut first = ProtoNode::value(ConstructionArgs::Nodes(vec![NodeId(0), NodeId(1)]), vec![NodeId(2)]);
-	first.identifier = graphene_std::poll_debug::fallback::IDENTIFIER;
-	let mut second = ProtoNode::value(ConstructionArgs::Nodes(vec![NodeId(2), NodeId(1)]), vec![NodeId(3)]);
-	second.identifier = graphene_std::poll_debug::fallback::IDENTIFIER;
 	let executor = DynamicExecutor::new(ProtoNetwork {
 		inputs: vec![],
-		output: NodeId(3),
-		nodes: vec![(NodeId(0), value_node), (NodeId(1), fail_node), (NodeId(2), first), (NodeId(3), second)],
+		output: NodeId(4),
+		nodes: [
+			throw_error_inputs(),
+			vec![(NodeId(3), throw_error_node([0, 1, 2, 1], 3)), (NodeId(4), throw_error_node([3, 1, 2, 1], 4))],
+		]
+		.concat(),
 	})
 	.unwrap();
 
@@ -941,68 +942,143 @@ fn chained_fallbacks_both_reach_the_root() {
 		panic!("Both nodes fall back, so the root should too")
 	};
 	let error = &boxed.1;
-	assert_eq!(executor.find_origin(executor.output(), error), Some(NodeId(3)));
+	assert!(executor.trace_error(executor.output(), error).is_empty());
 	let [upstream] = error.upstream() else {
 		panic!("The second node's error should carry the first node's error")
 	};
-	assert_eq!(executor.find_origin(NodeId(3), upstream), Some(NodeId(2)));
+	assert_eq!(*executor.trace_error(NodeId(4), upstream), [NodeId(3)]);
 }
 
 #[test]
 fn an_error_after_a_fallback_keeps_both() {
-	let value_node = ProtoNode::value(ConstructionArgs::Value(TaggedValue::Number(1.).into()), vec![NodeId(0)]);
-	let fail_node = ProtoNode::value(ConstructionArgs::Value(TaggedValue::Bool(true).into()), vec![NodeId(1)]);
-	let mut fallback = ProtoNode::value(ConstructionArgs::Nodes(vec![NodeId(0), NodeId(1)]), vec![NodeId(2)]);
-	fallback.identifier = graphene_std::poll_debug::fallback::IDENTIFIER;
-	let mut error = ProtoNode::value(ConstructionArgs::Nodes(vec![NodeId(2), NodeId(1)]), vec![NodeId(3)]);
-	error.identifier = graphene_std::poll_debug::error::IDENTIFIER;
 	let executor = DynamicExecutor::new(ProtoNetwork {
 		inputs: vec![],
-		output: NodeId(3),
-		nodes: vec![(NodeId(0), value_node), (NodeId(1), fail_node), (NodeId(2), fallback), (NodeId(3), error)],
+		output: NodeId(5),
+		nodes: [
+			throw_error_inputs(),
+			vec![
+				(NodeId(3), throw_error_node([0, 1, 2, 1], 3)),
+				value_node(4, TaggedValue::Bool(false)),
+				(NodeId(5), throw_error_node([3, 1, 2, 4], 5)),
+			],
+		]
+		.concat(),
 	})
 	.unwrap();
 
 	let context: Context = None;
 	let Ok(Poll::Error(error)) = (&executor).execute(context) else {
 		panic!("The failing node should stop the frame")
 	};
-	assert_eq!(executor.find_origin(executor.output(), &error), Some(NodeId(3)));
+	assert!(executor.trace_error(executor.output(), &error).is_empty());
 	let [upstream] = error.upstream() else {
 		panic!("The failing node's error should carry the fallback before it")
 	};
-	assert_eq!(executor.find_origin(NodeId(3), upstream), Some(NodeId(2)));
+	assert_eq!(*executor.trace_error(NodeId(5), upstream), [NodeId(3)]);
 }
 
 #[test]
 fn a_node_error_names_the_node_that_raised_it() {
-	let value_node = ProtoNode::value(ConstructionArgs::Value(TaggedValue::Number(1.).into()), vec![NodeId(0)]);
-	let fail_node = ProtoNode::value(ConstructionArgs::Value(TaggedValue::Bool(true).into()), vec![NodeId(1)]);
-	let mut error_node = ProtoNode::value(ConstructionArgs::Nodes(vec![NodeId(0), NodeId(1)]), vec![NodeId(2)]);
-	error_node.identifier = graphene_std::poll_debug::error::IDENTIFIER;
-	let mut count_node = ProtoNode::value(ConstructionArgs::Nodes(vec![NodeId(2)]), vec![NodeId(3)]);
-	count_node.identifier = graphene_std::poll_debug::evaluation_count::IDENTIFIER;
 	let executor = DynamicExecutor::new(ProtoNetwork {
 		inputs: vec![],
-		output: NodeId(3),
-		nodes: vec![(NodeId(0), value_node), (NodeId(1), fail_node), (NodeId(2), error_node), (NodeId(3), count_node)],
+		output: NodeId(5),
+		nodes: [
+			throw_error_inputs(),
+			vec![
+				value_node(3, TaggedValue::Bool(false)),
+				(NodeId(4), throw_error_node([0, 1, 2, 3], 4)),
+				(NodeId(5), throw_error_node([4, 3, 2, 3], 5)),
+			],
+		]
+		.concat(),
 	})
-	.expect("The debug nodes should resolve");
+	.expect("The Throw Error nodes should resolve");
 
 	let context: Context = None;
 	let Ok(Poll::Error(error)) = (&executor).execute(context) else {
 		panic!("The failing node should stop the frame")
 	};
-	let origin = executor.find_origin(executor.output(), &error);
-	assert_eq!(origin, Some(NodeId(2)), "The error should name the failing node, not the root that forwarded it");
+	let origin = executor.trace_error(executor.output(), &error).last().copied();
+	assert_eq!(origin, Some(NodeId(4)), "The error should name the failing node, not the root that forwarded it");
 	let paths: Vec<_> = executor
 		.tree()
 		.source_map()
 		.iter()
 		.filter(|(_, (node, _))| Some(*node) == origin)
 		.map(|(path, _)| path.to_vec())
 		.collect();
-	assert_eq!(paths, [vec![NodeId(2)]]);
+	assert_eq!(paths, [vec![NodeId(4)]]);
+}
+
+fn value_node(id: u64, value: TaggedValue) -> (NodeId, ProtoNode) {
+	(NodeId(id), ProtoNode::value(ConstructionArgs::Value(value.into()), vec![NodeId(id)]))
+}
+
+fn throw_error_inputs() -> Vec<(NodeId, ProtoNode)> {
+	vec![
+		value_node(0, TaggedValue::Number(1.)),
+		value_node(1, TaggedValue::Bool(true)),
+		value_node(2, TaggedValue::String("too small".to_string())),
+	]
+}
+
+fn throw_error_node(inputs: [u64; 4], id: u64) -> ProtoNode {
+	let mut node = ProtoNode::value(ConstructionArgs::Nodes(inputs.into_iter().map(NodeId).collect()), vec![NodeId(id)]);
+	node.identifier = graphene_core::poll::throw_error::IDENTIFIER;
+	node
+}
+
+fn handled_error_network(handler: ProtoNodeIdentifier, handler_inputs: &[u64], keep_value: bool) -> DynamicExecutor {
+	let mut handler_node = ProtoNode::value(ConstructionArgs::Nodes(handler_inputs.iter().copied().map(NodeId).collect()), vec![NodeId(6)]);
+	handler_node.identifier = handler;
+
+	DynamicExecutor::new(ProtoNetwork {
+		inputs: vec![],
+		output: NodeId(6),
+		nodes: [
+			throw_error_inputs(),
+			vec![
+				value_node(3, TaggedValue::Bool(keep_value)),
+				(NodeId(4), throw_error_node([0, 1, 2, 3], 4)),
+				value_node(5, TaggedValue::Number(2.)),
+				(NodeId(6), handler_node),
+			],
+		]
+		.concat(),
+	})
+	.expect("The error handling nodes should resolve")
+}
+
+#[test]
+fn a_caught_error_does_not_reach_the_output() {
+	let context: Context = None;
+	for keep_value in [false, true] {
+		let executor = handled_error_network(graphene_core::poll::catch::IDENTIFIER, &[4, 5], keep_value);
+		assert!(matches!((&executor).execute(context.clone()), Ok(Poll::Final(_))), "Catch should hide the error");
+	}
+}
+
+#[test]
+fn suppress_error_hides_only_an_error_with_a_value() {
+	let context: Context = None;
+	let executor = handled_error_network(graphene_core::poll::suppress_error::IDENTIFIER, &[4], true);
+	assert!(
+		matches!((&executor).execute(context.clone()), Ok(Poll::Final(_))),
+		"Suppress Error should keep the value and hide the error"
+	);
+	let executor = handled_error_network(graphene_core::poll::suppress_error::IDENTIFIER, &[4], false);
+	assert!(matches!((&executor).execute(context), Ok(Poll::Error(_))), "An error without a value should still stop the frame");
+}
+
+#[test]
+fn a_fallback_keeps_the_error_on_the_node_that_raised_it() {
+	let executor = handled_error_network(graphene_core::poll::fallback::IDENTIFIER, &[4, 5], false);
+	let context: Context = None;
+	let Ok(Poll::Fallback(boxed)) = (&executor).execute(context) else {
+		panic!("Fallback should keep the frame going with the error")
+	};
+	assert_eq!(boxed.1.to_string(), "too small");
+	assert_eq!(*executor.trace_error(executor.output(), &boxed.1), [NodeId(4)], "The error should stay on the Throw Error node");
 }
 
 #[test]

node-graph/node-macro/src/codegen.rs

@@ -996,7 +996,7 @@ pub(crate) fn generate_node_code(crate_ident: &CrateIdent, parsed: &ParsedNodeFn
 		quote! {
 			#(#description_doc_attrs)*
 			#[inline]
-			#[allow(clippy::too_many_arguments)]
+			#[allow(clippy::too_many_arguments, clippy::type_complexity)]
 			#vis #async_keyword fn #fn_name <#(#fn_generics,)*> (#input_ident: #input_type #(, #data_field_idents: #data_field_types)* #(, #field_idents: #field_types)*) -> #output_type #where_clause #body
 		}
 	};