From e190c098ab4761535105ecf2dd274bb8556a12f7 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 11 Apr 2026 11:53:53 -0400 Subject: [PATCH] refactor: standardize crate error types --- Cargo.lock | 1 + lib/crates/fabro-agent/src/error.rs | 91 +++-- lib/crates/fabro-agent/src/lib.rs | 2 +- .../fabro-cli/src/commands/config/mod.rs | 10 +- lib/crates/fabro-cli/src/main.rs | 2 +- .../fabro-config/src/effective_settings.rs | 12 +- lib/crates/fabro-config/src/error.rs | 89 ++++ lib/crates/fabro-config/src/lib.rs | 28 +- lib/crates/fabro-config/src/load.rs | 24 +- lib/crates/fabro-config/src/project.rs | 61 ++- lib/crates/fabro-config/src/run.rs | 16 +- lib/crates/fabro-config/src/user.rs | 5 +- lib/crates/fabro-core/src/error.rs | 71 ++-- lib/crates/fabro-core/src/lib.rs | 2 +- lib/crates/fabro-graphviz/src/error.rs | 9 +- lib/crates/fabro-graphviz/src/lib.rs | 1 + lib/crates/fabro-llm/src/error.rs | 379 +++++++++++------- lib/crates/fabro-llm/src/lib.rs | 1 + lib/crates/fabro-server/Cargo.toml | 1 + lib/crates/fabro-server/src/error.rs | 72 +++- lib/crates/fabro-server/src/lib.rs | 2 + lib/crates/fabro-server/src/serve.rs | 50 +-- lib/crates/fabro-store/src/error.rs | 5 +- lib/crates/fabro-store/src/lib.rs | 4 +- lib/crates/fabro-workflow/src/error.rs | 131 +++--- lib/crates/fabro-workflow/src/lib.rs | 20 +- 26 files changed, 661 insertions(+), 428 deletions(-) create mode 100644 lib/crates/fabro-config/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index c2f38f4c0..c3f35a61c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1927,6 +1927,7 @@ dependencies = [ "serde_yaml", "sha2", "tempfile", + "thiserror 2.0.18", "tokio", "tokio-rustls", "tokio-stream", diff --git a/lib/crates/fabro-agent/src/error.rs b/lib/crates/fabro-agent/src/error.rs index 6ba7228d6..cacba1f5c 100644 --- a/lib/crates/fabro-agent/src/error.rs +++ b/lib/crates/fabro-agent/src/error.rs @@ -1,4 +1,4 @@ -use fabro_llm::error::SdkError; +use fabro_llm::Error as LlmError; /// Why a session was interrupted. #[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)] @@ -19,9 +19,9 @@ impl std::fmt::Display for InterruptReason { #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, thiserror::Error)] #[serde(tag = "type", content = "data", rename_all = "snake_case")] -pub enum AgentError { +pub enum Error { #[error("LLM error: {0}")] - Llm(#[from] SdkError), + Llm(#[from] LlmError), #[error("Session is closed")] SessionClosed, @@ -36,50 +36,53 @@ pub enum AgentError { Interrupted(InterruptReason), } +pub type Result = std::result::Result; +pub type AgentError = Error; + #[cfg(test)] mod tests { - use fabro_llm::error::{ProviderErrorDetail, ProviderErrorKind}; + use fabro_llm::{ProviderErrorDetail, ProviderErrorKind}; use super::*; #[test] fn agent_error_from_sdk_error() { - let sdk_err = SdkError::Network { + let sdk_err = LlmError::Network { message: "connection refused".into(), - source: None, + source: None, }; - let agent_err = AgentError::from(sdk_err); - assert!(matches!(agent_err, AgentError::Llm(_))); + let agent_err = Error::from(sdk_err); + assert!(matches!(agent_err, Error::Llm(_))); assert!(agent_err.to_string().contains("connection refused")); } #[test] fn session_closed_display() { - let err = AgentError::SessionClosed; + let err = Error::SessionClosed; assert_eq!(err.to_string(), "Session is closed"); } #[test] fn invalid_state_display() { - let err = AgentError::InvalidState("bad state".into()); + let err = Error::InvalidState("bad state".into()); assert_eq!(err.to_string(), "Invalid state: bad state"); } #[test] fn tool_execution_display() { - let err = AgentError::ToolExecution("command failed".into()); + let err = Error::ToolExecution("command failed".into()); assert_eq!(err.to_string(), "Tool execution error: command failed"); } #[test] fn interrupted_display() { - let err = AgentError::Interrupted(InterruptReason::Cancelled); + let err = Error::Interrupted(InterruptReason::Cancelled); assert_eq!(err.to_string(), "Interrupted: cancelled"); } #[test] fn interrupted_wall_clock_timeout_display() { - let err = AgentError::Interrupted(InterruptReason::WallClockTimeout); + let err = Error::Interrupted(InterruptReason::WallClockTimeout); assert_eq!(err.to_string(), "Interrupted: wall clock timeout"); } @@ -87,62 +90,62 @@ mod tests { #[test] fn serde_roundtrip_llm_network() { - let err = AgentError::Llm(SdkError::Network { + let err = Error::Llm(LlmError::Network { message: "connection refused".into(), - source: None, + source: None, }); let json = serde_json::to_string(&err).unwrap(); - let deserialized: AgentError = serde_json::from_str(&json).unwrap(); + let deserialized: Error = serde_json::from_str(&json).unwrap(); assert_eq!(err.to_string(), deserialized.to_string()); } #[test] fn serde_roundtrip_llm_provider() { - let err = AgentError::Llm(SdkError::Provider { - kind: ProviderErrorKind::RateLimit, + let err = Error::Llm(LlmError::Provider { + kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail { - message: "too fast".into(), - provider: "openai".into(), + message: "too fast".into(), + provider: "openai".into(), status_code: Some(429), - error_code: None, + error_code: None, retry_after: Some(2.0), - raw: None, + raw: None, }), }); let json = serde_json::to_string(&err).unwrap(); - let deserialized: AgentError = serde_json::from_str(&json).unwrap(); + let deserialized: Error = serde_json::from_str(&json).unwrap(); assert_eq!(err.to_string(), deserialized.to_string()); } #[test] fn serde_roundtrip_session_closed() { - let err = AgentError::SessionClosed; + let err = Error::SessionClosed; let json = serde_json::to_string(&err).unwrap(); - let deserialized: AgentError = serde_json::from_str(&json).unwrap(); + let deserialized: Error = serde_json::from_str(&json).unwrap(); assert_eq!(err.to_string(), deserialized.to_string()); } #[test] fn serde_roundtrip_invalid_state() { - let err = AgentError::InvalidState("bad".into()); + let err = Error::InvalidState("bad".into()); let json = serde_json::to_string(&err).unwrap(); - let deserialized: AgentError = serde_json::from_str(&json).unwrap(); + let deserialized: Error = serde_json::from_str(&json).unwrap(); assert_eq!(err.to_string(), deserialized.to_string()); } #[test] fn serde_roundtrip_tool_execution() { - let err = AgentError::ToolExecution("cmd failed".into()); + let err = Error::ToolExecution("cmd failed".into()); let json = serde_json::to_string(&err).unwrap(); - let deserialized: AgentError = serde_json::from_str(&json).unwrap(); + let deserialized: Error = serde_json::from_str(&json).unwrap(); assert_eq!(err.to_string(), deserialized.to_string()); } #[test] fn serde_roundtrip_interrupted() { - let err = AgentError::Interrupted(InterruptReason::Cancelled); + let err = Error::Interrupted(InterruptReason::Cancelled); let json = serde_json::to_string(&err).unwrap(); - let deserialized: AgentError = serde_json::from_str(&json).unwrap(); + let deserialized: Error = serde_json::from_str(&json).unwrap(); assert_eq!(err.to_string(), deserialized.to_string()); } @@ -150,15 +153,15 @@ mod tests { #[test] fn clone_all_variants() { - let errors: Vec = vec![ - AgentError::Llm(SdkError::Network { + let errors: Vec = vec![ + Error::Llm(LlmError::Network { message: "refused".into(), - source: None, + source: None, }), - AgentError::SessionClosed, - AgentError::InvalidState("reason".into()), - AgentError::ToolExecution("reason".into()), - AgentError::Interrupted(InterruptReason::Cancelled), + Error::SessionClosed, + Error::InvalidState("reason".into()), + Error::ToolExecution("reason".into()), + Error::Interrupted(InterruptReason::Cancelled), ]; for err in &errors { assert_eq!(err.to_string(), err.clone().to_string()); @@ -169,9 +172,9 @@ mod tests { #[test] fn serde_tag_format_llm() { - let err = AgentError::Llm(SdkError::Network { + let err = Error::Llm(LlmError::Network { message: "refused".into(), - source: None, + source: None, }); let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); @@ -180,7 +183,7 @@ mod tests { #[test] fn serde_tag_format_session_closed() { - let err = AgentError::SessionClosed; + let err = Error::SessionClosed; let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); assert_eq!(v["type"], "session_closed"); @@ -188,7 +191,7 @@ mod tests { #[test] fn serde_tag_format_invalid_state() { - let err = AgentError::InvalidState("x".into()); + let err = Error::InvalidState("x".into()); let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); assert_eq!(v["type"], "invalid_state"); @@ -196,7 +199,7 @@ mod tests { #[test] fn serde_tag_format_tool_execution() { - let err = AgentError::ToolExecution("x".into()); + let err = Error::ToolExecution("x".into()); let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); assert_eq!(v["type"], "tool_execution"); @@ -204,7 +207,7 @@ mod tests { #[test] fn serde_tag_format_interrupted() { - let err = AgentError::Interrupted(InterruptReason::WallClockTimeout); + let err = Error::Interrupted(InterruptReason::WallClockTimeout); let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); assert_eq!(v["type"], "interrupted"); diff --git a/lib/crates/fabro-agent/src/lib.rs b/lib/crates/fabro-agent/src/lib.rs index ec4f08e41..3608c5137 100644 --- a/lib/crates/fabro-agent/src/lib.rs +++ b/lib/crates/fabro-agent/src/lib.rs @@ -30,7 +30,7 @@ pub use agent_profile::AgentProfile; pub use config::{SessionOptions, ToolApprovalAdapter, ToolHookCallback, ToolHookDecision}; #[cfg(feature = "docker")] pub use docker_sandbox::{DockerSandbox, DockerSandboxOptions}; -pub use error::{AgentError, InterruptReason}; +pub use error::{AgentError, Error, InterruptReason, Result}; pub use event::Emitter; pub use fabro_mcp::config::McpServerSettings; pub use history::History; diff --git a/lib/crates/fabro-cli/src/commands/config/mod.rs b/lib/crates/fabro-cli/src/commands/config/mod.rs index 0277ff9aa..a9aec0b0b 100644 --- a/lib/crates/fabro-cli/src/commands/config/mod.rs +++ b/lib/crates/fabro-cli/src/commands/config/mod.rs @@ -60,11 +60,11 @@ async fn merged_config(args: &SettingsArgs) -> anyhow::Result { let base_ctx = CommandContext::base()?; let layers = config_layers(&base_ctx, args.workflow.as_deref())?; if args.local { - return effective_settings::resolve_settings( + return Ok(effective_settings::resolve_settings( layers, None, EffectiveSettingsMode::LocalOnly, - ); + )?); } let ctx = CommandContext::for_target(&args.target)?; @@ -75,7 +75,11 @@ async fn merged_config(args: &SettingsArgs) -> anyhow::Result { user_config::ServerTarget::UnixSocket(_) => EffectiveSettingsMode::LocalDaemon, }; - effective_settings::resolve_settings(layers, Some(&server_settings), mode) + Ok(effective_settings::resolve_settings( + layers, + Some(&server_settings), + mode, + )?) } pub(crate) async fn execute(args: &SettingsArgs, globals: &GlobalArgs) -> anyhow::Result<()> { diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index a26be1ce6..36c9eee99 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -145,7 +145,7 @@ async fn main_inner() -> (String, Result<()>) { false, ) } - Err(err) => return (command_name, Err(err)), + Err(err) => return (command_name, Err(err.into())), } } else { match user_config::load_settings() { diff --git a/lib/crates/fabro-config/src/effective_settings.rs b/lib/crates/fabro-config/src/effective_settings.rs index b32baad84..39ac69a4c 100644 --- a/lib/crates/fabro-config/src/effective_settings.rs +++ b/lib/crates/fabro-config/src/effective_settings.rs @@ -7,12 +7,12 @@ //! `~/.fabro/settings.toml` plus explicit process-local overrides — their //! stanzas in `fabro.toml` and `workflow.toml` remain schema-valid but inert. -use anyhow::{Result, anyhow}; use fabro_types::settings::SettingsLayer; use fabro_types::settings::run::{RunExecutionLayer, RunLayer}; use fabro_types::settings::server::ServerLayer; use crate::merge::combine_files; +use crate::{Error, Result}; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum EffectiveSettingsMode { @@ -23,10 +23,10 @@ pub enum EffectiveSettingsMode { #[derive(Clone, Debug, Default)] pub struct EffectiveSettingsLayers { - pub args: SettingsLayer, + pub args: SettingsLayer, pub workflow: SettingsLayer, - pub project: SettingsLayer, - pub user: SettingsLayer, + pub project: SettingsLayer, + pub user: SettingsLayer, } impl EffectiveSettingsLayers { @@ -65,9 +65,7 @@ pub fn resolve_settings( args, )), EffectiveSettingsMode::RemoteServer | EffectiveSettingsMode::LocalDaemon => { - let server_settings = server_settings.ok_or_else(|| { - anyhow!("server settings are required for server-targeted settings resolution") - })?; + let server_settings = server_settings.ok_or(Error::MissingServerSettings)?; // Owner-specific domains (cli, server) may only come from the // local ~/.fabro/settings.toml, never from fabro.toml or // workflow.toml. The user layer keeps its cli/server fields. diff --git a/lib/crates/fabro-config/src/error.rs b/lib/crates/fabro-config/src/error.rs new file mode 100644 index 000000000..01ba23d44 --- /dev/null +++ b/lib/crates/fabro-config/src/error.rs @@ -0,0 +1,89 @@ +use std::path::{Path, PathBuf}; + +use crate::parse::ParseError; +use crate::resolve::ResolveError; + +fn format_resolve_errors(errors: &[ResolveError]) -> String { + errors + .iter() + .map(std::string::ToString::to_string) + .collect::>() + .join("\n") +} + +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error("reading config file {path}: {source}")] + ReadFile { + path: PathBuf, + #[source] + source: std::io::Error, + }, + + #[error("{context}: {source}")] + ParseSettings { + context: &'static str, + #[source] + source: ParseError, + }, + + #[error("parsing TOML config at {path}: {source}")] + TomlParse { + path: PathBuf, + #[source] + source: toml::de::Error, + }, + + #[error("{context}:\n{}", format_resolve_errors(.errors))] + Resolve { + context: &'static str, + errors: Vec, + }, + + #[error("missing required environment variable {var} for {field}")] + MissingEnvVar { + field: String, + var: String, + #[source] + source: std::env::VarError, + }, + + #[error("workflow not found: {0}")] + WorkflowNotFound(String), + + #[error("server settings are required for server-targeted settings resolution")] + MissingServerSettings, + + #[error("{0}")] + Other(String), +} + +impl Error { + pub fn read_file(path: &Path, source: std::io::Error) -> Self { + Self::ReadFile { + path: path.to_path_buf(), + source, + } + } + + pub fn parse(context: &'static str, source: ParseError) -> Self { + Self::ParseSettings { context, source } + } + + pub fn toml_parse(path: &Path, source: toml::de::Error) -> Self { + Self::TomlParse { + path: path.to_path_buf(), + source, + } + } + + pub fn resolve(context: &'static str, errors: Vec) -> Self { + Self::Resolve { context, errors } + } + + pub fn other(message: impl Into) -> Self { + Self::Other(message.into()) + } +} + +pub type Result = std::result::Result; diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index 862c363fa..2e8f40e77 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -1,6 +1,7 @@ extern crate self as fabro_config; pub mod effective_settings; +pub mod error; pub mod home; pub mod legacy_env; pub mod load; @@ -14,6 +15,7 @@ pub mod user; use std::path::Path; +pub use error::{Error, Result}; use fabro_types::settings::{Settings, SettingsLayer}; pub use fabro_util::path::expand_tilde; pub use home::Home; @@ -34,39 +36,33 @@ pub fn load_and_resolve( layers: effective_settings::EffectiveSettingsLayers, server_settings: Option<&SettingsLayer>, mode: effective_settings::EffectiveSettingsMode, -) -> anyhow::Result { +) -> Result { let layer = effective_settings::resolve_settings(layers, server_settings, mode)?; - resolve(&layer).map_err(|errors| { - anyhow::anyhow!( - "failed to resolve settings:\n{}", - errors - .into_iter() - .map(|error| error.to_string()) - .collect::>() - .join("\n") - ) - }) + resolve(&layer).map_err(|errors| Error::resolve("failed to resolve settings", errors)) } /// Load a TOML config from an explicit path or `~/.fabro/{filename}`. /// /// Returns `T::default()` when no explicit path is given and the default file /// doesn't exist. An explicit path that doesn't exist is an error. -pub fn load_config_file(path: Option<&Path>, filename: &str) -> anyhow::Result +pub fn load_config_file(path: Option<&Path>, filename: &str) -> Result where T: Default + DeserializeOwned, { if let Some(explicit) = path { tracing::debug!(path = %explicit.display(), "Loading config from explicit path"); - let contents = std::fs::read_to_string(explicit)?; - return Ok(toml::from_str(&contents)?); + let contents = std::fs::read_to_string(explicit) + .map_err(|source| Error::read_file(explicit, source))?; + return toml::from_str(&contents).map_err(|source| Error::toml_parse(explicit, source)); } let default_path = Home::from_env().root().join(filename); tracing::debug!(path = %default_path.display(), "Loading config"); match std::fs::read_to_string(&default_path) { - Ok(contents) => Ok(toml::from_str(&contents)?), + Ok(contents) => { + toml::from_str(&contents).map_err(|source| Error::toml_parse(&default_path, source)) + } Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(T::default()), - Err(e) => Err(e.into()), + Err(e) => Err(Error::read_file(&default_path, e)), } } diff --git a/lib/crates/fabro-config/src/load.rs b/lib/crates/fabro-config/src/load.rs index 9c7dfa0ec..dcfd6059e 100644 --- a/lib/crates/fabro-config/src/load.rs +++ b/lib/crates/fabro-config/src/load.rs @@ -1,31 +1,27 @@ use std::path::{Path, PathBuf}; -use anyhow::Context; use fabro_types::settings::run::RunGoalLayer; use fabro_types::settings::{InterpString, SettingsLayer}; use crate::merge::combine_files; use crate::parse::parse_settings_layer; -use crate::{project, user}; +use crate::{Error, Result, project, user}; -pub fn load_settings_path(path: &Path) -> anyhow::Result { - let content = std::fs::read_to_string(path) - .with_context(|| format!("Failed to read {}", path.display()))?; +pub fn load_settings_path(path: &Path) -> Result { + let content = std::fs::read_to_string(path).map_err(|source| Error::read_file(path, source))?; let mut layer = parse_settings_layer(&content) - .map_err(|err| anyhow::anyhow!("{err}")) - .context("Failed to parse settings file")?; + .map_err(|err| Error::parse("Failed to parse settings file", err))?; let base_dir = path.parent().unwrap_or_else(|| Path::new(".")); resolve_goal_file_paths(&mut layer, base_dir); Ok(layer) } -pub fn load_settings_for_workflow(path: &Path, cwd: &Path) -> anyhow::Result { +pub fn load_settings_for_workflow(path: &Path, cwd: &Path) -> Result { let resolution = project::resolve_workflow_path(path, cwd)?; if resolution.workflow_config.is_none() && !resolution.resolved_workflow_path.is_file() { - anyhow::bail!( - "Workflow not found: {}", - resolution.resolved_workflow_path.display() - ); + return Err(Error::WorkflowNotFound( + resolution.resolved_workflow_path.display().to_string(), + )); } let workflow_config = resolution.workflow_config.unwrap_or_default(); @@ -41,13 +37,13 @@ pub fn load_settings_for_workflow(path: &Path, cwd: &Path) -> anyhow::Result anyhow::Result { +pub fn load_settings_project(start: &Path) -> Result { Ok(project::discover_project_config(start)? .map(|(_, config)| config) .unwrap_or_default()) } -pub fn load_settings_user() -> anyhow::Result { +pub fn load_settings_user() -> Result { user::load_settings_config(None) } diff --git a/lib/crates/fabro-config/src/project.rs b/lib/crates/fabro-config/src/project.rs index af03ec584..d1741265e 100644 --- a/lib/crates/fabro-config/src/project.rs +++ b/lib/crates/fabro-config/src/project.rs @@ -7,39 +7,39 @@ use std::fmt::Write; use std::path::{Path, PathBuf}; -use anyhow::{Context, bail}; use fabro_types::settings::SettingsLayer; use serde::Serialize; use crate::load::load_settings_path; use crate::parse::parse_settings_layer; -use crate::{resolve_project_from_file, resolve_run_from_file, resolve_workflow_from_file, run}; +use crate::{ + Error, Result, resolve_project_from_file, resolve_run_from_file, resolve_workflow_from_file, + run, +}; const CONFIG_FILENAME: &str = "fabro.toml"; #[derive(Clone, Debug)] pub struct WorkflowPathResolution { pub resolved_workflow_path: PathBuf, - pub dot_path: PathBuf, - pub workflow_config: Option, - pub workflow_toml_path: Option, - pub workflow_slug: Option, + pub dot_path: PathBuf, + pub workflow_config: Option, + pub workflow_toml_path: Option, + pub workflow_slug: Option, } /// Parse a project config from a TOML string. -pub fn parse_project_config(content: &str) -> anyhow::Result { - parse_settings_layer(content) - .map_err(|err| anyhow::anyhow!("{err}")) - .context("Failed to parse project config") +pub fn parse_project_config(content: &str) -> Result { + parse_settings_layer(content).map_err(|err| Error::parse("Failed to parse project config", err)) } /// Load a project config from a file path. /// /// Goes through [`load_settings_path`] so that relative `run.goal.file` /// paths are anchored at the directory of `path` at load time. -pub fn load_project_config(path: &Path) -> anyhow::Result { - let config = load_settings_path(path).context("Failed to parse project config")?; +pub fn load_project_config(path: &Path) -> Result { + let config = load_settings_path(path)?; let root = resolve_project_from_file(&config) - .map_err(|errors| anyhow::anyhow!("Failed to resolve project settings: {errors:?}"))? + .map_err(|errors| Error::resolve("Failed to resolve project settings", errors))? .directory; tracing::debug!(path = %path.display(), root = %root, "Loaded project config"); Ok(config) @@ -47,7 +47,7 @@ pub fn load_project_config(path: &Path) -> anyhow::Result { /// Walk ancestor directories from `start` looking for `fabro.toml`. /// Returns the config file path and parsed config, or `None` if not found. -pub fn discover_project_config(start: &Path) -> anyhow::Result> { +pub fn discover_project_config(start: &Path) -> Result> { for ancestor in start.ancestors() { let candidate = ancestor.join(CONFIG_FILENAME); if candidate.is_file() { @@ -78,22 +78,19 @@ fn workflow_slug_from_path(workflow_path: &Path) -> Option { } /// Resolve a workflow argument to a path. -pub fn resolve_workflow_arg(arg: &Path) -> anyhow::Result { +pub fn resolve_workflow_arg(arg: &Path) -> Result { let start = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); resolve_workflow_arg_from(arg, &start) } -pub fn resolve_workflow_path( - workflow_path: &Path, - cwd: &Path, -) -> anyhow::Result { +pub fn resolve_workflow_path(workflow_path: &Path, cwd: &Path) -> Result { let path = resolve_workflow_arg_from(workflow_path, cwd)?; let workflow_slug = workflow_slug_from_path(&path); if path.extension().is_some_and(|ext| ext == "toml") { match run::load_run_config(&path) { Ok(cfg) => { let workflow = resolve_workflow_from_file(&cfg).map_err(|errors| { - anyhow::anyhow!("Failed to resolve workflow settings: {errors:?}") + Error::resolve("Failed to resolve workflow settings", errors) })?; let dot_path = run::resolve_graph_path(&path, &workflow.graph); Ok(WorkflowPathResolution { @@ -104,7 +101,7 @@ pub fn resolve_workflow_path( workflow_slug, }) } - Err(_) if !path.exists() => anyhow::bail!("Workflow not found: {}", path.display()), + Err(_) if !path.exists() => Err(Error::WorkflowNotFound(path.display().to_string())), Err(err) => Err(err), } } else { @@ -134,7 +131,7 @@ pub fn resolve_working_directory(settings: &SettingsLayer, caller_cwd: &Path) -> } } -fn resolve_workflow_arg_from(arg: &Path, start_dir: &Path) -> anyhow::Result { +fn resolve_workflow_arg_from(arg: &Path, start_dir: &Path) -> Result { resolve_workflow_arg_impl(arg, start_dir, Some(&user_workflows_dir())) } @@ -142,7 +139,7 @@ fn resolve_workflow_arg_impl( arg: &Path, start_dir: &Path, user_workflows: Option<&Path>, -) -> anyhow::Result { +) -> Result { if arg.extension().is_some() { let resolved = if arg.is_absolute() { arg.to_path_buf() @@ -177,10 +174,10 @@ fn resolve_workflow_arg_impl( let project_wf_dir = fabro_root.join("workflows"); let available = list_available_workflows(Some(&project_wf_dir), user_workflows); if available.is_empty() { - bail!( + return Err(Error::other(format!( "Unknown workflow '{name}'\n\nNo workflows found in {}", project_wf_dir.display() - ); + ))); } let mut msg = format!( "Unknown workflow '{name}'\n\nAvailable workflows: {}", @@ -189,7 +186,7 @@ fn resolve_workflow_arg_impl( if let Some(suggestion) = find_closest_match(&name, &available) { let _ = write!(msg, "\n\nDid you mean '{suggestion}'?"); } - bail!("{msg}"); + Err(Error::other(msg)) } Ok(None) => { if let Some(resolved) = resolve_user_workflow(user_workflows, &name, arg) { @@ -225,8 +222,8 @@ fn user_workflows_dir() -> PathBuf { /// Metadata about a discovered workflow. #[derive(Clone, Debug, Serialize)] pub struct WorkflowInfo { - pub name: String, - pub goal: Option, + pub name: String, + pub goal: Option, pub source: WorkflowSource, } @@ -342,7 +339,7 @@ fn find_closest_match(input: &str, candidates: &[String]) -> Option { } /// Resolve a workflow argument to a DOT path and optional run config. -pub fn resolve_workflow(arg: &Path) -> anyhow::Result<(PathBuf, Option)> { +pub fn resolve_workflow(arg: &Path) -> Result<(PathBuf, Option)> { let start = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let resolution = resolve_workflow_path(arg, &start)?; Ok((resolution.dot_path, resolution.workflow_config)) @@ -442,11 +439,7 @@ retros = true #[test] fn parse_higher_version_errors() { let err = parse_project_config("_version = 2\n").unwrap_err(); - let chain: String = err - .chain() - .map(ToString::to_string) - .collect::>() - .join("; "); + let chain = format!("{err:#}"); assert!( chain.contains("Upgrade") || chain.to_lowercase().contains("version"), "Expected version hint in chain: {chain}" diff --git a/lib/crates/fabro-config/src/run.rs b/lib/crates/fabro-config/src/run.rs index 89aea255d..d6a6d93af 100644 --- a/lib/crates/fabro-config/src/run.rs +++ b/lib/crates/fabro-config/src/run.rs @@ -7,27 +7,25 @@ use std::path::{Path, PathBuf}; -use anyhow::Context; use fabro_types::settings::SettingsLayer; use fabro_types::settings::run::{ResolvedGoalSource, ResolvedRunGoal, RunGoalLayer}; use crate::load::{load_settings_path, resolve_goal_file_path}; use crate::parse::parse_settings_layer; +use crate::{Error, Result}; /// Load and parse a run config from a TOML file. -pub fn parse_run_config(contents: &str) -> anyhow::Result { +pub fn parse_run_config(contents: &str) -> Result { parse_settings_layer(contents) - .map_err(|err| anyhow::anyhow!("{err}")) - .context("Failed to parse run config TOML") + .map_err(|err| Error::parse("Failed to parse run config TOML", err)) } /// Load and parse a run config from a TOML file. /// /// Goes through [`load_settings_path`] so that relative `run.goal.file` /// paths are anchored at the directory of `path` at load time. -pub fn load_run_config(path: &Path) -> anyhow::Result { +pub fn load_run_config(path: &Path) -> Result { load_settings_path(path) - .with_context(|| format!("Failed to parse workflow config at {}", path.display())) } /// Resolve a graph path relative to a workflow.toml. @@ -45,7 +43,7 @@ pub enum ResolveRunGoalError { var: String, }, Io { - path: PathBuf, + path: PathBuf, source: std::io::Error, }, } @@ -76,14 +74,14 @@ impl std::error::Error for ResolveRunGoalError { pub fn resolve_run_goal( settings: &SettingsLayer, base_dir: &Path, -) -> Result, ResolveRunGoalError> { +) -> std::result::Result, ResolveRunGoalError> { let Some(goal) = settings.run.as_ref().and_then(|run| run.goal.as_ref()) else { return Ok(None); }; match goal { RunGoalLayer::Inline(text) => Ok(Some(ResolvedRunGoal { - text: text.as_source(), + text: text.as_source(), source: ResolvedGoalSource::Inline, })), RunGoalLayer::File { file } => { diff --git a/lib/crates/fabro-config/src/user.rs b/lib/crates/fabro-config/src/user.rs index da15e9282..a4a390a4a 100644 --- a/lib/crates/fabro-config/src/user.rs +++ b/lib/crates/fabro-config/src/user.rs @@ -10,6 +10,7 @@ use std::sync::{Mutex, OnceLock}; use fabro_types::settings::SettingsLayer; +use crate::Result; use crate::home::Home; use crate::load::load_settings_path; @@ -77,7 +78,7 @@ fn should_warn_about_legacy_user_config(path: &Path) -> bool { /// returning defaults if the default file doesn't exist. An explicit path that /// doesn't exist is an error. #[allow(clippy::print_stderr)] -pub fn load_settings_config(path: Option<&Path>) -> anyhow::Result { +pub fn load_settings_config(path: Option<&Path>) -> Result { if let Some(explicit) = path .map(Path::to_path_buf) .or_else(|| std::env::var_os(FABRO_CONFIG_ENV).map(PathBuf::from)) @@ -111,7 +112,7 @@ pub fn load_settings_config(path: Option<&Path>) -> anyhow::Result anyhow::Result { +fn load_v2_layer_from_path(path: &Path) -> Result { load_settings_path(path) } diff --git a/lib/crates/fabro-core/src/error.rs b/lib/crates/fabro-core/src/error.rs index c45bafc01..cda29f081 100644 --- a/lib/crates/fabro-core/src/error.rs +++ b/lib/crates/fabro-core/src/error.rs @@ -17,14 +17,14 @@ impl fmt::Display for VisitLimitSource { } } -/// Structured failure data on handler errors. Maps to FabroError's +/// Structured failure data on handler errors. Maps to workflow error's /// is_retryable(), failure_class(), failure_signature_hint(), /// to_fail_outcome(). #[derive(Debug, Clone)] pub struct HandlerErrorDetail { - pub message: String, + pub message: String, pub retryable: bool, - pub category: Option, + pub category: Option, pub signature: Option, } @@ -35,7 +35,7 @@ impl fmt::Display for HandlerErrorDetail { } #[derive(Debug, thiserror::Error)] -pub enum CoreError { +pub enum Error { #[error("node not found: {id}")] NodeNotFound { id: String }, #[error("no start node found in graph")] @@ -48,9 +48,9 @@ pub enum CoreError { "node \"{node_id}\" visited {visits} times ({limit_source} limit {limit}); run is stuck in a cycle" )] VisitLimitExceeded { - node_id: String, - visits: usize, - limit: usize, + node_id: String, + visits: usize, + limit: usize, limit_source: VisitLimitSource, }, #[error("stall timeout on node \"{node_id}\"")] @@ -61,7 +61,7 @@ pub enum CoreError { Other(String), } -impl CoreError { +impl Error { pub fn handler(detail: HandlerErrorDetail) -> Self { Self::Handler { detail } } @@ -81,8 +81,8 @@ impl CoreError { Self::Handler { detail } => Outcome { status: StageStatus::Fail, failure: Some(FailureDetail { - message: detail.message.clone(), - category: detail.category.unwrap_or(FailureCategory::Deterministic), + message: detail.message.clone(), + category: detail.category.unwrap_or(FailureCategory::Deterministic), signature: detail.signature.clone(), }), ..Outcome::default() @@ -92,7 +92,8 @@ impl CoreError { } } -pub type Result = std::result::Result; +pub type Result = std::result::Result; +pub type CoreError = Error; #[cfg(test)] mod tests { @@ -101,58 +102,58 @@ mod tests { #[test] fn core_error_display() { assert_eq!( - CoreError::NodeNotFound { id: "n1".into() }.to_string(), + Error::NodeNotFound { id: "n1".into() }.to_string(), "node not found: n1" ); assert_eq!( - CoreError::NoStartNode.to_string(), + Error::NoStartNode.to_string(), "no start node found in graph" ); - assert_eq!(CoreError::Cancelled.to_string(), "run cancelled"); + assert_eq!(Error::Cancelled.to_string(), "run cancelled"); assert_eq!( - CoreError::Blocked { + Error::Blocked { message: "hook denied".into(), } .to_string(), "blocked: hook denied" ); assert_eq!( - CoreError::VisitLimitExceeded { - node_id: "n1".into(), - visits: 5, - limit: 3, + Error::VisitLimitExceeded { + node_id: "n1".into(), + visits: 5, + limit: 3, limit_source: VisitLimitSource::Node, } .to_string(), "node \"n1\" visited 5 times (node limit 3); run is stuck in a cycle" ); assert_eq!( - CoreError::StallTimeout { + Error::StallTimeout { node_id: "work".into(), } .to_string(), "stall timeout on node \"work\"" ); assert_eq!( - CoreError::Other("something broke".into()).to_string(), + Error::Other("something broke".into()).to_string(), "something broke" ); } #[test] fn core_error_handler_is_retryable() { - let retryable = CoreError::handler(HandlerErrorDetail { - message: "timeout".into(), + let retryable = Error::handler(HandlerErrorDetail { + message: "timeout".into(), retryable: true, - category: None, + category: None, signature: None, }); assert!(retryable.is_retryable()); - let not_retryable = CoreError::handler(HandlerErrorDetail { - message: "bad input".into(), + let not_retryable = Error::handler(HandlerErrorDetail { + message: "bad input".into(), retryable: false, - category: None, + category: None, signature: None, }); assert!(!not_retryable.is_retryable()); @@ -161,10 +162,10 @@ mod tests { #[test] fn core_error_handler_to_fail_outcome() { use crate::outcome::FailureCategory; - let err = CoreError::handler(HandlerErrorDetail { - message: "api down".into(), + let err = Error::handler(HandlerErrorDetail { + message: "api down".into(), retryable: true, - category: Some(FailureCategory::TransientInfra), + category: Some(FailureCategory::TransientInfra), signature: Some("sig123".into()), }); let outcome: Outcome = err.to_fail_outcome(); @@ -177,15 +178,15 @@ mod tests { #[test] fn core_error_non_handler_not_retryable() { - assert!(!CoreError::NodeNotFound { id: "x".into() }.is_retryable()); - assert!(!CoreError::Cancelled.is_retryable()); - assert!(!CoreError::NoStartNode.is_retryable()); + assert!(!Error::NodeNotFound { id: "x".into() }.is_retryable()); + assert!(!Error::Cancelled.is_retryable()); + assert!(!Error::NoStartNode.is_retryable()); assert!( - !CoreError::Blocked { + !Error::Blocked { message: "no".into(), } .is_retryable() ); - assert!(!CoreError::Other("err".into()).is_retryable()); + assert!(!Error::Other("err".into()).is_retryable()); } } diff --git a/lib/crates/fabro-core/src/lib.rs b/lib/crates/fabro-core/src/lib.rs index 4629250c2..8bdb0433a 100644 --- a/lib/crates/fabro-core/src/lib.rs +++ b/lib/crates/fabro-core/src/lib.rs @@ -13,7 +13,7 @@ pub mod state; pub mod test_fixtures; pub use context::Context; -pub use error::{CoreError, HandlerErrorDetail, Result, VisitLimitSource}; +pub use error::{CoreError, Error, HandlerErrorDetail, Result, VisitLimitSource}; pub use executor::{Executor, ExecutorBuilder, ExecutorOptions}; pub use graph::{EdgeSelection, EdgeSpec, Graph, NodeSpec}; pub use handler::NodeHandler; diff --git a/lib/crates/fabro-graphviz/src/error.rs b/lib/crates/fabro-graphviz/src/error.rs index 8ea9ea90d..16024d804 100644 --- a/lib/crates/fabro-graphviz/src/error.rs +++ b/lib/crates/fabro-graphviz/src/error.rs @@ -1,10 +1,13 @@ -use thiserror::Error; +use thiserror::Error as ThisError; -#[derive(Debug, Error)] -pub enum GraphvizError { +#[derive(Debug, ThisError)] +pub enum Error { #[error("Parse error: {0}")] Parse(String), #[error("Stylesheet error: {0}")] Stylesheet(String), } + +pub type Result = std::result::Result; +pub type GraphvizError = Error; diff --git a/lib/crates/fabro-graphviz/src/lib.rs b/lib/crates/fabro-graphviz/src/lib.rs index a38f04df6..7d5d92e01 100644 --- a/lib/crates/fabro-graphviz/src/lib.rs +++ b/lib/crates/fabro-graphviz/src/lib.rs @@ -6,4 +6,5 @@ pub mod parser; pub mod render; pub mod stylesheet; +pub use error::{Error, GraphvizError, Result}; pub use fidelity::Fidelity; diff --git a/lib/crates/fabro-llm/src/error.rs b/lib/crates/fabro-llm/src/error.rs index 99fee716d..5c9b97b06 100644 --- a/lib/crates/fabro-llm/src/error.rs +++ b/lib/crates/fabro-llm/src/error.rs @@ -30,23 +30,23 @@ impl std::fmt::Display for ProviderErrorKind { #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] pub struct ProviderErrorDetail { - pub message: String, - pub provider: String, + pub message: String, + pub provider: String, pub status_code: Option, - pub error_code: Option, + pub error_code: Option, pub retry_after: Option, - pub raw: Option, + pub raw: Option, } impl ProviderErrorDetail { pub fn new(message: impl Into, provider: impl Into) -> Self { Self { - message: message.into(), - provider: provider.into(), + message: message.into(), + provider: provider.into(), status_code: None, - error_code: None, + error_code: None, retry_after: None, - raw: None, + raw: None, } } } @@ -55,10 +55,10 @@ use std::sync::Arc; #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, thiserror::Error)] #[serde(tag = "type", rename_all = "snake_case")] -pub enum SdkError { +pub enum Error { #[error("{kind} {}: {}", .detail.provider, .detail.message)] Provider { - kind: ProviderErrorKind, + kind: ProviderErrorKind, detail: Box, }, @@ -67,7 +67,7 @@ pub enum SdkError { message: String, #[source] #[serde(skip)] - source: Option>, + source: Option>, }, #[error("Request interrupted: {message}")] @@ -78,7 +78,7 @@ pub enum SdkError { message: String, #[source] #[serde(skip)] - source: Option>, + source: Option>, }, #[error("Stream error: {message}")] @@ -86,7 +86,7 @@ pub enum SdkError { message: String, #[source] #[serde(skip)] - source: Option>, + source: Option>, }, #[error("Invalid tool call: {message}")] @@ -100,21 +100,21 @@ pub enum SdkError { message: String, #[source] #[serde(skip)] - source: Option>, + source: Option>, }, #[error("Unsupported tool choice: {message}")] UnsupportedToolChoice { message: String }, } -impl SdkError { +impl Error { pub fn network( message: impl Into, source: impl std::error::Error + Send + Sync + 'static, ) -> Self { Self::Network { message: message.into(), - source: Some(Arc::new(source)), + source: Some(Arc::new(source)), } } @@ -124,7 +124,7 @@ impl SdkError { ) -> Self { Self::RequestTimeout { message: message.into(), - source: Some(Arc::new(source)), + source: Some(Arc::new(source)), } } @@ -134,7 +134,7 @@ impl SdkError { ) -> Self { Self::Stream { message: message.into(), - source: Some(Arc::new(source)), + source: Some(Arc::new(source)), } } @@ -144,7 +144,7 @@ impl SdkError { ) -> Self { Self::Configuration { message: message.into(), - source: Some(Arc::new(source)), + source: Some(Arc::new(source)), } } @@ -289,9 +289,9 @@ pub fn error_from_status_code( 403 => ProviderErrorKind::AccessDenied, 404 => ProviderErrorKind::NotFound, 408 => { - return SdkError::RequestTimeout { + return Error::RequestTimeout { message: detail.message, - source: None, + source: None, }; } 413 => ProviderErrorKind::ContextLength, @@ -315,7 +315,7 @@ pub fn error_from_status_code( } }; - SdkError::Provider { + Error::Provider { kind, detail: Box::new(detail), } @@ -347,20 +347,23 @@ pub fn error_from_grpc_status( "PERMISSION_DENIED" => ProviderErrorKind::AccessDenied, "RESOURCE_EXHAUSTED" => ProviderErrorKind::RateLimit, "DEADLINE_EXCEEDED" => { - return SdkError::RequestTimeout { + return Error::RequestTimeout { message: detail.message, - source: None, + source: None, }; } _ => ProviderErrorKind::Server, }; - SdkError::Provider { + Error::Provider { kind, detail: Box::new(detail), } } +pub type Result = std::result::Result; +pub type SdkError = Error; + #[cfg(test)] mod tests { use std::error::Error as _; @@ -370,7 +373,7 @@ mod tests { #[test] fn retryable_classification() { let auth_err = SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail { status_code: Some(401), ..ProviderErrorDetail::new("bad key", "openai") @@ -379,7 +382,7 @@ mod tests { assert!(!auth_err.retryable()); let rate_err = SdkError::Provider { - kind: ProviderErrorKind::RateLimit, + kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail { status_code: Some(429), retry_after: Some(2.0), @@ -390,7 +393,7 @@ mod tests { assert_eq!(rate_err.retry_after(), Some(2.0)); let server_err = SdkError::Provider { - kind: ProviderErrorKind::Server, + kind: ProviderErrorKind::Server, detail: Box::new(ProviderErrorDetail { status_code: Some(500), ..ProviderErrorDetail::new("internal error", "anthropic") @@ -400,19 +403,19 @@ mod tests { let timeout = SdkError::RequestTimeout { message: "timed out".into(), - source: None, + source: None, }; assert!(!timeout.retryable()); let network = SdkError::Network { message: "connection refused".into(), - source: None, + source: None, }; assert!(network.retryable()); let config = SdkError::Configuration { message: "missing provider".into(), - source: None, + source: None, }; assert!(!config.retryable()); } @@ -422,37 +425,37 @@ mod tests { let detail = || Box::new(ProviderErrorDetail::new("error", "openai")); let access_denied = SdkError::Provider { - kind: ProviderErrorKind::AccessDenied, + kind: ProviderErrorKind::AccessDenied, detail: detail(), }; assert!(!access_denied.retryable()); let not_found = SdkError::Provider { - kind: ProviderErrorKind::NotFound, + kind: ProviderErrorKind::NotFound, detail: detail(), }; assert!(!not_found.retryable()); let invalid_req = SdkError::Provider { - kind: ProviderErrorKind::InvalidRequest, + kind: ProviderErrorKind::InvalidRequest, detail: detail(), }; assert!(!invalid_req.retryable()); let ctx_length = SdkError::Provider { - kind: ProviderErrorKind::ContextLength, + kind: ProviderErrorKind::ContextLength, detail: detail(), }; assert!(!ctx_length.retryable()); let quota = SdkError::Provider { - kind: ProviderErrorKind::QuotaExceeded, + kind: ProviderErrorKind::QuotaExceeded, detail: detail(), }; assert!(!quota.retryable()); let content_filter = SdkError::Provider { - kind: ProviderErrorKind::ContentFilter, + kind: ProviderErrorKind::ContentFilter, detail: detail(), }; assert!(!content_filter.retryable()); @@ -486,32 +489,44 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::Authentication, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::Authentication, + .. + } + )); assert!(!err.retryable()); let err = error_from_status_code(403, "forbidden".into(), "openai".into(), None, None, None); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::AccessDenied, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::AccessDenied, + .. + } + )); let err = error_from_status_code(404, "not found".into(), "openai".into(), None, None, None); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::NotFound, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::NotFound, + .. + } + )); let err = error_from_status_code(400, "bad request".into(), "openai".into(), None, None, None); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::InvalidRequest, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::InvalidRequest, + .. + } + )); let err = error_from_status_code( 422, @@ -521,20 +536,26 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::InvalidRequest, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::InvalidRequest, + .. + } + )); let err = error_from_status_code(408, "timeout".into(), "openai".into(), None, None, None); assert!(matches!(err, SdkError::RequestTimeout { .. })); let err = error_from_status_code(413, "too large".into(), "openai".into(), None, None, None); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::ContextLength, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::ContextLength, + .. + } + )); let err = error_from_status_code( 429, @@ -544,26 +565,35 @@ mod tests { None, Some(5.0), ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::RateLimit, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::RateLimit, + .. + } + )); assert!(err.retryable()); assert_eq!(err.retry_after(), Some(5.0)); let err = error_from_status_code(500, "internal".into(), "openai".into(), None, None, None); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::Server, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::Server, + .. + } + )); assert!(err.retryable()); let err = error_from_status_code(502, "bad gateway".into(), "openai".into(), None, None, None); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::Server, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::Server, + .. + } + )); let err = error_from_status_code( 529, @@ -573,10 +603,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::Server, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::Server, + .. + } + )); assert!(err.retryable()); } @@ -590,10 +623,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::ContextLength, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::ContextLength, + .. + } + )); } #[test] @@ -606,10 +642,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::ContextLength, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::ContextLength, + .. + } + )); } #[test] @@ -622,10 +661,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::ContentFilter, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::ContentFilter, + .. + } + )); } #[test] @@ -638,10 +680,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::ContentFilter, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::ContentFilter, + .. + } + )); } #[test] @@ -654,10 +699,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::NotFound, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::NotFound, + .. + } + )); } #[test] @@ -670,10 +718,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::NotFound, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::NotFound, + .. + } + )); } #[test] @@ -686,10 +737,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::Authentication, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::Authentication, + .. + } + )); } #[test] @@ -702,10 +756,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::Authentication, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::Authentication, + .. + } + )); } #[test] @@ -718,10 +775,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::NotFound, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::NotFound, + .. + } + )); let err = error_from_grpc_status( "RESOURCE_EXHAUSTED", @@ -731,10 +791,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::RateLimit, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::RateLimit, + .. + } + )); assert!(err.retryable()); let err = error_from_grpc_status( @@ -745,10 +808,13 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::Authentication, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::Authentication, + .. + } + )); let err = error_from_grpc_status( "DEADLINE_EXCEEDED", @@ -768,16 +834,19 @@ mod tests { None, None, ); - assert!(matches!(err, SdkError::Provider { - kind: ProviderErrorKind::Server, - .. - })); + assert!(matches!( + err, + SdkError::Provider { + kind: ProviderErrorKind::Server, + .. + } + )); } #[test] fn error_display_messages() { let err = SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail { status_code: Some(401), ..ProviderErrorDetail::new("invalid api key", "openai") @@ -790,7 +859,7 @@ mod tests { let err = SdkError::Configuration { message: "no provider".into(), - source: None, + source: None, }; assert_eq!(err.to_string(), "Configuration error: no provider"); } @@ -798,7 +867,7 @@ mod tests { #[test] fn status_code_accessor() { let err = SdkError::Provider { - kind: ProviderErrorKind::Server, + kind: ProviderErrorKind::Server, detail: Box::new(ProviderErrorDetail { status_code: Some(503), ..ProviderErrorDetail::new("error", "openai") @@ -808,7 +877,7 @@ mod tests { let err = SdkError::Network { message: "refused".into(), - source: None, + source: None, }; assert_eq!(err.status_code(), None); } @@ -816,7 +885,7 @@ mod tests { #[test] fn provider_name_from_provider_variant() { let err = SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail::new("bad key", "openai")), }; assert_eq!(err.provider_name(), "openai"); @@ -826,7 +895,7 @@ mod tests { fn provider_name_defaults_to_unknown() { let err = SdkError::Network { message: "refused".into(), - source: None, + source: None, }; assert_eq!(err.provider_name(), "unknown"); } @@ -837,7 +906,7 @@ mod tests { assert!( SdkError::Provider { - kind: ProviderErrorKind::RateLimit, + kind: ProviderErrorKind::RateLimit, detail: detail(), } .failover_eligible() @@ -845,7 +914,7 @@ mod tests { assert!( SdkError::Provider { - kind: ProviderErrorKind::Server, + kind: ProviderErrorKind::Server, detail: detail(), } .failover_eligible() @@ -853,7 +922,7 @@ mod tests { assert!( SdkError::Provider { - kind: ProviderErrorKind::QuotaExceeded, + kind: ProviderErrorKind::QuotaExceeded, detail: detail(), } .failover_eligible() @@ -865,7 +934,7 @@ mod tests { assert!( SdkError::RequestTimeout { message: "timed out".into(), - source: None, + source: None, } .failover_eligible() ); @@ -873,7 +942,7 @@ mod tests { assert!( SdkError::Network { message: "refused".into(), - source: None, + source: None, } .failover_eligible() ); @@ -881,7 +950,7 @@ mod tests { assert!( SdkError::Stream { message: "broken".into(), - source: None, + source: None, } .failover_eligible() ); @@ -893,7 +962,7 @@ mod tests { assert!( !SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: detail(), } .failover_eligible() @@ -901,7 +970,7 @@ mod tests { assert!( !SdkError::Provider { - kind: ProviderErrorKind::InvalidRequest, + kind: ProviderErrorKind::InvalidRequest, detail: detail(), } .failover_eligible() @@ -909,7 +978,7 @@ mod tests { assert!( !SdkError::Provider { - kind: ProviderErrorKind::ContextLength, + kind: ProviderErrorKind::ContextLength, detail: detail(), } .failover_eligible() @@ -917,7 +986,7 @@ mod tests { assert!( !SdkError::Provider { - kind: ProviderErrorKind::ContentFilter, + kind: ProviderErrorKind::ContentFilter, detail: detail(), } .failover_eligible() @@ -929,7 +998,7 @@ mod tests { assert!( !SdkError::Configuration { message: "bad".into(), - source: None, + source: None, } .failover_eligible() ); @@ -966,7 +1035,7 @@ mod tests { #[test] fn failure_signature_hint_provider_transient() { let err = SdkError::Provider { - kind: ProviderErrorKind::RateLimit, + kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail::new("too fast", "openai")), }; assert_eq!( @@ -975,7 +1044,7 @@ mod tests { ); let err = SdkError::Provider { - kind: ProviderErrorKind::Server, + kind: ProviderErrorKind::Server, detail: Box::new(ProviderErrorDetail::new("500", "anthropic")), }; assert_eq!( @@ -987,7 +1056,7 @@ mod tests { #[test] fn failure_signature_hint_provider_deterministic() { let err = SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail::new("bad key", "openai")), }; assert_eq!( @@ -996,7 +1065,7 @@ mod tests { ); let err = SdkError::Provider { - kind: ProviderErrorKind::AccessDenied, + kind: ProviderErrorKind::AccessDenied, detail: Box::new(ProviderErrorDetail::new("denied", "anthropic")), }; assert_eq!( @@ -1005,7 +1074,7 @@ mod tests { ); let err = SdkError::Provider { - kind: ProviderErrorKind::NotFound, + kind: ProviderErrorKind::NotFound, detail: Box::new(ProviderErrorDetail::new("missing", "openai")), }; assert_eq!( @@ -1014,7 +1083,7 @@ mod tests { ); let err = SdkError::Provider { - kind: ProviderErrorKind::InvalidRequest, + kind: ProviderErrorKind::InvalidRequest, detail: Box::new(ProviderErrorDetail::new("bad", "openai")), }; assert_eq!( @@ -1023,7 +1092,7 @@ mod tests { ); let err = SdkError::Provider { - kind: ProviderErrorKind::ContentFilter, + kind: ProviderErrorKind::ContentFilter, detail: Box::new(ProviderErrorDetail::new("blocked", "openai")), }; assert_eq!( @@ -1032,7 +1101,7 @@ mod tests { ); let err = SdkError::Provider { - kind: ProviderErrorKind::ContextLength, + kind: ProviderErrorKind::ContextLength, detail: Box::new(ProviderErrorDetail::new("too long", "openai")), }; assert_eq!( @@ -1041,7 +1110,7 @@ mod tests { ); let err = SdkError::Provider { - kind: ProviderErrorKind::QuotaExceeded, + kind: ProviderErrorKind::QuotaExceeded, detail: Box::new(ProviderErrorDetail::new("out of quota", "openai")), }; assert_eq!( @@ -1055,7 +1124,7 @@ mod tests { assert_eq!( SdkError::RequestTimeout { message: "timed out".into(), - source: None, + source: None, } .failure_signature_hint(), "api_transient|unknown|timeout" @@ -1063,7 +1132,7 @@ mod tests { assert_eq!( SdkError::Network { message: "refused".into(), - source: None, + source: None, } .failure_signature_hint(), "api_transient|unknown|network" @@ -1071,7 +1140,7 @@ mod tests { assert_eq!( SdkError::Stream { message: "broken".into(), - source: None, + source: None, } .failure_signature_hint(), "api_transient|unknown|stream" @@ -1086,7 +1155,7 @@ mod tests { assert_eq!( SdkError::Configuration { message: "bad".into(), - source: None, + source: None, } .failure_signature_hint(), "api_deterministic|unknown|configuration" diff --git a/lib/crates/fabro-llm/src/lib.rs b/lib/crates/fabro-llm/src/lib.rs index 5b68c6974..680229d4b 100644 --- a/lib/crates/fabro-llm/src/lib.rs +++ b/lib/crates/fabro-llm/src/lib.rs @@ -10,5 +10,6 @@ pub mod tools; pub mod types; // Re-export module-level default client helpers (Section 2.5). +pub use error::{Error, ProviderErrorDetail, ProviderErrorKind, Result, SdkError}; pub use fabro_model::{ModelHandle, Provider}; pub use generate::set_default_client; diff --git a/lib/crates/fabro-server/Cargo.toml b/lib/crates/fabro-server/Cargo.toml index 7581c1477..843a14343 100644 --- a/lib/crates/fabro-server/Cargo.toml +++ b/lib/crates/fabro-server/Cargo.toml @@ -75,6 +75,7 @@ regex.workspace = true semver.workspace = true walkdir.workspace = true multer = "3" +thiserror.workspace = true [dev-dependencies] tokio = { workspace = true, features = ["test-util", "macros"] } diff --git a/lib/crates/fabro-server/src/error.rs b/lib/crates/fabro-server/src/error.rs index 84e2a6226..7f93ec53f 100644 --- a/lib/crates/fabro-server/src/error.rs +++ b/lib/crates/fabro-server/src/error.rs @@ -3,10 +3,57 @@ use axum::http::StatusCode; use axum::response::{IntoResponse, Response}; use serde::Serialize; +#[derive(Debug, thiserror::Error)] +pub enum Error { + #[error(transparent)] + Workflow(#[from] fabro_workflow::Error), + + #[error(transparent)] + Agent(#[from] fabro_agent::Error), + + #[error(transparent)] + Llm(#[from] fabro_llm::Error), + + #[error(transparent)] + Store(#[from] fabro_store::Error), + + #[error(transparent)] + Config(#[from] fabro_config::Error), + + #[error(transparent)] + SecretStore(#[from] crate::secret_store::SecretStoreError), + + #[error("bad request: {0}")] + BadRequest(String), + + #[error("authentication required")] + Unauthorized, + + #[error("access denied")] + Forbidden, + + #[error("not found: {0}")] + NotFound(String), + + #[error("conflict: {0}")] + Conflict(String), + + #[error("service unavailable: {0}")] + ServiceUnavailable(String), + + #[error("bad gateway: {0}")] + BadGateway(String), + + #[error("internal server error: {0}")] + Internal(String), +} + +pub type Result = std::result::Result; + #[derive(Serialize)] struct ErrorEntry { status: String, - title: String, + title: String, detail: String, } @@ -49,6 +96,29 @@ impl ApiError { } } +impl From for ApiError { + fn from(err: Error) -> Self { + match err { + Error::BadRequest(msg) => Self::bad_request(msg), + Error::Unauthorized => Self::unauthorized(), + Error::Forbidden => Self::forbidden(), + Error::NotFound(msg) => Self::not_found(msg), + Error::Conflict(msg) => Self::new(StatusCode::CONFLICT, msg), + Error::ServiceUnavailable(msg) => Self::new(StatusCode::SERVICE_UNAVAILABLE, msg), + Error::BadGateway(msg) => Self::new(StatusCode::BAD_GATEWAY, msg), + Error::Workflow(err) => Self::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()), + Error::Agent(err) => Self::new(StatusCode::BAD_GATEWAY, err.to_string()), + Error::Llm(err) => Self::new(StatusCode::BAD_GATEWAY, err.to_string()), + Error::Store(err) => Self::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()), + Error::Config(err) => Self::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()), + Error::SecretStore(err) => { + Self::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()) + } + Error::Internal(msg) => Self::new(StatusCode::INTERNAL_SERVER_ERROR, msg), + } + } +} + impl IntoResponse for ApiError { fn into_response(self) -> Response { let title = self diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index 507c77f2a..8fb8fdf72 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -18,3 +18,5 @@ mod settings_view; pub mod static_files; pub mod tls; pub mod web_auth; + +pub use error::{ApiError, Error, Result}; diff --git a/lib/crates/fabro-server/src/serve.rs b/lib/crates/fabro-server/src/serve.rs index e8fb90f53..4db97e2ef 100644 --- a/lib/crates/fabro-server/src/serve.rs +++ b/lib/crates/fabro-server/src/serve.rs @@ -84,7 +84,7 @@ pub struct ServeArgs { } fn load_settings(path: Option<&Path>) -> anyhow::Result { - load_settings_config(path) + Ok(load_settings_config(path)?) } fn resolved_config_path(path: Option<&Path>) -> PathBuf { @@ -707,15 +707,15 @@ mod tests { fn apply_runtime_settings_preserves_storage_dir() { let base = SettingsLayer::default(); let args = ServeArgs { - bind: None, - model: None, - provider: None, - dry_run: false, - sandbox: None, - web: false, - no_web: false, + bind: None, + model: None, + provider: None, + dry_run: false, + sandbox: None, + web: false, + no_web: false, max_concurrent_runs: None, - config: None, + config: None, }; let resolved = @@ -741,15 +741,15 @@ enabled = false ", ); let args = ServeArgs { - bind: None, - model: None, - provider: None, - dry_run: false, - sandbox: None, - web: true, - no_web: false, + bind: None, + model: None, + provider: None, + dry_run: false, + sandbox: None, + web: true, + no_web: false, max_concurrent_runs: None, - config: None, + config: None, }; let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro")); @@ -768,15 +768,15 @@ enabled = false fn apply_runtime_settings_disables_web_from_cli_flag() { let base = SettingsLayer::default(); let args = ServeArgs { - bind: None, - model: None, - provider: None, - dry_run: false, - sandbox: None, - web: false, - no_web: true, + bind: None, + model: None, + provider: None, + dry_run: false, + sandbox: None, + web: false, + no_web: true, max_concurrent_runs: None, - config: None, + config: None, }; let resolved = apply_runtime_settings(&base, &args, false, &PathBuf::from("/srv/fabro")); diff --git a/lib/crates/fabro-store/src/error.rs b/lib/crates/fabro-store/src/error.rs index 5dccf318f..fd5f4cc3b 100644 --- a/lib/crates/fabro-store/src/error.rs +++ b/lib/crates/fabro-store/src/error.rs @@ -1,7 +1,8 @@ -pub type Result = std::result::Result; +pub type Result = std::result::Result; +pub type StoreError = Error; #[derive(Debug, thiserror::Error)] -pub enum StoreError { +pub enum Error { #[error("SlateDB error: {0}")] Slate(#[from] slatedb::Error), #[error("Object store error: {0}")] diff --git a/lib/crates/fabro-store/src/lib.rs b/lib/crates/fabro-store/src/lib.rs index 79de0fe45..dc6c858ca 100644 --- a/lib/crates/fabro-store/src/lib.rs +++ b/lib/crates/fabro-store/src/lib.rs @@ -8,7 +8,7 @@ mod slate; mod types; pub use artifact_store::{ArtifactStore, NodeArtifact}; -pub use error::{Result, StoreError}; +pub use error::{Error, Result, StoreError}; pub use fabro_types::{RunBlobId, StageId}; pub use run_state::{NodeState, PendingInterviewRecord, RunProjection}; pub use slate::{Database, RunDatabase, Runs}; @@ -17,5 +17,5 @@ pub use types::{EventEnvelope, EventPayload, RunSummary}; #[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct ListRunsQuery { pub start: Option>, - pub end: Option>, + pub end: Option>, } diff --git a/lib/crates/fabro-workflow/src/error.rs b/lib/crates/fabro-workflow/src/error.rs index 70f011fd7..0af240b09 100644 --- a/lib/crates/fabro-workflow/src/error.rs +++ b/lib/crates/fabro-workflow/src/error.rs @@ -1,18 +1,18 @@ -use fabro_graphviz::error::GraphvizError; -use fabro_llm::error::{ProviderErrorKind, SdkError}; +use fabro_graphviz::Error as GraphvizError; +use fabro_llm::{Error as LlmError, ProviderErrorKind}; pub use fabro_types::failure_signature::FailureSignature; pub use fabro_types::outcome::FailureCategory; use fabro_validate::Diagnostic; use serde::{Deserialize, Serialize}; -use thiserror::Error; +use thiserror::Error as ThisError; use crate::outcome::{FailureDetail, Outcome, StageStatus}; -/// Classify an `SdkError` into a `FailureCategory` based on its structure. +/// Classify an LLM error into a `FailureCategory` based on its structure. #[must_use] -pub fn classify_sdk_error(err: &SdkError) -> FailureCategory { +pub fn classify_sdk_error(err: &LlmError) -> FailureCategory { match err { - SdkError::Provider { kind, .. } => match kind { + LlmError::Provider { kind, .. } => match kind { ProviderErrorKind::RateLimit | ProviderErrorKind::Server => { FailureCategory::TransientInfra } @@ -25,14 +25,14 @@ pub fn classify_sdk_error(err: &SdkError) -> FailureCategory { | ProviderErrorKind::InvalidRequest | ProviderErrorKind::ContentFilter => FailureCategory::Deterministic, }, - SdkError::RequestTimeout { .. } | SdkError::Network { .. } | SdkError::Stream { .. } => { + LlmError::RequestTimeout { .. } | LlmError::Network { .. } | LlmError::Stream { .. } => { FailureCategory::TransientInfra } - SdkError::Interrupt { .. } => FailureCategory::Canceled, - SdkError::InvalidToolCall { .. } - | SdkError::NoObjectGenerated { .. } - | SdkError::Configuration { .. } - | SdkError::UnsupportedToolChoice { .. } => FailureCategory::Deterministic, + LlmError::Interrupt { .. } => FailureCategory::Canceled, + LlmError::InvalidToolCall { .. } + | LlmError::NoObjectGenerated { .. } + | LlmError::Configuration { .. } + | LlmError::UnsupportedToolChoice { .. } => FailureCategory::Deterministic, } } @@ -188,9 +188,9 @@ impl FailureSignatureExt for FailureSignature { } } -#[derive(Error, Debug, Clone, Serialize, Deserialize)] +#[derive(ThisError, Debug, Clone, Serialize, Deserialize)] #[serde(tag = "type", content = "data", rename_all = "snake_case")] -pub enum FabroError { +pub enum Error { #[error("Parse error: {0}")] Parse(String), @@ -202,18 +202,18 @@ pub enum FabroError { #[error("Engine error: {message}")] Engine { - message: String, + message: String, failure_class: FailureCategory, }, #[error("Handler error: {message}")] Handler { - message: String, + message: String, failure_class: FailureCategory, }, #[error("LLM error: {0}")] - Llm(SdkError), + Llm(LlmError), #[error("Checkpoint error: {0}")] Checkpoint(String), @@ -231,7 +231,7 @@ pub enum FabroError { Cancelled, } -impl FabroError { +impl Error { /// Smart constructor for Handler errors. Classifies the failure reason /// eagerly. pub fn handler(message: impl Into) -> Self { @@ -308,8 +308,8 @@ impl FabroError { /// Build a fail `Outcome` with structured `FailureDetail`. pub fn to_fail_outcome(&self) -> Outcome { let failure = FailureDetail { - message: self.to_string(), - category: self.failure_category(), + message: self.to_string(), + category: self.failure_category(), signature: self.failure_signature_hint(), }; Outcome { @@ -320,19 +320,19 @@ impl FabroError { } } -impl From for FabroError { +impl From for Error { fn from(err: std::io::Error) -> Self { Self::Io(err.to_string()) } } -impl From for FabroError { - fn from(err: SdkError) -> Self { +impl From for Error { + fn from(err: LlmError) -> Self { Self::Llm(err) } } -impl From for FabroError { +impl From for Error { fn from(e: GraphvizError) -> Self { match e { GraphvizError::Parse(msg) => Self::Parse(msg), @@ -341,19 +341,19 @@ impl From for FabroError { } } -impl From for FabroError { +impl From for Error { fn from(err: fabro_template::TemplateError) -> Self { Self::Validation(err.to_string()) } } -impl From for FabroError { +impl From for Error { fn from(e: fabro_validate::ValidationError) -> Self { Self::Validation(e.0) } } -impl From for FabroError { +impl From for Error { fn from(err: fabro_checkpoint::MetadataError) -> Self { let message = err.to_string(); match err { @@ -366,12 +366,13 @@ impl From for FabroError { } } -pub type Result = std::result::Result; +pub type Result = std::result::Result; +pub type FabroError = Error; #[cfg(test)] mod tests { use fabro_checkpoint::MetadataError; - use fabro_llm::error::ProviderErrorDetail; + use fabro_llm::{Error as SdkError, ProviderErrorDetail}; use super::*; use crate::outcome::OutcomeExt; @@ -392,12 +393,12 @@ mod tests { fn validation_failed_display() { let err = FabroError::ValidationFailed { diagnostics: vec![Diagnostic { - rule: "test".to_string(), + rule: "test".to_string(), severity: fabro_validate::Severity::Error, - message: "missing start node".to_string(), - node_id: None, - edge: None, - fix: None, + message: "missing start node".to_string(), + node_id: None, + edge: None, + fix: None, }], }; assert_eq!(err.to_string(), "Validation failed"); @@ -676,7 +677,7 @@ mod tests { fn llm_error_display() { let sdk_err = SdkError::Network { message: "connection refused".into(), - source: None, + source: None, }; let err = FabroError::Llm(sdk_err); assert_eq!( @@ -689,13 +690,13 @@ mod tests { fn llm_error_retryable_delegates_to_sdk() { let retryable = FabroError::Llm(SdkError::Network { message: "timeout".into(), - source: None, + source: None, }); assert!(retryable.is_retryable()); let non_retryable = FabroError::Llm(SdkError::Configuration { message: "bad config".into(), - source: None, + source: None, }); assert!(!non_retryable.is_retryable()); } @@ -704,7 +705,7 @@ mod tests { fn llm_error_from_sdk_error() { let sdk_err = SdkError::Stream { message: "broken pipe".into(), - source: None, + source: None, }; let err = FabroError::from(sdk_err); assert!(matches!(err, FabroError::Llm(_))); @@ -755,7 +756,7 @@ mod tests { #[test] fn failure_class_llm_rate_limit() { let err = FabroError::Llm(SdkError::Provider { - kind: ProviderErrorKind::RateLimit, + kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail::new("too fast", "openai")), }); assert_eq!(err.failure_category(), FailureCategory::TransientInfra); @@ -764,7 +765,7 @@ mod tests { #[test] fn failure_class_llm_context_length() { let err = FabroError::Llm(SdkError::Provider { - kind: ProviderErrorKind::ContextLength, + kind: ProviderErrorKind::ContextLength, detail: Box::new(ProviderErrorDetail::new("too long", "openai")), }); assert_eq!(err.failure_category(), FailureCategory::BudgetExhausted); @@ -773,7 +774,7 @@ mod tests { #[test] fn failure_class_llm_auth() { let err = FabroError::Llm(SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail::new("bad key", "openai")), }); assert_eq!(err.failure_category(), FailureCategory::Deterministic); @@ -791,7 +792,7 @@ mod tests { fn failure_class_llm_timeout() { let err = FabroError::Llm(SdkError::RequestTimeout { message: "timed out".into(), - source: None, + source: None, }); assert_eq!(err.failure_category(), FailureCategory::TransientInfra); } @@ -801,7 +802,7 @@ mod tests { #[test] fn classify_sdk_rate_limit() { let err = SdkError::Provider { - kind: ProviderErrorKind::RateLimit, + kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail::new("too fast", "openai")), }; assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); @@ -810,7 +811,7 @@ mod tests { #[test] fn classify_sdk_server() { let err = SdkError::Provider { - kind: ProviderErrorKind::Server, + kind: ProviderErrorKind::Server, detail: Box::new(ProviderErrorDetail::new("500", "openai")), }; assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); @@ -819,7 +820,7 @@ mod tests { #[test] fn classify_sdk_context_length() { let err = SdkError::Provider { - kind: ProviderErrorKind::ContextLength, + kind: ProviderErrorKind::ContextLength, detail: Box::new(ProviderErrorDetail::new("too long", "openai")), }; assert_eq!(classify_sdk_error(&err), FailureCategory::BudgetExhausted); @@ -828,7 +829,7 @@ mod tests { #[test] fn classify_sdk_quota_exceeded() { let err = SdkError::Provider { - kind: ProviderErrorKind::QuotaExceeded, + kind: ProviderErrorKind::QuotaExceeded, detail: Box::new(ProviderErrorDetail::new("out of quota", "openai")), }; assert_eq!(classify_sdk_error(&err), FailureCategory::BudgetExhausted); @@ -837,7 +838,7 @@ mod tests { #[test] fn classify_sdk_auth() { let err = SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail::new("bad key", "openai")), }; assert_eq!(classify_sdk_error(&err), FailureCategory::Deterministic); @@ -847,7 +848,7 @@ mod tests { fn classify_sdk_request_timeout() { let err = SdkError::RequestTimeout { message: "timed out".into(), - source: None, + source: None, }; assert_eq!(classify_sdk_error(&err), FailureCategory::TransientInfra); } @@ -1492,7 +1493,7 @@ mod tests { #[test] fn failure_signature_hint_llm_returns_some() { let err = FabroError::Llm(SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail::new("bad key", "openai")), }); assert_eq!( @@ -1518,7 +1519,7 @@ mod tests { #[test] fn to_fail_outcome_llm_has_class_and_signature() { let err = FabroError::Llm(SdkError::Provider { - kind: ProviderErrorKind::Authentication, + kind: ProviderErrorKind::Authentication, detail: Box::new(ProviderErrorDetail::new("bad key", "openai")), }); let outcome = err.to_fail_outcome(); @@ -1545,7 +1546,7 @@ mod tests { fn to_fail_outcome_includes_error_message_as_reason() { let err = FabroError::Llm(SdkError::Network { message: "connection refused".into(), - source: None, + source: None, }); let outcome = err.to_fail_outcome(); assert!( @@ -1560,7 +1561,7 @@ mod tests { fn to_fail_outcome_no_context_updates() { let err = FabroError::Llm(SdkError::Network { message: "refused".into(), - source: None, + source: None, }); let outcome = err.to_fail_outcome(); assert!(outcome.context_updates.is_empty()); @@ -1604,19 +1605,19 @@ mod tests { FabroError::Validation("bad".into()), FabroError::ValidationFailed { diagnostics: vec![Diagnostic { - rule: "test".into(), + rule: "test".into(), severity: fabro_validate::Severity::Error, - message: "bad".into(), - node_id: None, - edge: None, - fix: None, + message: "bad".into(), + node_id: None, + edge: None, + fix: None, }], }, FabroError::engine("engine err"), FabroError::handler("handler err"), FabroError::Llm(SdkError::Network { message: "refused".into(), - source: None, + source: None, }), FabroError::Checkpoint("cp err".into()), FabroError::Stylesheet("style err".into()), @@ -1684,7 +1685,7 @@ mod tests { // 1. Create SdkError → FabroError let sdk_err = SdkError::Provider { - kind: ProviderErrorKind::RateLimit, + kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail::new("too fast", "openai")), }; let arc_err = FabroError::Llm(sdk_err); @@ -1700,10 +1701,10 @@ mod tests { // 3. Outcome → StageFailed event let failure = outcome.failure.clone().unwrap(); let event = Event::StageFailed { - node_id: "code".into(), - name: "code".into(), - index: 0, - failure: failure.clone(), + node_id: "code".into(), + name: "code".into(), + index: 0, + failure: failure.clone(), will_retry: false, }; @@ -1766,10 +1767,10 @@ mod tests { #[test] fn e2e_serde_stability_agent_error() { - use fabro_agent::error::AgentError; + use fabro_agent::Error as AgentError; let err = AgentError::Llm(SdkError::Provider { - kind: ProviderErrorKind::RateLimit, + kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail::new("too fast", "openai")), }); let json = serde_json::to_string(&err).unwrap(); diff --git a/lib/crates/fabro-workflow/src/lib.rs b/lib/crates/fabro-workflow/src/lib.rs index a7b634778..ab563a789 100644 --- a/lib/crates/fabro-workflow/src/lib.rs +++ b/lib/crates/fabro-workflow/src/lib.rs @@ -72,15 +72,15 @@ pub fn build_completed_stages(cp: &records::Checkpoint, run_failed: bool) -> Vec last.failed = true; } else { stages.push(CompletedStage { - node_id: "unknown".to_string(), - status: "fail".to_string(), - succeeded: false, - failed: true, - retries: 0, + node_id: "unknown".to_string(), + status: "fail".to_string(), + succeeded: false, + failed: true, + retries: 0, billing_usd_micros: None, - notes: None, - failure_reason: None, - files_touched: vec![], + notes: None, + failure_reason: None, + files_touched: vec![], }); } } @@ -138,6 +138,10 @@ pub mod run_control; pub(crate) mod run_dir; pub mod run_dump; pub mod run_lookup; + +pub use error::{ + Error, FabroError, FailureCategory, FailureSignature, FailureSignatureExt, Result, +}; pub mod run_materialization; pub mod run_options; pub mod run_status;