diff --git a/src/harness/model/mod.rs b/src/harness/model/mod.rs index aa24cb1..80bfd24 100644 --- a/src/harness/model/mod.rs +++ b/src/harness/model/mod.rs @@ -694,8 +694,13 @@ pub struct StreamAccumulator { usage: Option, /// Authoritative final response, when a `Completed` item was seen. completed: Option, - /// Terminal error message, when a failure item was seen. + /// Terminal error message, when an unstructured `Failed` item was seen. failed: Option, + /// Terminal structured provider failure, when a `ProviderFailed` item was + /// seen. Kept as the full struct (not stringified) so `finish()` can return + /// [`crate::error::TinyAgentsError::Provider`] and preserve the + /// status/code/`retryable` classification the retry layer needs. + failed_provider: Option, } impl StreamAccumulator { @@ -732,16 +737,11 @@ impl StreamAccumulator { self.failed = Some(message.clone()); } ModelStreamItem::ProviderFailed(error) => { - self.failed = Some(format!( - "{} provider error{}: {}", - error.provider, - error - .code - .as_deref() - .map(|code| format!(" ({code})")) - .unwrap_or_default(), - error.message - )); + // Retain the structured error rather than stringifying it, so + // `finish()` can surface `TinyAgentsError::Provider` and the retry + // layer sees the real status/code/`retryable` (a permanent 401 / + // `insufficient_quota` / 400 must not be retried as transient). + self.failed_provider = Some(error.clone()); } } } @@ -753,10 +753,10 @@ impl StreamAccumulator { fn push_tool_chunk(&mut self, call_id: &str, content: &str, tool_name: Option<&str>) { if let Some(entry) = self.tool_chunks.iter_mut().find(|(id, ..)| id == call_id) { entry.1.push_str(content); - if entry.2.is_none() { - if let Some(name) = tool_name.filter(|n| !n.is_empty()) { - entry.2 = Some(name.to_string()); - } + if entry.2.is_none() + && let Some(name) = tool_name.filter(|n| !n.is_empty()) + { + entry.2 = Some(name.to_string()); } } else { self.tool_chunks.push(( @@ -767,10 +767,10 @@ impl StreamAccumulator { } } - /// Returns `true` when a terminal item (`Completed` or `Failed`) has been - /// folded in. + /// Returns `true` when a terminal item (`Completed`, `Failed`, or + /// `ProviderFailed`) has been folded in. pub fn is_terminal(&self) -> bool { - self.completed.is_some() || self.failed.is_some() + self.completed.is_some() || self.failed.is_some() || self.failed_provider.is_some() } /// Returns the accumulated reasoning/thinking text streamed so far (the @@ -783,9 +783,16 @@ impl StreamAccumulator { /// /// # Errors /// - /// Returns [`crate::error::TinyAgentsError::Model`] when a - /// [`ModelStreamItem::Failed`] item was folded in. + /// Returns [`crate::error::TinyAgentsError::Provider`] when a + /// [`ModelStreamItem::ProviderFailed`] item was folded in — preserving the + /// structured status/code/`retryable` so a permanent failure is not retried + /// as transient — or [`crate::error::TinyAgentsError::Model`] when an + /// unstructured [`ModelStreamItem::Failed`] item was folded in. pub fn finish(self) -> Result { + if let Some(error) = self.failed_provider { + return Err(crate::error::TinyAgentsError::Provider(Box::new(error))); + } + if let Some(message) = self.failed { return Err(crate::error::TinyAgentsError::Model(message)); } diff --git a/src/harness/model/test.rs b/src/harness/model/test.rs index 64a86c5..63e5036 100644 --- a/src/harness/model/test.rs +++ b/src/harness/model/test.rs @@ -684,6 +684,59 @@ fn finish_backfills_usage_from_stream_delta_when_completed_lacks_it() { assert_eq!(finished.message.usage, Some(Usage::new(4, 6))); } +#[test] +fn finish_preserves_provider_error_classification_from_provider_failed() { + // A streamed `ProviderFailed` carrying a non-retryable provider error (401 + // auth) must surface as `TinyAgentsError::Provider` with the struct intact — + // not stringified into `Model` — so `is_retryable` classifies it as permanent + // instead of retrying + fallback-churning it as transient. + use crate::error::TinyAgentsError; + use crate::harness::retry::is_retryable; + + let mut acc = StreamAccumulator::new(); + acc.push(&ModelStreamItem::Started); + acc.push(&ModelStreamItem::ProviderFailed(ProviderError { + provider: "openai".into(), + status: Some(401), + code: Some("invalid_api_key".into()), + message: "Incorrect API key provided".into(), + retryable: false, + ..ProviderError::default() + })); + + assert!(acc.is_terminal()); + let err = acc.finish().unwrap_err(); + match &err { + TinyAgentsError::Provider(error) => { + assert_eq!(error.status, Some(401)); + assert_eq!(error.code.as_deref(), Some("invalid_api_key")); + assert!(!error.retryable); + } + other => panic!("expected Provider error, got {other:?}"), + } + assert!( + !is_retryable(&err), + "a permanent streamed provider failure must not be retried as transient" + ); +} + +#[test] +fn finish_maps_unstructured_failed_to_model_error() { + // The unstructured `Failed(String)` path stays `TinyAgentsError::Model` — no + // structured detail to classify from, so the retry layer treats it as a + // transient transport/parse failure. + use crate::error::TinyAgentsError; + + let mut acc = StreamAccumulator::new(); + acc.push(&ModelStreamItem::Failed("stream broke".into())); + + assert!(acc.is_terminal()); + match acc.finish().unwrap_err() { + TinyAgentsError::Model(message) => assert_eq!(message, "stream broke"), + other => panic!("expected Model error, got {other:?}"), + } +} + #[test] fn finish_names_reconstructed_tool_call_from_the_call_opening_delta_name() { // A call-opening delta carries the tool name (no args yet); subsequent diff --git a/src/harness/providers/openai/test.rs b/src/harness/providers/openai/test.rs index 0868568..3ca9066 100644 --- a/src/harness/providers/openai/test.rs +++ b/src/harness/providers/openai/test.rs @@ -513,7 +513,11 @@ fn provider_spec_builds_compatible_model() { } #[test] -fn provider_failed_stream_item_finishes_as_model_error() { +fn provider_failed_stream_item_finishes_as_provider_error() { + // A streamed `ProviderFailed` must finish as `TinyAgentsError::Provider` with + // the structured error preserved (status/code/`retryable`), so the retry + // layer classifies it from `retryable` instead of the old behavior that + // stringified it into `Model` and always retried it as transient. let mut accumulator = StreamAccumulator::new(); accumulator.push(&ModelStreamItem::ProviderFailed(ProviderError { provider: "groq".to_string(), @@ -525,8 +529,16 @@ fn provider_failed_stream_item_finishes_as_model_error() { raw: None, })); - let error = accumulator.finish().unwrap_err().to_string(); - assert!(error.contains("groq provider error (rate_limit): too many requests")); + match accumulator.finish().unwrap_err() { + TinyAgentsError::Provider(error) => { + assert_eq!(error.provider, "groq"); + assert_eq!(error.status, Some(429)); + assert_eq!(error.code.as_deref(), Some("rate_limit")); + assert_eq!(error.message, "too many requests"); + assert!(error.retryable); + } + other => panic!("expected Provider error, got {other:?}"), + } } #[tokio::test] diff --git a/src/harness/stream/mod.rs b/src/harness/stream/mod.rs index 14411f0..1712142 100644 --- a/src/harness/stream/mod.rs +++ b/src/harness/stream/mod.rs @@ -143,6 +143,5 @@ pub fn stream(chunks: &[StreamChunk], modes: &[StreamMode]) -> Vec .collect() } -#[cfg(test)] #[cfg(test)] mod test; diff --git a/tests/e2e_harness_provider_contracts.rs b/tests/e2e_harness_provider_contracts.rs index a4b09b5..5f3baee 100644 --- a/tests/e2e_harness_provider_contracts.rs +++ b/tests/e2e_harness_provider_contracts.rs @@ -281,9 +281,14 @@ async fn model_request_response_registry_and_stream_contracts_are_stable() { }, )])); let err = collect_model_stream(failed_stream).await.unwrap_err(); + // A streamed `ProviderFailed` now surfaces as a structured + // `TinyAgentsError::Provider` whose `Display` renders the provider, HTTP + // status, code, and message (preserving the classification the retry layer + // needs) rather than the old flattened " provider error" string. assert!( err.to_string() - .contains("mock provider error (internal): nope") + .contains("mock returned HTTP 500 (internal): nope"), + "unexpected error string: {err}" ); }