From b91ae3df472d0a9ef9d49657a3b5cd58bd4a744e Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 23 Mar 2026 15:38:00 -0400 Subject: [PATCH] Add #[source] error chaining to SdkError and tool_type field to ToolCall MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preserve original error chains (reqwest, serde_json, etc.) in SdkError via Arc-backed #[source] fields on Network, RequestTimeout, Stream, and Configuration variants. This makes production debugging of network/TLS/DNS issues easier since error reporters can now walk the full chain. Serde-compatible via #[serde(skip)] — message string still carries the text for serialized forms. Also add a `type` field to ToolCall (defaulting to "function") so non-function tool types from providers won't be silently mishandled. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/reference/sdk.mdx | 4 +- lib/crates/fabro-agent/src/error.rs | 4 + lib/crates/fabro-agent/src/session.rs | 6 +- lib/crates/fabro-agent/src/tool_execution.rs | 1 + lib/crates/fabro-agent/src/types.rs | 1 + lib/crates/fabro-llm/src/client.rs | 2 + lib/crates/fabro-llm/src/error.rs | 130 ++++++++++++++++-- lib/crates/fabro-llm/src/generate.rs | 10 +- .../fabro-llm/src/providers/anthropic.rs | 29 ++-- lib/crates/fabro-llm/src/providers/common.rs | 20 ++- .../fabro-llm/src/providers/fabro_server.rs | 32 ++--- lib/crates/fabro-llm/src/providers/gemini.rs | 41 +++--- lib/crates/fabro-llm/src/providers/openai.rs | 17 ++- .../src/providers/openai_compatible.rs | 23 ++-- lib/crates/fabro-llm/src/types.rs | 23 ++++ lib/crates/fabro-workflows/src/error.rs | 9 ++ lib/crates/fabro-workflows/src/event.rs | 1 + 17 files changed, 254 insertions(+), 99 deletions(-) diff --git a/docs/reference/sdk.mdx b/docs/reference/sdk.mdx index d2afde968..a38a9ebed 100644 --- a/docs/reference/sdk.mdx +++ b/docs/reference/sdk.mdx @@ -755,8 +755,8 @@ match result { println!("HTTP {code}"); } } - Err(SdkError::RequestTimeout { message }) => println!("Timeout: {message}"), - Err(SdkError::Network { message }) => println!("Network: {message}"), + Err(SdkError::RequestTimeout { message, .. }) => println!("Timeout: {message}"), + Err(SdkError::Network { message, .. }) => println!("Network: {message}"), Err(SdkError::Abort { message }) => println!("Cancelled: {message}"), Err(e) => println!("Other: {e}"), Ok(_) => {} diff --git a/lib/crates/fabro-agent/src/error.rs b/lib/crates/fabro-agent/src/error.rs index 149997b71..24f2b72d6 100644 --- a/lib/crates/fabro-agent/src/error.rs +++ b/lib/crates/fabro-agent/src/error.rs @@ -45,6 +45,7 @@ mod tests { fn agent_error_from_sdk_error() { let sdk_err = SdkError::Network { message: "connection refused".into(), + source: None, }; let agent_err = AgentError::from(sdk_err); assert!(matches!(agent_err, AgentError::Llm(_))); @@ -87,6 +88,7 @@ mod tests { fn serde_roundtrip_llm_network() { let err = AgentError::Llm(SdkError::Network { message: "connection refused".into(), + source: None, }); let json = serde_json::to_string(&err).unwrap(); let deserialized: AgentError = serde_json::from_str(&json).unwrap(); @@ -150,6 +152,7 @@ mod tests { let errors: Vec = vec![ AgentError::Llm(SdkError::Network { message: "refused".into(), + source: None, }), AgentError::SessionClosed, AgentError::InvalidState("reason".into()), @@ -167,6 +170,7 @@ mod tests { fn serde_tag_format_llm() { let err = AgentError::Llm(SdkError::Network { message: "refused".into(), + source: None, }); let json = serde_json::to_string(&err).unwrap(); let v: serde_json::Value = serde_json::from_str(&json).unwrap(); diff --git a/lib/crates/fabro-agent/src/session.rs b/lib/crates/fabro-agent/src/session.rs index 6915e9736..1b6b18316 100644 --- a/lib/crates/fabro-agent/src/session.rs +++ b/lib/crates/fabro-agent/src/session.rs @@ -770,6 +770,7 @@ impl Session { None => { return Err(self.emit_llm_error(SdkError::Stream { message: "Stream ended without a Finish event (after retries)".into(), + source: None, })) } }; @@ -955,7 +956,7 @@ impl Session { .and_then(fabro_model::Model::max_output) }), stop_sequences: None, - reasoning_effort: self.config.reasoning_effort.clone(), + reasoning_effort: self.config.reasoning_effort, speed: self.config.speed.clone(), metadata: None, provider_options: None, @@ -1038,6 +1039,7 @@ mod tests { async fn complete(&self, _request: &Request) -> Result { Err(SdkError::Configuration { message: "ScriptedStreamProvider does not implement complete()".into(), + source: None, }) } @@ -2032,6 +2034,7 @@ mod tests { partial_text: "partial".into(), error: SdkError::Stream { message: "connection reset".into(), + source: None, }, }); let client = make_client(provider as Arc).await; @@ -2293,6 +2296,7 @@ mod tests { async fn complete(&self, _request: &Request) -> Result { Err(SdkError::Stream { message: "summarization failed".into(), + source: None, }) } diff --git a/lib/crates/fabro-agent/src/tool_execution.rs b/lib/crates/fabro-agent/src/tool_execution.rs index 52363ceca..1acb8c53e 100644 --- a/lib/crates/fabro-agent/src/tool_execution.rs +++ b/lib/crates/fabro-agent/src/tool_execution.rs @@ -389,6 +389,7 @@ mod tests { ToolCall { id: id.to_string(), name: name.to_string(), + tool_type: "function".to_string(), arguments: args, raw_arguments: None, provider_metadata: None, diff --git a/lib/crates/fabro-agent/src/types.rs b/lib/crates/fabro-agent/src/types.rs index d92e9f1f7..4b9ad64a1 100644 --- a/lib/crates/fabro-agent/src/types.rs +++ b/lib/crates/fabro-agent/src/types.rs @@ -671,6 +671,7 @@ mod tests { let event = AgentEvent::Error { error: crate::error::AgentError::Llm(fabro_llm::error::SdkError::Network { message: "refused".into(), + source: None, }), }; let json = serde_json::to_string(&event).unwrap(); diff --git a/lib/crates/fabro-llm/src/client.rs b/lib/crates/fabro-llm/src/client.rs index ebe62d44a..6399cfd74 100644 --- a/lib/crates/fabro-llm/src/client.rs +++ b/lib/crates/fabro-llm/src/client.rs @@ -154,6 +154,7 @@ impl Client { .or(self.default_provider.as_deref()) .ok_or_else(|| SdkError::Configuration { message: "No provider specified and no default provider set".into(), + source: None, })?; self.providers @@ -161,6 +162,7 @@ impl Client { .cloned() .ok_or_else(|| SdkError::Configuration { message: format!("Provider '{provider_name}' not registered"), + source: None, }) } diff --git a/lib/crates/fabro-llm/src/error.rs b/lib/crates/fabro-llm/src/error.rs index e1a54dd65..612f314f9 100644 --- a/lib/crates/fabro-llm/src/error.rs +++ b/lib/crates/fabro-llm/src/error.rs @@ -51,6 +51,8 @@ impl ProviderErrorDetail { } } +use std::sync::Arc; + #[derive(Debug, Clone, serde::Serialize, serde::Deserialize, thiserror::Error)] #[serde(tag = "type", rename_all = "snake_case")] pub enum SdkError { @@ -61,16 +63,31 @@ pub enum SdkError { }, #[error("Request timed out: {message}")] - RequestTimeout { message: String }, + RequestTimeout { + message: String, + #[source] + #[serde(skip)] + source: Option>, + }, #[error("Request aborted: {message}")] Abort { message: String }, #[error("Network error: {message}")] - Network { message: String }, + Network { + message: String, + #[source] + #[serde(skip)] + source: Option>, + }, #[error("Stream error: {message}")] - Stream { message: String }, + Stream { + message: String, + #[source] + #[serde(skip)] + source: Option>, + }, #[error("Invalid tool call: {message}")] InvalidToolCall { message: String }, @@ -79,13 +96,58 @@ pub enum SdkError { NoObjectGenerated { message: String }, #[error("Configuration error: {message}")] - Configuration { message: String }, + Configuration { + message: String, + #[source] + #[serde(skip)] + source: Option>, + }, #[error("Unsupported tool choice: {message}")] UnsupportedToolChoice { message: String }, } impl SdkError { + 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)), + } + } + + pub fn request_timeout( + message: impl Into, + source: impl std::error::Error + Send + Sync + 'static, + ) -> Self { + Self::RequestTimeout { + message: message.into(), + source: Some(Arc::new(source)), + } + } + + pub fn stream_error( + message: impl Into, + source: impl std::error::Error + Send + Sync + 'static, + ) -> Self { + Self::Stream { + message: message.into(), + source: Some(Arc::new(source)), + } + } + + pub fn configuration_error( + message: impl Into, + source: impl std::error::Error + Send + Sync + 'static, + ) -> Self { + Self::Configuration { + message: message.into(), + source: Some(Arc::new(source)), + } + } + #[must_use] pub const fn retryable(&self) -> bool { match self { @@ -228,6 +290,7 @@ pub fn error_from_status_code( 408 => { return SdkError::RequestTimeout { message: detail.message, + source: None, } } 413 => ProviderErrorKind::ContextLength, @@ -285,6 +348,7 @@ pub fn error_from_grpc_status( "DEADLINE_EXCEEDED" => { return SdkError::RequestTimeout { message: detail.message, + source: None, } } _ => ProviderErrorKind::Server, @@ -299,6 +363,7 @@ pub fn error_from_grpc_status( #[cfg(test)] mod tests { use super::*; + use std::error::Error as _; #[test] fn retryable_classification() { @@ -333,16 +398,19 @@ mod tests { let timeout = SdkError::RequestTimeout { message: "timed out".into(), + source: None, }; assert!(!timeout.retryable()); let network = SdkError::Network { message: "connection refused".into(), + source: None, }; assert!(network.retryable()); let config = SdkError::Configuration { message: "missing provider".into(), + source: None, }; assert!(!config.retryable()); } @@ -786,6 +854,7 @@ mod tests { let err = SdkError::Configuration { message: "no provider".into(), + source: None, }; assert_eq!(err.to_string(), "Configuration error: no provider"); } @@ -803,6 +872,7 @@ mod tests { let err = SdkError::Network { message: "refused".into(), + source: None, }; assert_eq!(err.status_code(), None); } @@ -820,6 +890,7 @@ mod tests { fn provider_name_defaults_to_unknown() { let err = SdkError::Network { message: "refused".into(), + source: None, }; assert_eq!(err.provider_name(), "unknown"); } @@ -850,17 +921,20 @@ mod tests { #[test] fn failover_eligible_transient_non_provider_errors() { assert!(SdkError::RequestTimeout { - message: "timed out".into() + message: "timed out".into(), + source: None, } .failover_eligible()); assert!(SdkError::Network { - message: "refused".into() + message: "refused".into(), + source: None, } .failover_eligible()); assert!(SdkError::Stream { - message: "broken".into() + message: "broken".into(), + source: None, } .failover_eligible()); } @@ -897,7 +971,8 @@ mod tests { #[test] fn failover_not_eligible_non_provider_errors() { assert!(!SdkError::Configuration { - message: "bad".into() + message: "bad".into(), + source: None, } .failover_eligible()); @@ -1013,21 +1088,24 @@ mod tests { fn failure_signature_hint_non_provider_variants() { assert_eq!( SdkError::RequestTimeout { - message: "timed out".into() + message: "timed out".into(), + source: None, } .failure_signature_hint(), "api_transient|unknown|timeout" ); assert_eq!( SdkError::Network { - message: "refused".into() + message: "refused".into(), + source: None, } .failure_signature_hint(), "api_transient|unknown|network" ); assert_eq!( SdkError::Stream { - message: "broken".into() + message: "broken".into(), + source: None, } .failure_signature_hint(), "api_transient|unknown|stream" @@ -1041,7 +1119,8 @@ mod tests { ); assert_eq!( SdkError::Configuration { - message: "bad".into() + message: "bad".into(), + source: None, } .failure_signature_hint(), "api_deterministic|unknown|configuration" @@ -1068,4 +1147,31 @@ mod tests { "api_deterministic|unknown|unsupported_tool_choice" ); } + + #[test] + fn sdk_error_source_chaining() { + let io_err = std::io::Error::new(std::io::ErrorKind::ConnectionRefused, "refused"); + let err = SdkError::network("connection failed", io_err); + assert!(err.source().is_some()); + } + + #[test] + fn sdk_error_source_chain_walkable() { + let io_err = std::io::Error::new(std::io::ErrorKind::ConnectionRefused, "refused"); + let err = SdkError::network("connection failed", io_err); + // The source chain is walkable — the Arc wrapper preserves the inner error's display + let source = err.source().unwrap(); + assert!(source.to_string().contains("refused")); + } + + #[test] + fn sdk_error_serde_roundtrip_without_source() { + let io_err = std::io::Error::new(std::io::ErrorKind::Other, "boom"); + let err = SdkError::network("network failed", io_err); + let json = serde_json::to_string(&err).unwrap(); + let deserialized: SdkError = serde_json::from_str(&json).unwrap(); + // source is lost through serde, message is preserved + assert!(deserialized.source().is_none()); + assert_eq!(deserialized.to_string(), "Network error: network failed"); + } } diff --git a/lib/crates/fabro-llm/src/generate.rs b/lib/crates/fabro-llm/src/generate.rs index 719781deb..f868a7d4c 100644 --- a/lib/crates/fabro-llm/src/generate.rs +++ b/lib/crates/fabro-llm/src/generate.rs @@ -43,6 +43,7 @@ fn build_initial_messages(params: &GenerateParams) -> Result, SdkEr if params.messages.is_some() { return Err(SdkError::Configuration { message: "Cannot specify both 'prompt' and 'messages'".into(), + source: None, }); } messages.push(Message::user(prompt)); @@ -68,7 +69,7 @@ fn build_request( top_p: params.top_p, max_tokens: params.max_tokens, stop_sequences: params.stop_sequences.clone(), - reasoning_effort: params.reasoning_effort.clone(), + reasoning_effort: params.reasoning_effort, speed: params.speed.clone(), metadata: params.metadata.clone(), provider_options: params.provider_options.clone(), @@ -165,6 +166,7 @@ pub async fn generate(params: GenerateParams) -> Result Result Result Result Some(( Err(SdkError::RequestTimeout { message: format!("Total timeout of {total_copy}s exceeded"), + source: None, }), (stream, true), )), @@ -1081,6 +1088,7 @@ pub async fn stream_object( Err(e) => { events.push(Err(SdkError::Stream { message: format!("{e}"), + source: None, })); } } diff --git a/lib/crates/fabro-llm/src/providers/anthropic.rs b/lib/crates/fabro-llm/src/providers/anthropic.rs index 25bd0cc37..d278009db 100644 --- a/lib/crates/fabro-llm/src/providers/anthropic.rs +++ b/lib/crates/fabro-llm/src/providers/anthropic.rs @@ -75,6 +75,7 @@ impl Adapter { response.ok_or_else(|| SdkError::Stream { message: "complete_via_stream: stream ended without a Finish event".to_string(), + source: None, }) } } @@ -1230,8 +1231,11 @@ impl ProviderAdapter for Adapter { } let (body, headers) = send_and_read_response(req, &self.provider_name, "type").await?; - let api_resp: ApiResponse = serde_json::from_str(&body).map_err(|e| SdkError::Network { - message: format!("failed to parse {} response: {e}", self.provider_name), + let api_resp: ApiResponse = serde_json::from_str(&body).map_err(|e| { + SdkError::network( + format!("failed to parse {} response: {e}", self.provider_name), + e, + ) })?; let content_parts: Vec = api_resp @@ -1290,16 +1294,18 @@ impl ProviderAdapter for Adapter { } let (_api_request, req_builder) = build_api_request(self, request, true); - let http_resp = req_builder.send().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let http_resp = req_builder + .send() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; let status = http_resp.status(); if !status.is_success() { let retry_after = parse_retry_after(http_resp.headers()); - let body = http_resp.text().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let body = http_resp + .text() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; let (msg, code, raw) = parse_error_body(&body, "type"); return Err(crate::error::error_from_status_code( status.as_u16(), @@ -1336,9 +1342,10 @@ impl ProviderAdapter for Adapter { Ok(v) => v, Err(e) => { return Some(( - Err(SdkError::Stream { - message: format!("failed to parse SSE data: {e}"), - }), + Err(SdkError::stream_error( + format!("failed to parse SSE data: {e}"), + e, + )), state, )); } diff --git a/lib/crates/fabro-llm/src/providers/common.rs b/lib/crates/fabro-llm/src/providers/common.rs index 9d0fa1e97..e812dd0bf 100644 --- a/lib/crates/fabro-llm/src/providers/common.rs +++ b/lib/crates/fabro-llm/src/providers/common.rs @@ -170,23 +170,20 @@ pub async fn send_and_read_response( let http_resp = request.send().await.map_err(|e| { if e.is_timeout() { warn!(provider = %provider, error = %e, "Provider request timed out"); - SdkError::RequestTimeout { - message: format!("{provider}: {e}"), - } + SdkError::request_timeout(format!("{provider}: {e}"), e) } else { warn!(provider = %provider, error = %e, "Provider network error"); - SdkError::Network { - message: e.to_string(), - } + SdkError::network(e.to_string(), e) } })?; let status = http_resp.status(); let retry_after = parse_retry_after(http_resp.headers()); let headers = http_resp.headers().clone(); - let body = http_resp.text().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let body = http_resp + .text() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; if !status.is_success() { warn!(provider = %provider, status = status.as_u16(), "Provider returned error"); @@ -258,14 +255,13 @@ impl LineReader { return Ok(Some(remaining)); } Ok(Err(e)) => { - return Err(SdkError::Stream { - message: e.to_string(), - }); + return Err(SdkError::stream_error(e.to_string(), e)); } Err(_) => { warn!("Stream read timed out waiting for next event"); return Err(SdkError::Stream { message: "stream read timed out waiting for next event".to_string(), + source: None, }); } } diff --git a/lib/crates/fabro-llm/src/providers/fabro_server.rs b/lib/crates/fabro-llm/src/providers/fabro_server.rs index 604176f8b..feffba837 100644 --- a/lib/crates/fabro-llm/src/providers/fabro_server.rs +++ b/lib/crates/fabro-llm/src/providers/fabro_server.rs @@ -62,8 +62,8 @@ fn map_stop_reason(reason: &str) -> FinishReason { /// Build the JSON request body by serializing the `Request` and injecting /// the `stream` flag. fn build_body(request: &Request, stream: bool) -> Result { - let mut body = serde_json::to_value(request).map_err(|e| SdkError::Configuration { - message: format!("failed to serialize request: {e}"), + let mut body = serde_json::to_value(request).map_err(|e| { + SdkError::configuration_error(format!("failed to serialize request: {e}"), e) })?; body["stream"] = serde_json::Value::Bool(stream); Ok(body) @@ -80,13 +80,9 @@ async fn send_request( ) -> Result { let http_resp = client.post(url).json(body).send().await.map_err(|e| { if e.is_timeout() { - SdkError::RequestTimeout { - message: e.to_string(), - } + SdkError::request_timeout(e.to_string(), e) } else { - SdkError::Network { - message: e.to_string(), - } + SdkError::network(e.to_string(), e) } })?; @@ -127,13 +123,14 @@ impl ProviderAdapter for Adapter { let body = build_body(request, false)?; let http_resp = send_request(&self.client, &url, &body, &self.provider_name).await?; - let resp_body = http_resp.text().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let resp_body = http_resp + .text() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; let server_resp: ServerCompletionResponse = - serde_json::from_str(&resp_body).map_err(|e| SdkError::Stream { - message: format!("failed to parse completion response: {e}"), + serde_json::from_str(&resp_body).map_err(|e| { + SdkError::stream_error(format!("failed to parse completion response: {e}"), e) })?; let finish_reason = map_stop_reason(&server_resp.stop_reason); @@ -175,11 +172,10 @@ impl ProviderAdapter for Adapter { Ok(event) => return Some((Ok(event), reader)), Err(e) => { return Some(( - Err(SdkError::Stream { - message: format!( - "failed to parse stream event: {e}" - ), - }), + Err(SdkError::stream_error( + format!("failed to parse stream event: {e}"), + e, + )), reader, )); } diff --git a/lib/crates/fabro-llm/src/providers/gemini.rs b/lib/crates/fabro-llm/src/providers/gemini.rs index 377c3fab2..ad2a6f466 100644 --- a/lib/crates/fabro-llm/src/providers/gemini.rs +++ b/lib/crates/fabro-llm/src/providers/gemini.rs @@ -508,22 +508,19 @@ async fn send_gemini_response( ) -> Result<(String, reqwest::header::HeaderMap), SdkError> { let http_resp = request.send().await.map_err(|e| { if e.is_timeout() { - SdkError::RequestTimeout { - message: format!("gemini: {e}"), - } + SdkError::request_timeout(format!("gemini: {e}"), e) } else { - SdkError::Network { - message: e.to_string(), - } + SdkError::network(e.to_string(), e) } })?; let status = http_resp.status(); let retry_after = parse_retry_after(http_resp.headers()); let headers = http_resp.headers().clone(); - let body = http_resp.text().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let body = http_resp + .text() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; if !status.is_success() { let (msg, code, raw) = parse_error_body(&body, "status"); @@ -568,16 +565,18 @@ fn gemini_error( async fn send_streaming_request( request: reqwest::RequestBuilder, ) -> Result { - let http_resp = request.send().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let http_resp = request + .send() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; let status = http_resp.status(); if !status.is_success() { let retry_after = parse_retry_after(http_resp.headers()); - let body = http_resp.text().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let body = http_resp + .text() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; let (msg, code, raw) = parse_error_body(&body, "status"); return Err(gemini_error(status.as_u16(), msg, code, raw, retry_after)); } @@ -635,9 +634,10 @@ fn process_sse_stream( Ok(v) => v, Err(e) => { return Some(( - Err(SdkError::Stream { - message: format!("failed to parse Gemini SSE chunk: {e}"), - }), + Err(SdkError::stream_error( + format!("failed to parse Gemini SSE chunk: {e}"), + e, + )), state, )); } @@ -903,9 +903,8 @@ impl ProviderAdapter for Adapter { } let (body, headers) = send_gemini_response(gemini_req).await?; - let api_resp: ApiResponse = serde_json::from_str(&body).map_err(|e| SdkError::Network { - message: format!("failed to parse Gemini response: {e}"), - })?; + let api_resp: ApiResponse = serde_json::from_str(&body) + .map_err(|e| SdkError::network(format!("failed to parse Gemini response: {e}"), e))?; let candidate = api_resp .candidates diff --git a/lib/crates/fabro-llm/src/providers/openai.rs b/lib/crates/fabro-llm/src/providers/openai.rs index 04ecd9eff..a62edab3d 100644 --- a/lib/crates/fabro-llm/src/providers/openai.rs +++ b/lib/crates/fabro-llm/src/providers/openai.rs @@ -107,6 +107,7 @@ impl Adapter { } last_response.ok_or_else(|| SdkError::Network { message: "Stream ended without a finish event".into(), + source: None, }) } } @@ -954,9 +955,8 @@ impl ProviderAdapter for Adapter { } let (body, headers) = send_and_read_response(req, "openai", "type").await?; - let api_resp: ApiResponse = serde_json::from_str(&body).map_err(|e| SdkError::Network { - message: format!("failed to parse OpenAI response: {e}"), - })?; + let api_resp: ApiResponse = serde_json::from_str(&body) + .map_err(|e| SdkError::network(format!("failed to parse OpenAI response: {e}"), e))?; let (content_parts, has_tool_calls) = parse_output(&api_resp.output); let finish_reason = map_finish_reason(api_resp.status.as_deref(), has_tool_calls); @@ -1009,16 +1009,15 @@ impl ProviderAdapter for Adapter { .json(&request_body) .send() .await - .map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + .map_err(|e| SdkError::network(e.to_string(), e))?; let status = http_resp.status(); if !status.is_success() { let retry_after = parse_retry_after(http_resp.headers()); - let body = http_resp.text().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let body = http_resp + .text() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; let (msg, code, raw) = parse_error_body(&body, "type"); return Err(crate::error::error_from_status_code( status.as_u16(), diff --git a/lib/crates/fabro-llm/src/providers/openai_compatible.rs b/lib/crates/fabro-llm/src/providers/openai_compatible.rs index b54ed041a..7cb39babe 100644 --- a/lib/crates/fabro-llm/src/providers/openai_compatible.rs +++ b/lib/crates/fabro-llm/src/providers/openai_compatible.rs @@ -462,9 +462,8 @@ impl ProviderAdapter for Adapter { } let (body, headers) = send_and_read_response(req, &self.provider_name, "type").await?; - let api_resp: ApiResponse = serde_json::from_str(&body).map_err(|e| SdkError::Network { - message: format!("failed to parse response: {e}"), - })?; + let api_resp: ApiResponse = serde_json::from_str(&body) + .map_err(|e| SdkError::network(format!("failed to parse response: {e}"), e))?; let choice = api_resp.choices.first().ok_or_else(|| SdkError::Provider { kind: ProviderErrorKind::Server, @@ -541,16 +540,15 @@ impl ProviderAdapter for Adapter { .json(&api_body) .send() .await - .map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + .map_err(|e| SdkError::network(e.to_string(), e))?; let status = http_resp.status(); if !status.is_success() { let retry_after = parse_retry_after(http_resp.headers()); - let body = http_resp.text().await.map_err(|e| SdkError::Network { - message: e.to_string(), - })?; + let body = http_resp + .text() + .await + .map_err(|e| SdkError::network(e.to_string(), e))?; let (msg, code, raw) = parse_error_body(&body, "type"); return Err(error_from_status_code( status.as_u16(), @@ -614,9 +612,10 @@ impl ProviderAdapter for Adapter { Ok(c) => c, Err(e) => { return Some(( - Err(SdkError::Stream { - message: format!("failed to parse SSE chunk: {e}"), - }), + Err(SdkError::stream_error( + format!("failed to parse SSE chunk: {e}"), + e, + )), state, )); } diff --git a/lib/crates/fabro-llm/src/types.rs b/lib/crates/fabro-llm/src/types.rs index dd1aebf17..9c161c111 100644 --- a/lib/crates/fabro-llm/src/types.rs +++ b/lib/crates/fabro-llm/src/types.rs @@ -49,10 +49,16 @@ pub struct ThinkingData { // --- 5.4 ToolCall / ToolResult --- +fn default_tool_type() -> String { + "function".to_string() +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ToolCall { pub id: String, pub name: String, + #[serde(rename = "type", default = "default_tool_type")] + pub tool_type: String, pub arguments: serde_json::Value, pub raw_arguments: Option, /// Opaque provider-specific metadata (e.g. Gemini `thought_signature`). @@ -71,6 +77,7 @@ impl ToolCall { Self { id: id.into(), name: name.into(), + tool_type: "function".to_string(), arguments, raw_arguments: None, provider_metadata: None, @@ -1158,6 +1165,7 @@ mod tests { fn stream_event_error() { let event = StreamEvent::error(SdkError::Stream { message: "something went wrong".into(), + source: None, }); match &event { StreamEvent::Error { error, .. } => { @@ -1263,9 +1271,24 @@ mod tests { let tc = ToolCall::new("c1", "test", serde_json::json!({})); assert_eq!(tc.id, "c1"); assert_eq!(tc.name, "test"); + assert_eq!(tc.tool_type, "function"); assert_eq!(tc.raw_arguments, None); } + #[test] + fn tool_call_deserialize_without_type_defaults_to_function() { + let json = r#"{"id":"c1","name":"test","arguments":{}}"#; + let tc: ToolCall = serde_json::from_str(json).unwrap(); + assert_eq!(tc.tool_type, "function"); + } + + #[test] + fn tool_call_serializes_type_field() { + let tc = ToolCall::new("c1", "test", serde_json::json!({})); + let json = serde_json::to_value(&tc).unwrap(); + assert_eq!(json["type"], "function"); + } + #[test] fn stream_event_step_finish_constructor() { let response = Response { diff --git a/lib/crates/fabro-workflows/src/error.rs b/lib/crates/fabro-workflows/src/error.rs index b649205f3..101874166 100644 --- a/lib/crates/fabro-workflows/src/error.rs +++ b/lib/crates/fabro-workflows/src/error.rs @@ -683,6 +683,7 @@ mod tests { fn llm_error_display() { let sdk_err = SdkError::Network { message: "connection refused".into(), + source: None, }; let err = FabroError::Llm(sdk_err); assert_eq!( @@ -695,11 +696,13 @@ mod tests { fn llm_error_retryable_delegates_to_sdk() { let retryable = FabroError::Llm(SdkError::Network { message: "timeout".into(), + source: None, }); assert!(retryable.is_retryable()); let non_retryable = FabroError::Llm(SdkError::Configuration { message: "bad config".into(), + source: None, }); assert!(!non_retryable.is_retryable()); } @@ -708,6 +711,7 @@ mod tests { fn llm_error_from_sdk_error() { let sdk_err = SdkError::Stream { message: "broken pipe".into(), + source: None, }; let err = FabroError::from(sdk_err); assert!(matches!(err, FabroError::Llm(_))); @@ -794,6 +798,7 @@ mod tests { fn failure_class_llm_timeout() { let err = FabroError::Llm(SdkError::RequestTimeout { message: "timed out".into(), + source: None, }); assert_eq!(err.failure_class(), FailureClass::TransientInfra); } @@ -849,6 +854,7 @@ mod tests { fn classify_sdk_request_timeout() { let err = SdkError::RequestTimeout { message: "timed out".into(), + source: None, }; assert_eq!(classify_sdk_error(&err), FailureClass::TransientInfra); } @@ -1546,6 +1552,7 @@ mod tests { fn to_fail_outcome_includes_error_message_as_reason() { let err = FabroError::Llm(SdkError::Network { message: "connection refused".into(), + source: None, }); let outcome = err.to_fail_outcome(); assert!(outcome @@ -1558,6 +1565,7 @@ mod tests { fn to_fail_outcome_no_context_updates() { let err = FabroError::Llm(SdkError::Network { message: "refused".into(), + source: None, }); let outcome = err.to_fail_outcome(); assert!(outcome.context_updates.is_empty()); @@ -1600,6 +1608,7 @@ mod tests { FabroError::handler("handler err"), FabroError::Llm(SdkError::Network { message: "refused".into(), + source: None, }), FabroError::Checkpoint("cp err".into()), FabroError::Stylesheet("style err".into()), diff --git a/lib/crates/fabro-workflows/src/event.rs b/lib/crates/fabro-workflows/src/event.rs index 29460ee55..438a673e3 100644 --- a/lib/crates/fabro-workflows/src/event.rs +++ b/lib/crates/fabro-workflows/src/event.rs @@ -1554,6 +1554,7 @@ mod tests { delay_secs: 1.5, error: fabro_llm::error::SdkError::Network { message: "rate limited".to_string(), + source: None, }, }, };