[timon-schelling]

Allow nodes to return any impl Into<NodeError> and allow async nodes to return error and fallback.

#[node_macro::node(...)]
fn parse(_: impl Ctx, text: Item<String>) -> Result<Item<f64>, ParseFloatError> {
	Ok(Item::new_from_element(text.element().parse()?))
}

#[node_macro::node(...)]
async fn load(_: impl Ctx, hash: Item<ResourceHash>) -> Result<Item<Resource>, std::io::Error> {
	let resource = storage.load(*hash.element()).await.ok_or_else(|| std::io::Error::other("Not in storage"))?;
	Ok(Item::new_from_element(resource))
}

[cubic-dev-ai]

1 issue found across 4 files

Confidence score: 3/5

• poll.rs removes std::error::Error from the public NodeError type, which can break downstream code that uses it as a standard error or propagates it with ?. Preserve the Error implementation or account for this API break.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/libraries/core-types/src/poll.rs">

<violation number="1" location="node-graph/libraries/core-types/src/poll.rs:120">
P2: This replacement removes `std::error::Error` from the public `NodeError` type, so downstream code can no longer use it as `Box<dyn Error>`, propagate it with `?` into standard-error results, or inspect its `source()` chain. Keep the `Error` implementation and expose this conversion through a differently named constructor or method, since the blanket `From<E>` cannot coexist with that implementation.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: This replacement removes std::error::Error from the public NodeError type, so downstream code can no longer use it as Box<dyn Error>, propagate it with ? into standard-error results, or inspect its source() chain. Keep the Error implementation and expose this conversion through a differently named constructor or method, since the blanket From<E> cannot coexist with that implementation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/libraries/core-types/src/poll.rs, line 120:

<comment>This replacement removes `std::error::Error` from the public `NodeError` type, so downstream code can no longer use it as `Box<dyn Error>`, propagate it with `?` into standard-error results, or inspect its `source()` chain. Keep the `Error` implementation and expose this conversion through a differently named constructor or method, since the blanket `From<E>` cannot coexist with that implementation.</comment>

<file context>
@@ -117,9 +117,9 @@ impl std::fmt::Display for NodeError {
-impl Error for NodeError {
-	fn source(&self) -> Option<&(dyn Error + 'static)> {
-		self.error.source()
+impl<E: Error + Send + Sync + 'static> From<E> for NodeError {
+	fn from(error: E) -> Self {
+		Self::new(error)
</file context>

[timon-schelling]

@TrueDoctor This is a tradeoff, we could do something like NodeError::from_std(<impl Error>) instead. I think I like this better for now.

[TrueDoctor]

how is this done in crates like anyhow?

[timon-schelling]

They also impl From<std::Error> and then do the rest with deref I think.