diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index 6185cb54cc..e64bb0af08 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -4530,7 +4530,7 @@ impl Thread { // provider's requested delay when it gave one, and fall back to // exponential backoff when it didn't. ProviderRejection { retry_after, .. } => { - if error.retry_delay(1).is_none() { + if !error.is_transient() { return None; } Some(match retry_after { diff --git a/crates/language_model_core/src/language_model_core.rs b/crates/language_model_core/src/language_model_core.rs index 5bfc4edbbf..388d8b591e 100644 --- a/crates/language_model_core/src/language_model_core.rs +++ b/crates/language_model_core/src/language_model_core.rs @@ -369,6 +369,37 @@ impl LanguageModelCompletionError { } } + /// Whether retrying the same request after a delay may succeed. + pub fn is_transient(&self) -> bool { + match self { + Self::ProviderRejection { + provider, + status, + retry_after, + category, + .. + } => { + status.is_some_and(|status| is_retryable_provider_status(provider, status)) + || matches!( + category, + ProviderErrorCategory::RateLimit + | ProviderErrorCategory::Overloaded + | ProviderErrorCategory::Timeout + | ProviderErrorCategory::InternalServer + ) + || retry_after.is_some() + } + Self::ApiReadResponseError { .. } | Self::HttpSend { .. } => true, + Self::DataRetentionConsentRequired { .. } + | Self::NoApiKey { .. } + | Self::SerializeRequest { .. } + | Self::BuildRequestBody { .. } + | Self::DeserializeResponse { .. } + | Self::StreamEndedUnexpectedly { .. } + | Self::Other(_) => false, + } + } + /// Returns the delay before a retry attempt, honoring a provider-supplied /// delay before falling back to exponential backoff from five to forty /// seconds. @@ -377,26 +408,12 @@ impl LanguageModelCompletionError { /// transient, and error kinds without shared retry semantics, return /// `None`. pub fn retry_delay(&self, attempt: usize) -> Option { - if attempt == 0 { + if attempt == 0 || !self.is_transient() { return None; } match self { - Self::ProviderRejection { - status, - retry_after, - category, - .. - } if status.is_some_and(is_retryable_provider_status) - || matches!( - category, - ProviderErrorCategory::RateLimit - | ProviderErrorCategory::Overloaded - | ProviderErrorCategory::Timeout - | ProviderErrorCategory::InternalServer - ) - || retry_after.is_some() => - { + Self::ProviderRejection { retry_after, .. } => { (*retry_after).or_else(|| exponential_backoff(attempt)) } Self::ApiReadResponseError { .. } | Self::HttpSend { .. } => { @@ -404,7 +421,6 @@ impl LanguageModelCompletionError { } Self::DataRetentionConsentRequired { .. } | Self::NoApiKey { .. } - | Self::ProviderRejection { .. } | Self::SerializeRequest { .. } | Self::BuildRequestBody { .. } | Self::DeserializeResponse { .. } @@ -446,8 +462,13 @@ fn category_from_cloud_failure(code: &str, message: &str) -> ProviderErrorCatego } } -fn is_retryable_provider_status(status: StatusCode) -> bool { - status.is_server_error() || matches!(status.as_u16(), 408 | 425 | 429) +fn is_retryable_provider_status(provider: &LanguageModelProviderName, status: StatusCode) -> bool { + // OpenAI sometimes returns 404 during service disruptions instead of a server-error status. + // Retrying adds only bounded delay and network traffic when the endpoint is genuinely absent, + // while potentially hiding a spurious failure from the user when the outage is transient. + status.is_server_error() + || matches!(status.as_u16(), 408 | 425 | 429) + || (provider == &OPEN_AI_PROVIDER_NAME && status == StatusCode::NOT_FOUND) } fn exponential_backoff(attempt: usize) -> Option { @@ -998,6 +1019,7 @@ mod tests { "Rate limit exceeded".to_string(), Some(retry_after), ); + assert!(error.is_transient()); assert_eq!(error.retry_delay(1), Some(retry_after)); let error = LanguageModelCompletionError::from_http_status( @@ -1006,6 +1028,7 @@ mod tests { "Internal server error".to_string(), None, ); + assert!(error.is_transient()); assert_eq!(error.retry_delay(0), None); assert_eq!(error.retry_delay(1), Some(Duration::from_secs(5))); assert_eq!(error.retry_delay(2), Some(Duration::from_secs(10))); @@ -1028,10 +1051,44 @@ mod tests { category, ); + assert!(error.is_transient()); assert_eq!(error.retry_delay(1), Some(Duration::from_secs(5))); } } + #[test] + fn test_open_ai_not_found_is_transient() { + let error = LanguageModelCompletionError::from_http_status( + OPEN_AI_PROVIDER_NAME, + StatusCode::NOT_FOUND, + "Not found".to_string(), + None, + ); + + assert!(matches!( + error, + LanguageModelCompletionError::ProviderRejection { + category: ProviderErrorCategory::EndpointNotFound, + .. + } + )); + assert!(error.is_transient()); + assert_eq!(error.retry_delay(1), Some(Duration::from_secs(5))); + } + + #[test] + fn test_not_found_is_permanent_for_other_providers() { + let error = LanguageModelCompletionError::from_http_status( + ANTHROPIC_PROVIDER_NAME, + StatusCode::NOT_FOUND, + "Not found".to_string(), + None, + ); + + assert!(!error.is_transient()); + assert_eq!(error.retry_delay(1), None); + } + #[test] fn test_retry_delay_rejects_permanent_provider_error() { let error = LanguageModelCompletionError::from_provider_response( @@ -1043,6 +1100,7 @@ mod tests { ProviderErrorCategory::Other, ); + assert!(!error.is_transient()); assert_eq!(error.retry_delay(1), None); }