diff --git a/Cargo.lock b/Cargo.lock index b6a0102b0b..5aed7fa2f4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12124,7 +12124,6 @@ dependencies = [ "serde", "serde_json", "settings", - "strum 0.27.2", "thiserror 2.0.17", ] diff --git a/crates/agent/Cargo.toml b/crates/agent/Cargo.toml index 2ef00b0953..5d8408fd77 100644 --- a/crates/agent/Cargo.toml +++ b/crates/agent/Cargo.toml @@ -53,6 +53,7 @@ paths.workspace = true project.workspace = true prompt_store.workspace = true quick-xml.workspace = true +rand.workspace = true regex.workspace = true rust-embed.workspace = true sandbox.workspace = true @@ -105,7 +106,6 @@ lsp = { workspace = true, "features" = ["test-support"] } pretty_assertions.workspace = true project = { workspace = true, "features" = ["test-support"] } proptest.workspace = true -rand.workspace = true reqwest_client.workspace = true settings = { workspace = true, "features" = ["test-support"] } diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index af52932447..1b46bdfb25 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -60,16 +60,31 @@ use project::{ trusted_worktrees::TrustedWorktrees, }; use prompt_store::{ProjectContext, RULES_FILE_NAMES, RulesFileContext, WorktreeContext}; +use rand::Rng as _; use serde::{Deserialize, Serialize}; use settings::{LanguageModelSelection, Settings as _, update_settings_file}; use std::any::Any; use std::path::PathBuf; use std::rc::Rc; use std::sync::{Arc, LazyLock}; +use std::time::Duration; use util::ResultExt; use util::path_list::PathList; use util::rel_path::RelPath; +const MAXIMUM_RETRY_JITTER_FRACTION: f64 = 0.1; + +pub(crate) fn jitter_retry_delay(delay: Duration) -> Duration { + let jitter = delay.mul_f64(rand::rng().random_range(0.0..MAXIMUM_RETRY_JITTER_FRACTION)); + delay.checked_add(jitter).unwrap_or(Duration::MAX) +} + +#[cfg(test)] +pub(crate) fn maximum_retry_delay_with_jitter(delay: Duration) -> Duration { + let maximum_jitter = delay.mul_f64(MAXIMUM_RETRY_JITTER_FRACTION); + delay.checked_add(maximum_jitter).unwrap_or(Duration::MAX) +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct ProjectSnapshot { pub worktree_snapshots: Vec, diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index 3f50789786..2ed44e285c 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -29,7 +29,7 @@ use language_model::{ LanguageModelId, LanguageModelImageExt, LanguageModelProviderId, LanguageModelProviderName, LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage, LanguageModelToolResult, LanguageModelToolSchemaFormat, LanguageModelToolUse, MessageContent, - Role, StopReason, TokenUsage, + ProviderErrorCategory, Role, StopReason, TokenUsage, fake_provider::{FakeLanguageModel, FakeLanguageModelProvider}, }; use pretty_assertions::assert_eq; @@ -3263,13 +3263,12 @@ async fn test_retry_cancelled_promptly_on_new_send(cx: &mut TestAppContext) { assert_eq!(model_a.completion_count(), 1); // Model returns a retryable upstream 500. The turn enters the retry delay. - model_a.send_last_completion_stream_error( - LanguageModelCompletionError::UpstreamProviderError { - message: "Internal server error".to_string(), - status: http_client::StatusCode::INTERNAL_SERVER_ERROR, - retry_after: None, - }, - ); + model_a.send_last_completion_stream_error(LanguageModelCompletionError::from_http_status( + language_model::LanguageModelProviderName::new("test"), + http_client::StatusCode::INTERNAL_SERVER_ERROR, + "Internal server error".to_string(), + None, + )); model_a.end_last_completion_stream(); cx.run_until_parked(); @@ -3614,9 +3613,12 @@ async fn test_prompt_too_large_marks_token_usage_exceeded(cx: &mut TestAppContex .unwrap(); cx.run_until_parked(); - fake_model.send_last_completion_stream_error(LanguageModelCompletionError::PromptTooLarge { - tokens: None, - }); + fake_model.send_last_completion_stream_error(LanguageModelCompletionError::from_http_status( + LanguageModelProviderName::new("test"), + http_client::StatusCode::PAYLOAD_TOO_LARGE, + "prompt too large".to_string(), + None, + )); fake_model.end_last_completion_stream(); cx.run_until_parked(); @@ -3640,9 +3642,12 @@ async fn test_prompt_too_large_uses_reported_token_count(cx: &mut TestAppContext .unwrap(); cx.run_until_parked(); - fake_model.send_last_completion_stream_error(LanguageModelCompletionError::PromptTooLarge { - tokens: Some(1_500_000), - }); + fake_model.send_last_completion_stream_error(LanguageModelCompletionError::from_http_status( + LanguageModelProviderName::new("test"), + http_client::StatusCode::PAYLOAD_TOO_LARGE, + "prompt is too long: 1500000 tokens".to_string(), + None, + )); fake_model.end_last_completion_stream(); cx.run_until_parked(); @@ -4047,11 +4052,12 @@ async fn test_title_generation_failure_allows_retry(cx: &mut TestAppContext) { cx.run_until_parked(); fake_summary_model.send_last_completion_stream_error( - LanguageModelCompletionError::UpstreamProviderError { - message: "Internal server error".to_string(), - status: gpui::http_client::StatusCode::INTERNAL_SERVER_ERROR, - retry_after: None, - }, + LanguageModelCompletionError::from_http_status( + language_model::LanguageModelProviderName::new("test"), + gpui::http_client::StatusCode::INTERNAL_SERVER_ERROR, + "Internal server error".to_string(), + None, + ), ); fake_summary_model.end_last_completion_stream(); send.collect::>().await; @@ -4434,13 +4440,18 @@ async fn test_send_retry_on_error(cx: &mut TestAppContext) { cx.run_until_parked(); fake_model.send_last_completion_stream_text_chunk("Hey,"); - fake_model.send_last_completion_stream_error(LanguageModelCompletionError::ServerOverloaded { - provider: LanguageModelProviderName::new("Anthropic"), - retry_after: Some(Duration::from_secs(3)), - }); + fake_model.send_last_completion_stream_error(LanguageModelCompletionError::from_http_status( + LanguageModelProviderName::new("Anthropic"), + http_client::StatusCode::SERVICE_UNAVAILABLE, + "Anthropic's API servers are overloaded right now".to_string(), + Some(Duration::from_secs(3)), + )); fake_model.end_last_completion_stream(); - cx.executor().advance_clock(Duration::from_secs(3)); + cx.executor() + .advance_clock(crate::maximum_retry_delay_with_jitter(Duration::from_secs( + 3, + ))); cx.run_until_parked(); fake_model.send_last_completion_stream_text_chunk("there!"); @@ -4509,13 +4520,18 @@ async fn test_send_retry_finishes_tool_calls_on_error(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( tool_use_1.clone(), )); - fake_model.send_last_completion_stream_error(LanguageModelCompletionError::ServerOverloaded { - provider: LanguageModelProviderName::new("Anthropic"), - retry_after: Some(Duration::from_secs(3)), - }); + fake_model.send_last_completion_stream_error(LanguageModelCompletionError::from_http_status( + LanguageModelProviderName::new("Anthropic"), + http_client::StatusCode::SERVICE_UNAVAILABLE, + "Anthropic's API servers are overloaded right now".to_string(), + Some(Duration::from_secs(3)), + )); fake_model.end_last_completion_stream(); - cx.executor().advance_clock(Duration::from_secs(3)); + cx.executor() + .advance_clock(crate::maximum_retry_delay_with_jitter(Duration::from_secs( + 3, + ))); let completion = fake_model.pending_completions().pop().unwrap(); assert_eq!( completion.messages[1..], @@ -4579,13 +4595,18 @@ async fn test_send_max_retries_exceeded(cx: &mut TestAppContext) { for _ in 0..crate::thread::MAX_RETRY_ATTEMPTS + 1 { fake_model.send_last_completion_stream_error( - LanguageModelCompletionError::ServerOverloaded { - provider: LanguageModelProviderName::new("Anthropic"), - retry_after: Some(Duration::from_secs(3)), - }, + LanguageModelCompletionError::from_http_status( + LanguageModelProviderName::new("Anthropic"), + http_client::StatusCode::SERVICE_UNAVAILABLE, + "Anthropic's API servers are overloaded right now".to_string(), + Some(Duration::from_secs(3)), + ), ); fake_model.end_last_completion_stream(); - cx.executor().advance_clock(Duration::from_secs(3)); + cx.executor() + .advance_clock(crate::maximum_retry_delay_with_jitter(Duration::from_secs( + 3, + ))); cx.run_until_parked(); } @@ -4615,7 +4636,10 @@ async fn test_send_max_retries_exceeded(cx: &mut TestAppContext) { .unwrap(); assert!(matches!( error, - LanguageModelCompletionError::ServerOverloaded { .. } + LanguageModelCompletionError::ProviderRejection { + category: ProviderErrorCategory::Overloaded, + .. + } )); } @@ -4662,17 +4686,19 @@ async fn test_streaming_tool_completes_when_llm_stream_ends_without_final_input( // Before the fix, this would deadlock: the tool waits for more partials // (or cancellation), run_turn_internal waits for the tool, and the sender // keeping the channel open lives inside RunningTurn. - fake_model.send_last_completion_stream_error( - LanguageModelCompletionError::UpstreamProviderError { - message: "Internal server error".to_string(), - status: http_client::StatusCode::INTERNAL_SERVER_ERROR, - retry_after: None, - }, - ); + fake_model.send_last_completion_stream_error(LanguageModelCompletionError::from_http_status( + language_model::LanguageModelProviderName::new("test"), + http_client::StatusCode::INTERNAL_SERVER_ERROR, + "Internal server error".to_string(), + None, + )); fake_model.end_last_completion_stream(); // Advance past the retry delay so run_turn_internal retries. - cx.executor().advance_clock(Duration::from_secs(5)); + cx.executor() + .advance_clock(crate::maximum_retry_delay_with_jitter(Duration::from_secs( + 5, + ))); cx.run_until_parked(); // The retry request should contain the streaming tool's error result, @@ -6948,9 +6974,12 @@ async fn test_subagent_error_propagation(cx: &mut TestAppContext) { }); // The subagent's model returns a non-retryable error - model.send_last_completion_stream_error(LanguageModelCompletionError::PromptTooLarge { - tokens: None, - }); + model.send_last_completion_stream_error(LanguageModelCompletionError::from_http_status( + LanguageModelProviderName::new("test"), + http_client::StatusCode::PAYLOAD_TOO_LARGE, + "prompt too large".to_string(), + None, + )); cx.run_until_parked(); diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index f2e34495c1..f850907d7a 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -44,8 +44,8 @@ use language_model::{ LanguageModelId, LanguageModelImage, LanguageModelProviderId, LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage, LanguageModelRequestTool, LanguageModelToolResult, LanguageModelToolResultContent, LanguageModelToolSchemaFormat, - LanguageModelToolUse, LanguageModelToolUseId, MessageContent, Role, SelectedModel, Speed, - StopReason, TokenUsage, ZED_CLOUD_PROVIDER_ID, + LanguageModelToolUse, LanguageModelToolUseId, MessageContent, ProviderErrorCategory, Role, + SelectedModel, Speed, StopReason, TokenUsage, ZED_CLOUD_PROVIDER_ID, }; use project::{Project, trusted_worktrees::TrustedWorktrees}; use prompt_store::ProjectContext; @@ -169,16 +169,34 @@ impl std::fmt::Display for PromptId { pub(crate) const MAX_RETRY_ATTEMPTS: u8 = 4; pub(crate) const BASE_RETRY_DELAY: Duration = Duration::from_secs(5); -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq)] enum RetryStrategy { - ExponentialBackoff { - initial_delay: Duration, - max_attempts: u8, - }, - Fixed { - delay: Duration, - max_attempts: u8, - }, + ExponentialBackoff, + FixedDelay { delay: Duration, max_attempts: u8 }, +} + +impl RetryStrategy { + fn delay_after(&self, error: &LanguageModelCompletionError, attempt: u8) -> Option { + if attempt == 0 { + return None; + } + match self { + RetryStrategy::ExponentialBackoff => (attempt <= MAX_RETRY_ATTEMPTS) + .then(|| error.retry_delay(attempt as usize)) + .flatten(), + RetryStrategy::FixedDelay { + delay, + max_attempts, + } => (attempt <= *max_attempts).then_some(*delay), + } + } + + fn max_attempts(&self) -> u8 { + match self { + RetryStrategy::ExponentialBackoff => MAX_RETRY_ATTEMPTS, + RetryStrategy::FixedDelay { max_attempts, .. } => *max_attempts, + } + } } #[derive(Debug, PartialEq, Serialize, Deserialize)] @@ -3291,7 +3309,11 @@ impl Thread { plan: Option, cx: &mut Context, ) -> Result { - if let LanguageModelCompletionError::PromptTooLarge { tokens } = &error { + if let LanguageModelCompletionError::ProviderRejection { + category: ProviderErrorCategory::PromptTooLarge { tokens }, + .. + } = &error + { self.mark_token_limit_exceeded(*tokens, cx); } @@ -3313,28 +3335,17 @@ impl Thread { return Err(anyhow!(error)); }; - let max_attempts = match &strategy { - RetryStrategy::ExponentialBackoff { max_attempts, .. } => *max_attempts, - RetryStrategy::Fixed { max_attempts, .. } => *max_attempts, - }; - - if attempt > max_attempts { + let Some(delay) = strategy.delay_after(&error, attempt) else { return Err(anyhow!(error)); - } - - let delay = match &strategy { - RetryStrategy::ExponentialBackoff { initial_delay, .. } => { - let delay_secs = initial_delay.as_secs() * 2u64.pow((attempt - 1) as u32); - Duration::from_secs(delay_secs) - } - RetryStrategy::Fixed { delay, .. } => *delay, }; + let delay = crate::jitter_retry_delay(delay); + log::debug!("Retry attempt {attempt} with delay {delay:?}"); Ok(acp_thread::RetryStatus { last_error: error.to_string().into(), attempt: attempt as usize, - max_attempts: max_attempts as usize, + max_attempts: strategy.max_attempts() as usize, started_at: Instant::now(), duration: delay, meta: None, @@ -4509,104 +4520,48 @@ impl Thread { fn retry_strategy_for(error: &LanguageModelCompletionError) -> Option { use LanguageModelCompletionError::*; - use http_client::StatusCode; - // General strategy here: - // - If retrying won't help (e.g. invalid API key or payload too large), return None so we don't retry at all. - // - If it's a time-based issue (e.g. server overloaded, rate limit exceeded), retry up to 4 times with exponential backoff. - // - If it's an issue that *might* be fixed by retrying (e.g. internal server error), retry up to 3 times. match error { - HttpResponseError { - status_code: StatusCode::TOO_MANY_REQUESTS, - .. - } => Some(RetryStrategy::ExponentialBackoff { - initial_delay: BASE_RETRY_DELAY, - max_attempts: MAX_RETRY_ATTEMPTS, - }), - ServerOverloaded { retry_after, .. } | RateLimitExceeded { retry_after, .. } => { - Some(RetryStrategy::Fixed { - delay: retry_after.unwrap_or(BASE_RETRY_DELAY), - max_attempts: MAX_RETRY_ATTEMPTS, + // A rejection with no status (e.g. a content-policy rejection + // like `cyber_policy`) is permanent: retrying sends the same + // request and gets the same answer. A rejection with a + // non-retryable status (auth, payload too large, ...) is + // permanent for the same reason. Otherwise, honor the + // 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() { + return None; + } + Some(match retry_after { + Some(delay) => RetryStrategy::FixedDelay { + delay: *delay, + max_attempts: MAX_RETRY_ATTEMPTS, + }, + None => RetryStrategy::ExponentialBackoff, }) } - UpstreamProviderError { - status, - retry_after, - .. - } => match *status { - StatusCode::TOO_MANY_REQUESTS | StatusCode::SERVICE_UNAVAILABLE => { - Some(RetryStrategy::Fixed { - delay: retry_after.unwrap_or(BASE_RETRY_DELAY), - max_attempts: MAX_RETRY_ATTEMPTS, - }) - } - StatusCode::INTERNAL_SERVER_ERROR => Some(RetryStrategy::Fixed { - delay: retry_after.unwrap_or(BASE_RETRY_DELAY), - // Internal Server Error could be anything, retry up to 3 times. + ApiReadResponseError { .. } | HttpSend { .. } | DeserializeResponse { .. } => { + Some(RetryStrategy::FixedDelay { + delay: BASE_RETRY_DELAY, max_attempts: 3, - }), - status => { - // There is no StatusCode variant for the unofficial HTTP 529 ("The service is overloaded"), - // but we frequently get them in practice. See https://http.dev/529 - if status.as_u16() == 529 { - Some(RetryStrategy::Fixed { - delay: retry_after.unwrap_or(BASE_RETRY_DELAY), - max_attempts: MAX_RETRY_ATTEMPTS, - }) - } else { - Some(RetryStrategy::Fixed { - delay: retry_after.unwrap_or(BASE_RETRY_DELAY), - max_attempts: 2, - }) - } - } - }, - ApiInternalServerError { .. } => Some(RetryStrategy::Fixed { - delay: BASE_RETRY_DELAY, - max_attempts: 3, - }), - ApiReadResponseError { .. } - | HttpSend { .. } - | DeserializeResponse { .. } - | BadRequestFormat { .. } => Some(RetryStrategy::Fixed { - delay: BASE_RETRY_DELAY, - max_attempts: 3, - }), - // Retrying these errors definitely shouldn't help. - HttpResponseError { - status_code: - StatusCode::PAYLOAD_TOO_LARGE | StatusCode::FORBIDDEN | StatusCode::UNAUTHORIZED, - .. + }) } - | AuthenticationError { .. } - | PermissionError { .. } - | NoApiKey { .. } - | ApiEndpointNotFound { .. } - | PromptTooLarge { .. } - | InvalidEncryptedContent { .. } => None, + // Retrying these errors definitely shouldn't help. + NoApiKey { .. } => None, // These errors might be transient, so retry them SerializeRequest { .. } | BuildRequestBody { .. } | StreamEndedUnexpectedly { .. } => { - Some(RetryStrategy::Fixed { + Some(RetryStrategy::FixedDelay { delay: BASE_RETRY_DELAY, max_attempts: 1, }) } - // Retry all other 4xx and 5xx errors once. - HttpResponseError { status_code, .. } - if status_code.is_client_error() || status_code.is_server_error() => - { - Some(RetryStrategy::Fixed { - delay: BASE_RETRY_DELAY, - max_attempts: 3, - }) - } - // Retrying won't help for Payment Required errors. - PaymentRequired => None, // Retrying won't help until the user consents to data retention // or switches models. DataRetentionConsentRequired { .. } => None, - // Conservatively assume that any other errors are non-retryable - HttpResponseError { .. } | Other(..) => Some(RetryStrategy::Fixed { + // `Other` includes mid-stream mapping failures that can be caused by + // a transient malformed or interrupted provider event. + Other(..) => Some(RetryStrategy::FixedDelay { delay: BASE_RETRY_DELAY, max_attempts: 2, }), @@ -8431,6 +8386,135 @@ mod tests { assert_eq!(deny.option_kind, acp::PermissionOptionKind::RejectOnce); } + #[test] + fn test_retry_strategy_does_not_retry_status_less_provider_rejection() { + // A rejection like OpenAI's `cyber_policy` content-policy error has + // no HTTP status of its own (it arrives as a Zed cloud upstream + // error code); retrying sends the identical request and gets the + // identical rejection, so it must not retry. + let error = LanguageModelCompletionError::from_provider_response( + language_model::OPEN_AI_PROVIDER_NAME, + None, + Some("cyber_policy".to_string()), + "This content was flagged as potentially violating our terms of use.".to_string(), + None, + language_model::ProviderErrorCategory::Other, + ); + let LanguageModelCompletionError::ProviderRejection { + category, + code, + message, + .. + } = &error + else { + panic!("expected a ProviderRejection, got {error:?}"); + }; + assert_eq!(*category, language_model::ProviderErrorCategory::Other); + assert_eq!(code.as_deref(), Some("cyber_policy")); + assert_eq!( + message, + "This content was flagged as potentially violating our terms of use." + ); + + assert!(Thread::retry_strategy_for(&error).is_none()); + } + + #[test] + fn test_retry_strategy_does_not_retry_non_retryable_status() { + // 401 Unauthorized is permanent: retrying with the same credentials + // fails the same way. + let error = LanguageModelCompletionError::from_http_status( + language_model::LanguageModelProviderName::new("Anthropic"), + http_client::StatusCode::UNAUTHORIZED, + "invalid x-api-key".to_string(), + None, + ); + assert!(Thread::retry_strategy_for(&error).is_none()); + } + + #[test] + fn test_retry_strategy_retries_status_less_transient_category() { + let error = LanguageModelCompletionError::from_provider_response( + language_model::ANTHROPIC_PROVIDER_NAME, + None, + Some("rate_limit_error".to_string()), + "Rate limit exceeded".to_string(), + None, + language_model::ProviderErrorCategory::RateLimit, + ); + + assert_eq!( + Thread::retry_strategy_for(&error), + Some(RetryStrategy::ExponentialBackoff) + ); + } + + #[test] + fn test_retry_strategy_uses_exponential_backoff_without_retry_after() { + for status in [ + http_client::StatusCode::TOO_MANY_REQUESTS, + http_client::StatusCode::INTERNAL_SERVER_ERROR, + http_client::StatusCode::SERVICE_UNAVAILABLE, + http_client::StatusCode::from_u16(529).unwrap(), + ] { + let error = LanguageModelCompletionError::from_http_status( + language_model::LanguageModelProviderName::new("Anthropic"), + status, + "upstream failure".to_string(), + None, + ); + let strategy = Thread::retry_strategy_for(&error) + .unwrap_or_else(|| panic!("expected a retry strategy for status {status}")); + assert_eq!( + strategy, + RetryStrategy::ExponentialBackoff, + "status {status} should use exponential backoff when no retry_after is given" + ); + assert_eq!(strategy.delay_after(&error, 0), None); + assert_eq!( + strategy.delay_after(&error, 1), + Some(BASE_RETRY_DELAY), + "first retry should use the base delay" + ); + assert_eq!( + strategy.delay_after(&error, 2), + Some(BASE_RETRY_DELAY * 2), + "second retry should double the delay" + ); + assert_eq!( + strategy.delay_after(&error, MAX_RETRY_ATTEMPTS + 1), + None, + "retrying should stop once attempts are exhausted" + ); + } + } + + #[test] + fn test_retry_strategy_honors_retry_after_with_a_fixed_delay() { + let retry_after = Duration::from_secs(3); + let error = LanguageModelCompletionError::from_http_status( + language_model::LanguageModelProviderName::new("Anthropic"), + http_client::StatusCode::TOO_MANY_REQUESTS, + "rate limited".to_string(), + Some(retry_after), + ); + let strategy = Thread::retry_strategy_for(&error).expect("429 should retry"); + assert_eq!( + strategy, + RetryStrategy::FixedDelay { + delay: retry_after, + max_attempts: MAX_RETRY_ATTEMPTS, + } + ); + // The delay stays fixed across attempts, unlike exponential backoff. + assert_eq!(strategy.delay_after(&error, 1), Some(retry_after)); + assert_eq!( + strategy.delay_after(&error, MAX_RETRY_ATTEMPTS), + Some(retry_after) + ); + assert_eq!(strategy.delay_after(&error, MAX_RETRY_ATTEMPTS + 1), None); + } + #[gpui::test] async fn test_replay_tool_call_replays_image_content(cx: &mut TestAppContext) { let (thread, _event_stream) = setup_thread_for_test(cx).await; diff --git a/crates/agent/src/tools/evals.rs b/crates/agent/src/tools/evals.rs index ac11ffe74a..d110761af7 100644 --- a/crates/agent/src/tools/evals.rs +++ b/crates/agent/src/tools/evals.rs @@ -3,7 +3,7 @@ use futures::future::LocalBoxFuture; #[cfg(all(test, feature = "unit-eval"))] use gpui::TestAppContext; #[cfg(all(test, feature = "unit-eval"))] -use std::fmt::Display; +use std::{fmt::Display, time::Duration}; #[cfg(all(test, feature = "unit-eval"))] mod edit_file; @@ -12,6 +12,14 @@ mod terminal_tool; #[cfg(all(test, feature = "unit-eval"))] mod write_file; +#[cfg(all(test, feature = "unit-eval"))] +fn completion_retry_delay(error: &anyhow::Error, retry_attempt: usize) -> Option { + error + .downcast_ref::()? + .retry_delay(retry_attempt) + .map(crate::jitter_retry_delay) +} + #[cfg(all(test, feature = "unit-eval"))] fn run_gpui_eval( eval: impl for<'a> FnOnce(&'a mut TestAppContext) -> LocalBoxFuture<'a, anyhow::Result>, diff --git a/crates/agent/src/tools/evals/edit_file.rs b/crates/agent/src/tools/evals/edit_file.rs index 4a8537805d..73e449fa3c 100644 --- a/crates/agent/src/tools/evals/edit_file.rs +++ b/crates/agent/src/tools/evals/edit_file.rs @@ -9,17 +9,14 @@ use client::{Client, RefreshLlmTokenListener, UserStore}; use fs::FakeFs; use futures::{FutureExt, StreamExt, future::LocalBoxFuture}; use gpui::{AppContext as _, AsyncApp, Entity, TestAppContext, UpdateGlobal as _}; -use http_client::StatusCode; use language::language_settings::FormatOnSave; use language_model::{ - LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, - LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage, - LanguageModelToolResult, LanguageModelToolResultContent, LanguageModelToolUse, - LanguageModelToolUseId, MessageContent, Role, SelectedModel, + LanguageModel, LanguageModelCompletionEvent, LanguageModelRegistry, LanguageModelRequest, + LanguageModelRequestMessage, LanguageModelToolResult, LanguageModelToolResultContent, + LanguageModelToolUse, LanguageModelToolUseId, MessageContent, Role, SelectedModel, }; use project::Project; use prompt_store::{ProjectContext, WorktreeContext}; -use rand::prelude::*; use reqwest_client::ReqwestClient; use serde::Serialize; use serde_json::json; @@ -29,7 +26,6 @@ use std::{ path::{Path, PathBuf}, str::FromStr, sync::Arc, - time::Duration, }; use util::path; @@ -639,57 +635,27 @@ fn strip_empty_lines(text: &str) -> String { } async fn retry_on_rate_limit(mut request: impl AsyncFnMut() -> Result) -> Result { - const MAX_RETRIES: usize = 20; - let mut attempt = 0; + const MAX_ATTEMPTS: usize = 20; + let mut completed_attempts = 0; loop { - attempt += 1; let response = request().await; + completed_attempts += 1; - if attempt >= MAX_RETRIES { + if completed_attempts >= MAX_ATTEMPTS { return response; } - let retry_delay = match &response { - Ok(_) => None, - Err(err) => match err.downcast_ref::() { - Some(err) => match &err { - LanguageModelCompletionError::RateLimitExceeded { retry_after, .. } - | LanguageModelCompletionError::ServerOverloaded { retry_after, .. } => { - Some(retry_after.unwrap_or(Duration::from_secs(5))) - } - LanguageModelCompletionError::UpstreamProviderError { - status, - retry_after, - .. - } => { - let should_retry = matches!( - *status, - StatusCode::TOO_MANY_REQUESTS | StatusCode::SERVICE_UNAVAILABLE - ) || status.as_u16() == 529; + let retry_attempt = completed_attempts; + let retry_delay = response + .as_ref() + .err() + .and_then(|error| super::completion_retry_delay(error, retry_attempt)); - if should_retry { - Some(retry_after.unwrap_or(Duration::from_secs(5))) - } else { - None - } - } - LanguageModelCompletionError::ApiReadResponseError { .. } - | LanguageModelCompletionError::ApiInternalServerError { .. } - | LanguageModelCompletionError::HttpSend { .. } => { - Some(Duration::from_secs(2_u64.pow((attempt - 1) as u32).min(30))) - } - _ => None, - }, - _ => None, - }, - }; - - if let Some(retry_after) = retry_delay { - let jitter = retry_after.mul_f64(rand::rng().random_range(0.0..1.0)); - eprintln!("Attempt #{attempt}: Retry after {retry_after:?} + jitter of {jitter:?}"); + if let Some(retry_delay) = retry_delay { + eprintln!("Retry attempt #{retry_attempt}: Retry after {retry_delay:?}"); #[allow(clippy::disallowed_methods)] - async_io::Timer::after(retry_after + jitter).await; + async_io::Timer::after(retry_delay).await; } else { return response; } diff --git a/crates/agent/src/tools/evals/terminal_tool.rs b/crates/agent/src/tools/evals/terminal_tool.rs index 31b35c0e53..463a95975a 100644 --- a/crates/agent/src/tools/evals/terminal_tool.rs +++ b/crates/agent/src/tools/evals/terminal_tool.rs @@ -4,14 +4,11 @@ use anyhow::{Context as _, Result}; use client::{Client, RefreshLlmTokenListener, UserStore}; use futures::{FutureExt as _, StreamExt}; use gpui::{AppContext as _, AsyncApp, TestAppContext}; -use http_client::StatusCode; use language_model::{ - LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, - LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage, MessageContent, Role, - SelectedModel, + LanguageModel, LanguageModelCompletionEvent, LanguageModelRegistry, LanguageModelRequest, + LanguageModelRequestMessage, MessageContent, Role, SelectedModel, }; use prompt_store::{ProjectContext, WorktreeContext}; -use rand::prelude::*; use reqwest_client::ReqwestClient; use settings::SettingsStore; use std::{ @@ -19,7 +16,6 @@ use std::{ path::Path, str::FromStr, sync::Arc, - time::Duration, }; #[derive(Clone)] @@ -377,57 +373,27 @@ async fn extract_tool_use( } async fn retry_on_rate_limit(mut request: impl AsyncFnMut() -> Result) -> Result { - const MAX_RETRIES: usize = 20; - let mut attempt = 0; + const MAX_ATTEMPTS: usize = 20; + let mut completed_attempts = 0; loop { - attempt += 1; let response = request().await; + completed_attempts += 1; - if attempt >= MAX_RETRIES { + if completed_attempts >= MAX_ATTEMPTS { return response; } - let retry_delay = match &response { - Ok(_) => None, - Err(err) => match err.downcast_ref::() { - Some(err) => match &err { - LanguageModelCompletionError::RateLimitExceeded { retry_after, .. } - | LanguageModelCompletionError::ServerOverloaded { retry_after, .. } => { - Some(retry_after.unwrap_or(Duration::from_secs(5))) - } - LanguageModelCompletionError::UpstreamProviderError { - status, - retry_after, - .. - } => { - let should_retry = matches!( - *status, - StatusCode::TOO_MANY_REQUESTS | StatusCode::SERVICE_UNAVAILABLE - ) || status.as_u16() == 529; + let retry_attempt = completed_attempts; + let retry_delay = response + .as_ref() + .err() + .and_then(|error| super::completion_retry_delay(error, retry_attempt)); - if should_retry { - Some(retry_after.unwrap_or(Duration::from_secs(5))) - } else { - None - } - } - LanguageModelCompletionError::ApiReadResponseError { .. } - | LanguageModelCompletionError::ApiInternalServerError { .. } - | LanguageModelCompletionError::HttpSend { .. } => { - Some(Duration::from_secs(2_u64.pow((attempt - 1) as u32).min(30))) - } - _ => None, - }, - _ => None, - }, - }; - - if let Some(retry_after) = retry_delay { - let jitter = retry_after.mul_f64(rand::rng().random_range(0.0..1.0)); - eprintln!("Attempt #{attempt}: Retry after {retry_after:?} + jitter of {jitter:?}"); + if let Some(retry_delay) = retry_delay { + eprintln!("Retry attempt #{retry_attempt}: Retry after {retry_delay:?}"); #[allow(clippy::disallowed_methods)] - async_io::Timer::after(retry_after + jitter).await; + async_io::Timer::after(retry_delay).await; } else { return response; } diff --git a/crates/agent/src/tools/evals/write_file.rs b/crates/agent/src/tools/evals/write_file.rs index c0fb1de7de..739b251770 100644 --- a/crates/agent/src/tools/evals/write_file.rs +++ b/crates/agent/src/tools/evals/write_file.rs @@ -8,17 +8,14 @@ use client::{Client, RefreshLlmTokenListener, UserStore}; use fs::FakeFs; use futures::{FutureExt as _, StreamExt}; use gpui::{AppContext as _, AsyncApp, Entity, TestAppContext, UpdateGlobal as _}; -use http_client::StatusCode; use language::language_settings::FormatOnSave; use language_model::{ - LanguageModel, LanguageModelCompletionError, LanguageModelCompletionEvent, - LanguageModelRegistry, LanguageModelRequest, LanguageModelRequestMessage, - LanguageModelToolResult, LanguageModelToolResultContent, LanguageModelToolUse, - LanguageModelToolUseId, MessageContent, Role, SelectedModel, + LanguageModel, LanguageModelCompletionEvent, LanguageModelRegistry, LanguageModelRequest, + LanguageModelRequestMessage, LanguageModelToolResult, LanguageModelToolResultContent, + LanguageModelToolUse, LanguageModelToolUseId, MessageContent, Role, SelectedModel, }; use project::Project; use prompt_store::{ProjectContext, WorktreeContext}; -use rand::prelude::*; use reqwest_client::ReqwestClient; use serde::Serialize; use settings::SettingsStore; @@ -27,7 +24,6 @@ use std::{ path::{Path, PathBuf}, str::FromStr, sync::Arc, - time::Duration, }; use util::path; @@ -435,57 +431,27 @@ fn tool_result( } async fn retry_on_rate_limit(mut request: impl AsyncFnMut() -> Result) -> Result { - const MAX_RETRIES: usize = 20; - let mut attempt = 0; + const MAX_ATTEMPTS: usize = 20; + let mut completed_attempts = 0; loop { - attempt += 1; let response = request().await; + completed_attempts += 1; - if attempt >= MAX_RETRIES { + if completed_attempts >= MAX_ATTEMPTS { return response; } - let retry_delay = match &response { - Ok(_) => None, - Err(err) => match err.downcast_ref::() { - Some(err) => match &err { - LanguageModelCompletionError::RateLimitExceeded { retry_after, .. } - | LanguageModelCompletionError::ServerOverloaded { retry_after, .. } => { - Some(retry_after.unwrap_or(Duration::from_secs(5))) - } - LanguageModelCompletionError::UpstreamProviderError { - status, - retry_after, - .. - } => { - let should_retry = matches!( - *status, - StatusCode::TOO_MANY_REQUESTS | StatusCode::SERVICE_UNAVAILABLE - ) || status.as_u16() == 529; + let retry_attempt = completed_attempts; + let retry_delay = response + .as_ref() + .err() + .and_then(|error| super::completion_retry_delay(error, retry_attempt)); - if should_retry { - Some(retry_after.unwrap_or(Duration::from_secs(5))) - } else { - None - } - } - LanguageModelCompletionError::ApiReadResponseError { .. } - | LanguageModelCompletionError::ApiInternalServerError { .. } - | LanguageModelCompletionError::HttpSend { .. } => { - Some(Duration::from_secs(2_u64.pow((attempt - 1) as u32).min(30))) - } - _ => None, - }, - _ => None, - }, - }; - - if let Some(retry_after) = retry_delay { - let jitter = retry_after.mul_f64(rand::rng().random_range(0.0..1.0)); - eprintln!("Attempt #{attempt}: Retry after {retry_after:?} + jitter of {jitter:?}"); + if let Some(retry_delay) = retry_delay { + eprintln!("Retry attempt #{retry_attempt}: Retry after {retry_delay:?}"); #[allow(clippy::disallowed_methods)] - async_io::Timer::after(retry_after + jitter).await; + async_io::Timer::after(retry_delay).await; } else { return response; } diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 281e4797ab..0b52beb4cd 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -34,7 +34,7 @@ use gpui::{ linear_gradient, list, pulsating_between, }; use language::{Buffer, Language, Rope}; -use language_model::LanguageModelCompletionError; +use language_model::{LanguageModelCompletionError, ProviderErrorCategory}; use markdown::{ CodeBlockRenderer, CopyButtonVisibility, Markdown, MarkdownElement, MarkdownFont, MarkdownStyle, }; @@ -146,7 +146,9 @@ pub(crate) enum ThreadError { provider: SharedString, message: Option, }, - RequestFailed, + ProviderRejection { + message: SharedString, + }, MaxOutputTokens, NoModelSelected, ApiError { @@ -171,16 +173,39 @@ impl From for ThreadError { } else if let Some(lm_error) = error.downcast_ref::() { use LanguageModelCompletionError::*; match lm_error { - RateLimitExceeded { provider, .. } => Self::RateLimitExceeded { - provider: provider.to_string().into(), - }, - ServerOverloaded { provider, .. } | ApiInternalServerError { provider, .. } => { - Self::ServerOverloaded { + ProviderRejection { + provider, + message, + category, + .. + } => match category { + ProviderErrorCategory::RateLimit => Self::RateLimitExceeded { provider: provider.to_string().into(), - } - } - PromptTooLarge { .. } => Self::PromptTooLarge, - PaymentRequired => Self::PaymentRequired, + }, + ProviderErrorCategory::Overloaded => Self::ServerOverloaded { + provider: provider.to_string().into(), + }, + ProviderErrorCategory::PromptTooLarge { .. } => Self::PromptTooLarge, + ProviderErrorCategory::PaymentRequired => Self::PaymentRequired, + ProviderErrorCategory::Authentication => Self::AuthenticationFailed { + provider: provider.to_string().into(), + }, + ProviderErrorCategory::Permission => Self::PermissionDenied { + provider: provider.to_string().into(), + message: Some(message.clone().into()), + }, + ProviderErrorCategory::EndpointNotFound => Self::ApiError { + provider: provider.to_string().into(), + }, + ProviderErrorCategory::InvalidEncryptedContent + | ProviderErrorCategory::InvalidRequest + | ProviderErrorCategory::Conflict + | ProviderErrorCategory::Timeout + | ProviderErrorCategory::InternalServer + | ProviderErrorCategory::Other => Self::ProviderRejection { + message: message.clone().into(), + }, + }, NoApiKey { provider } => Self::NoCredentials { provider: provider.to_string().into(), }, @@ -190,20 +215,7 @@ impl From for ThreadError { | HttpSend { provider, .. } => Self::StreamError { provider: provider.to_string().into(), }, - AuthenticationError { provider, .. } => Self::AuthenticationFailed { - provider: provider.to_string().into(), - }, - PermissionError { provider, message } => Self::PermissionDenied { - provider: provider.to_string().into(), - message: Some(message.clone().into()), - }, - UpstreamProviderError { .. } => Self::RequestFailed, DataRetentionConsentRequired { .. } => Self::DataRetentionConsentRequired, - BadRequestFormat { provider, .. } - | HttpResponseError { provider, .. } - | ApiEndpointNotFound { provider } => Self::ApiError { - provider: provider.to_string().into(), - }, _ => { let message: SharedString = format!("{:#}", error).into(); Self::Other { @@ -3707,6 +3719,26 @@ pub(crate) mod tests { ); } + #[test] + fn test_provider_rejection_preserves_provider_message() { + let provider_error = LanguageModelCompletionError::from_provider_response( + language_model::OPEN_AI_PROVIDER_NAME, + None, + Some("cyber_policy".to_string()), + "This content was flagged as potentially violating our terms of use.".to_string(), + None, + ProviderErrorCategory::Other, + ); + + let error = ThreadError::from(anyhow!(provider_error)); + + assert!(matches!( + error, + ThreadError::ProviderRejection { message } + if message == "This content was flagged as potentially violating our terms of use." + )); + } + #[gpui::test] async fn test_drop(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 3a6ea2118f..dfff3050a3 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -1928,11 +1928,9 @@ impl ThreadView { .into() }), ), - ThreadError::RequestFailed => ( - "request_failed", - None, - "Request could not be completed after multiple attempts.".into(), - ), + ThreadError::ProviderRejection { message } => { + ("provider_rejection", None, message.clone()) + } ThreadError::MaxOutputTokens => ( "max_output_tokens", None, @@ -11089,15 +11087,9 @@ impl ThreadView { self.render_error_callout("Permission Denied", message, false, false, cx) } - ThreadError::RequestFailed => self.render_error_callout( - "Request Failed", - "The request could not be completed after multiple attempts. \ - Try again in a moment." - .into(), - true, - false, - cx, - ), + ThreadError::ProviderRejection { message } => { + self.render_error_callout("Request Failed", message.clone(), true, false, cx) + } ThreadError::MaxOutputTokens => self.render_error_callout( "Output Limit Reached", "The model stopped because it reached its maximum output length. \ diff --git a/crates/anthropic/src/anthropic.rs b/crates/anthropic/src/anthropic.rs index 570efc19ec..bb229c923c 100644 --- a/crates/anthropic/src/anthropic.rs +++ b/crates/anthropic/src/anthropic.rs @@ -452,14 +452,19 @@ async fn handle_error_response( mut response: http::Response, rate_limits: RateLimitInfo, ) -> AnthropicError { - if response.status().as_u16() == 529 { + let status = response.status(); + if status.as_u16() == 529 { return AnthropicError::ServerOverloaded { + status, retry_after: rate_limits.retry_after, }; } if let Some(retry_after) = rate_limits.retry_after { - return AnthropicError::RateLimit { retry_after }; + return AnthropicError::RateLimit { + status, + retry_after, + }; } let mut body = String::new(); @@ -474,9 +479,12 @@ async fn handle_error_response( } match serde_json::from_str::(&body) { - Ok(Event::Error { error }) => AnthropicError::ApiError(error), + Ok(Event::Error { error }) => AnthropicError::ApiError { + status: Some(status), + error, + }, Ok(_) | Err(_) => AnthropicError::HttpResponseError { - status_code: response.status(), + status_code: status, message: body, }, } @@ -1096,13 +1104,22 @@ pub enum AnthropicError { }, /// Rate limit exceeded - RateLimit { retry_after: Duration }, + RateLimit { + status: StatusCode, + retry_after: Duration, + }, /// Server overloaded - ServerOverloaded { retry_after: Option }, + ServerOverloaded { + status: StatusCode, + retry_after: Option, + }, /// API returned an error response - ApiError(ApiError), + ApiError { + status: Option, + error: ApiError, + }, } #[derive(Debug, Serialize, Deserialize, Error)] @@ -1212,21 +1229,37 @@ pub fn completion_error_from_anthropic( AnthropicError::HttpResponseError { status_code, message, - } => Error::HttpResponseError { - provider, - status_code, - message, - }, - AnthropicError::RateLimit { retry_after } => Error::RateLimitExceeded { - provider, - retry_after: Some(retry_after), - }, - AnthropicError::ServerOverloaded { retry_after } => Error::ServerOverloaded { - provider, + } => Error::from_http_status(provider, status_code, message, None), + AnthropicError::RateLimit { + status, retry_after, - }, - AnthropicError::ApiError(api_error) => { - completion_error_from_anthropic_api(api_error, provider) + } => { + let message = format!("{provider}'s API rate limit exceeded"); + Error::from_provider_response( + provider, + Some(status), + None, + message, + Some(retry_after), + language_model_core::ProviderErrorCategory::RateLimit, + ) + } + AnthropicError::ServerOverloaded { + status, + retry_after, + } => { + let message = format!("{provider}'s API servers are overloaded right now"); + Error::from_provider_response( + provider, + Some(status), + None, + message, + retry_after, + language_model_core::ProviderErrorCategory::Overloaded, + ) + } + AnthropicError::ApiError { status, error } => { + completion_error_from_anthropic_api_with_status(error, provider, status) } } } @@ -1234,53 +1267,50 @@ pub fn completion_error_from_anthropic( pub fn completion_error_from_anthropic_api( error: ApiError, provider: language_model_core::LanguageModelProviderName, +) -> language_model_core::LanguageModelCompletionError { + completion_error_from_anthropic_api_with_status(error, provider, None) +} + +fn completion_error_from_anthropic_api_with_status( + error: ApiError, + provider: language_model_core::LanguageModelProviderName, + status: Option, ) -> language_model_core::LanguageModelCompletionError { use ApiErrorCode::*; use language_model_core::LanguageModelCompletionError as Error; - match error.code() { - Some(code) => match code { - InvalidRequestError => Error::BadRequestFormat { - provider, - message: error.message, - }, - AuthenticationError => Error::AuthenticationError { - provider, - message: error.message, - }, - BillingError => Error::PaymentRequired, - PermissionError => Error::PermissionError { - provider, - message: error.message, - }, - NotFoundError => Error::ApiEndpointNotFound { provider }, - ConflictError => Error::HttpResponseError { - provider, - status_code: StatusCode::CONFLICT, - message: error.message, - }, - RequestTooLarge => Error::PromptTooLarge { - tokens: language_model_core::parse_prompt_too_long(&error.message), - }, - RateLimitError => Error::RateLimitExceeded { - provider, - retry_after: None, - }, - TimeoutError => Error::UpstreamProviderError { - message: error.message, - status: StatusCode::GATEWAY_TIMEOUT, - retry_after: None, - }, - ApiError => Error::ApiInternalServerError { - provider, - message: error.message, - }, - OverloadedError => Error::ServerOverloaded { - provider, - retry_after: None, - }, + use language_model_core::ProviderErrorCategory; + let category = match error.code() { + Some(InvalidRequestError) => { + if let Some(tokens) = parse_prompt_too_long(&error.message) { + ProviderErrorCategory::PromptTooLarge { + tokens: Some(tokens), + } + } else { + ProviderErrorCategory::InvalidRequest + } + } + Some(AuthenticationError) => ProviderErrorCategory::Authentication, + Some(BillingError) => ProviderErrorCategory::PaymentRequired, + Some(PermissionError) => ProviderErrorCategory::Permission, + Some(NotFoundError) => ProviderErrorCategory::EndpointNotFound, + Some(ConflictError) => ProviderErrorCategory::Conflict, + Some(RequestTooLarge) => ProviderErrorCategory::PromptTooLarge { + tokens: parse_prompt_too_long(&error.message), }, - None => Error::Other(error.into()), - } + Some(RateLimitError) => ProviderErrorCategory::RateLimit, + Some(TimeoutError) => ProviderErrorCategory::Timeout, + Some(ApiError) => ProviderErrorCategory::InternalServer, + Some(OverloadedError) => ProviderErrorCategory::Overloaded, + None => ProviderErrorCategory::Other, + }; + Error::from_provider_response( + provider, + status, + Some(error.error_type), + error.message, + None, + category, + ) } #[cfg(test)] @@ -1309,10 +1339,13 @@ mod tests { assert!(matches!( error, - AnthropicError::ApiError(ApiError { - error_type, - message, - }) if error_type == "authentication_error" && message == "invalid x-api-key" + AnthropicError::ApiError { + status: Some(StatusCode::UNAUTHORIZED), + error: ApiError { + error_type, + message, + }, + } if error_type == "authentication_error" && message == "invalid x-api-key" )); } @@ -1362,12 +1395,36 @@ mod tests { assert!(matches!( completion_error, - language_model_core::LanguageModelCompletionError::PaymentRequired + language_model_core::LanguageModelCompletionError::ProviderRejection { + category: language_model_core::ProviderErrorCategory::PaymentRequired, + .. + } )); } #[test] - fn list_models_maps_anthropic_conflict_errors_to_http_conflict() { + fn request_too_large_preserves_reported_token_count() { + let error = completion_error_from_anthropic_api( + ApiError { + error_type: "request_too_large".to_string(), + message: "prompt is too long: 1500000 tokens".to_string(), + }, + language_model_core::ANTHROPIC_PROVIDER_NAME, + ); + + assert!(matches!( + error, + language_model_core::LanguageModelCompletionError::ProviderRejection { + category: language_model_core::ProviderErrorCategory::PromptTooLarge { + tokens: Some(1_500_000), + }, + .. + } + )); + } + + #[test] + fn list_models_preserves_anthropic_conflict_errors() { let client = FakeHttpClient::create(|_| async move { Ok(http::Response::builder() .status(StatusCode::CONFLICT) @@ -1388,11 +1445,15 @@ mod tests { assert!(matches!( completion_error, - language_model_core::LanguageModelCompletionError::HttpResponseError { + language_model_core::LanguageModelCompletionError::ProviderRejection { provider, - status_code: StatusCode::CONFLICT, + status: Some(StatusCode::CONFLICT), + code: Some(code), message, + retry_after: None, + category: language_model_core::ProviderErrorCategory::Conflict, } if provider == language_model_core::ANTHROPIC_PROVIDER_NAME + && code == "conflict_error" && message == "The resource was modified concurrently." )); } @@ -1419,11 +1480,16 @@ mod tests { assert!(matches!( completion_error, - language_model_core::LanguageModelCompletionError::UpstreamProviderError { + language_model_core::LanguageModelCompletionError::ProviderRejection { + provider, message, - status: StatusCode::GATEWAY_TIMEOUT, + status: Some(StatusCode::GATEWAY_TIMEOUT), + code: Some(code), retry_after: None, - } if message == "The request timed out." + category: language_model_core::ProviderErrorCategory::Timeout, + } if provider == language_model_core::ANTHROPIC_PROVIDER_NAME + && code == "timeout_error" + && message == "The request timed out." )); } diff --git a/crates/copilot_chat/src/model.rs b/crates/copilot_chat/src/model.rs index eb0f9d2eef..3d03bbe0b7 100644 --- a/crates/copilot_chat/src/model.rs +++ b/crates/copilot_chat/src/model.rs @@ -22,8 +22,8 @@ use language_model::{ LanguageModelCostInfo, LanguageModelEffortLevel, LanguageModelId, LanguageModelName, LanguageModelProviderId, LanguageModelProviderName, LanguageModelRequest, LanguageModelRequestMessage, LanguageModelToolChoice, LanguageModelToolResultContent, - LanguageModelToolSchemaFormat, LanguageModelToolUse, MessageContent, RateLimiter, Role, - StopReason, TokenUsage, + LanguageModelToolSchemaFormat, LanguageModelToolUse, MessageContent, ProviderErrorCategory, + RateLimiter, Role, StopReason, TokenUsage, }; use util::debug_panic; @@ -674,28 +674,38 @@ impl CopilotResponsesEventMapper { } copilot_responses::StreamEvent::Failed { response } => { - let provider = PROVIDER_NAME; - let (status_code, message) = match response.error { + let (code, message, category) = match response.error { Some(error) => { - let status_code = StatusCode::from_str(&error.code) - .unwrap_or(StatusCode::INTERNAL_SERVER_ERROR); - (status_code, error.message) + let category = category_from_copilot_error(&error.code, &error.message); + (Some(error.code), error.message, category) } None => ( - StatusCode::INTERNAL_SERVER_ERROR, + Some("response.failed".to_string()), "response.failed".to_string(), + ProviderErrorCategory::Other, ), }; - vec![Err(LanguageModelCompletionError::HttpResponseError { - provider, - status_code, + vec![Err(LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + None, + code, message, - })] + None, + category, + ))] } - copilot_responses::StreamEvent::GenericError { error } => vec![Err( - LanguageModelCompletionError::Other(anyhow!(error.message)), - )], + copilot_responses::StreamEvent::GenericError { error } => { + let category = category_from_copilot_error(&error.code, &error.message); + vec![Err(LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + None, + Some(error.code), + error.message, + None, + category, + ))] + } copilot_responses::StreamEvent::Created { .. } | copilot_responses::StreamEvent::Unknown => Vec::new(), @@ -738,6 +748,13 @@ impl CopilotResponsesEventMapper { } } +fn category_from_copilot_error(code: &str, message: &str) -> ProviderErrorCategory { + StatusCode::from_str(code) + .ok() + .map(|status| ProviderErrorCategory::from_http_status(status, message)) + .unwrap_or(ProviderErrorCategory::Other) +} + #[derive(serde::Serialize, serde::Deserialize)] struct CopilotResponseMessageMetadata { #[serde(default, skip_serializing_if = "Vec::is_empty")] @@ -1262,6 +1279,7 @@ mod tests { use super::*; use crate::responses; use futures::StreamExt; + use language_model::ProviderErrorCategory; use serde_json::json; fn map_events(events: Vec) -> Vec { @@ -1649,7 +1667,7 @@ mod tests { } #[test] - fn responses_stream_failed_maps_http_response_error() { + fn responses_stream_failed_maps_rate_limit_error() { let events = vec![responses::StreamEvent::Failed { response: responses::Response { error: Some(responses::ResponseError { @@ -1669,15 +1687,16 @@ mod tests { assert_eq!(mapped_results.len(), 1); match &mapped_results[0] { - Err(LanguageModelCompletionError::HttpResponseError { - status_code, - message, + Err(LanguageModelCompletionError::ProviderRejection { + provider, + retry_after, + category: ProviderErrorCategory::RateLimit, .. }) => { - assert_eq!(*status_code, http_client::StatusCode::TOO_MANY_REQUESTS); - assert_eq!(message, "too many requests"); + assert_eq!(provider, &PROVIDER_NAME); + assert_eq!(*retry_after, None); } - other => panic!("expected HttpResponseError, got {:?}", other), + other => panic!("expected RateLimit ProviderRejection, got {:?}", other), } } diff --git a/crates/language_model_core/src/language_model_core.rs b/crates/language_model_core/src/language_model_core.rs index db36ac7583..ca74ce0314 100644 --- a/crates/language_model_core/src/language_model_core.rs +++ b/crates/language_model_core/src/language_model_core.rs @@ -102,10 +102,79 @@ impl LanguageModelCompletionEvent { } } +/// Normalized semantic classification of a provider-originated rejection. +/// +/// Each language model provider reports failures with its own wire format +/// (Anthropic's `error.type` strings, OpenRouter's numeric codes, Zed cloud's +/// `upstream_http_*` codes, ...). Callers that need to react to a rejection's +/// meaning — deciding whether to retry, which message to show — shouldn't +/// have to match on every provider's raw vocabulary. Wire-level parsing stays +/// local to each provider crate, but is mapped once into this shared +/// category so the rest of Zed only needs to understand one vocabulary. +/// +/// A category does not replace `ProviderRejection`'s `status`, `code`, and +/// `message` fields: those are always preserved verbatim alongside it so a +/// caller can fall back to them for a category that doesn't need special +/// handling (`Other`) or for diagnostics. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ProviderErrorCategory { + /// The request exceeded the model's context window, optionally including + /// the provider-reported token count. + PromptTooLarge { + tokens: Option, + }, + /// The provider rejected previously-generated opaque content (e.g. + /// encrypted reasoning state) as invalid or unreadable. + InvalidEncryptedContent, + Authentication, + Permission, + EndpointNotFound, + PaymentRequired, + RateLimit, + Overloaded, + InvalidRequest, + Conflict, + Timeout, + InternalServer, + /// No known semantic applies; callers should fall back to `status`/`code`. + Other, +} + +impl ProviderErrorCategory { + /// Classifies a rejection from an HTTP status that the provider actually + /// returned, with message inspection for ambiguous bad-request responses. + pub fn from_http_status(status: StatusCode, message: &str) -> Self { + match status { + StatusCode::BAD_REQUEST if is_invalid_encrypted_content_message(message) => { + Self::InvalidEncryptedContent + } + StatusCode::BAD_REQUEST if is_context_window_exceeded_message(message) => { + Self::PromptTooLarge { tokens: None } + } + StatusCode::UNAUTHORIZED => Self::Authentication, + StatusCode::FORBIDDEN => Self::Permission, + StatusCode::NOT_FOUND => Self::EndpointNotFound, + StatusCode::PAYMENT_REQUIRED => Self::PaymentRequired, + StatusCode::PAYLOAD_TOO_LARGE => Self::PromptTooLarge { + tokens: parse_prompt_too_long(message), + }, + StatusCode::BAD_REQUEST => Self::InvalidRequest, + StatusCode::CONFLICT => Self::Conflict, + StatusCode::REQUEST_TIMEOUT | StatusCode::GATEWAY_TIMEOUT => Self::Timeout, + StatusCode::TOO_MANY_REQUESTS => Self::RateLimit, + StatusCode::SERVICE_UNAVAILABLE => Self::Overloaded, + // There is no `StatusCode` variant for the unofficial HTTP 529 + // ("the service is overloaded"), but providers such as + // Anthropic send it in practice. See https://http.dev/529 + status_code if status_code.as_u16() == 529 => Self::Overloaded, + status_code if status_code.is_server_error() => Self::InternalServer, + _ => Self::Other, + } + } +} + #[derive(Error, Debug)] pub enum LanguageModelCompletionError { - #[error("prompt too large for context window")] - PromptTooLarge { tokens: Option }, /// The model requires the user to consent to the upstream provider /// retaining inference logs (see `LanguageModel::requires_data_retention`) /// and that consent has not been given. @@ -116,55 +185,22 @@ pub enum LanguageModelCompletionError { DataRetentionConsentRequired { model_name: String }, #[error("missing {provider} API key")] NoApiKey { provider: LanguageModelProviderName }, - #[error("{provider}'s API rate limit exceeded")] - RateLimitExceeded { - provider: LanguageModelProviderName, - retry_after: Option, - }, - #[error("{provider}'s API servers are overloaded right now")] - ServerOverloaded { - provider: LanguageModelProviderName, - retry_after: Option, - }, - #[error("{provider}'s API server reported an internal server error: {message}")] - ApiInternalServerError { - provider: LanguageModelProviderName, - message: String, - }, + /// A rejection reported by the language model provider itself, as + /// opposed to a transport or plumbing failure on Zed's side. + /// + /// `status` and `code` are preserved verbatim from the provider's + /// response even when `category` already gives them a known meaning, so + /// callers that need the raw wire details (diagnostics, provider-specific + /// workarounds) never lose them. #[error("{message}")] - UpstreamProviderError { + ProviderRejection { + provider: LanguageModelProviderName, + status: Option, + code: Option, message: String, - status: StatusCode, retry_after: Option, + category: ProviderErrorCategory, }, - #[error("HTTP response error from {provider}'s API: status {status_code} - {message:?}")] - HttpResponseError { - provider: LanguageModelProviderName, - status_code: StatusCode, - message: String, - }, - #[error("invalid request format to {provider}'s API: {message}")] - BadRequestFormat { - provider: LanguageModelProviderName, - message: String, - }, - #[error("invalid encrypted content from {provider}'s API: {message}")] - InvalidEncryptedContent { - provider: LanguageModelProviderName, - message: String, - }, - #[error("authentication error with {provider}'s API: {message}")] - AuthenticationError { - provider: LanguageModelProviderName, - message: String, - }, - #[error("Permission error with {provider}'s API: {message}")] - PermissionError { - provider: LanguageModelProviderName, - message: String, - }, - #[error("language model provider API endpoint not found")] - ApiEndpointNotFound { provider: LanguageModelProviderName }, #[error("I/O error reading response from {provider}'s API")] ApiReadResponseError { provider: LanguageModelProviderName, @@ -198,8 +234,6 @@ pub enum LanguageModelCompletionError { }, #[error("stream from {provider} ended unexpectedly")] StreamEndedUnexpectedly { provider: LanguageModelProviderName }, - #[error("payment required to use this language model; please upgrade your account")] - PaymentRequired, #[error(transparent)] Other(#[from] anyhow::Error), } @@ -226,34 +260,66 @@ impl LanguageModelCompletionError { message: String, retry_after: Option, ) -> Self { - if let Some(tokens) = parse_prompt_too_long(&message) { - Self::PromptTooLarge { - tokens: Some(tokens), - } - } else if code == "upstream_http_error" { + if code == "upstream_http_error" { if let Some((upstream_status, inner_message)) = Self::parse_upstream_error_json(&message) { - return Self::from_http_status( + let category = + ProviderErrorCategory::from_http_status(upstream_status, &inner_message); + return Self::from_provider_response( upstream_provider, - upstream_status, + Some(upstream_status), + Some(code), inner_message, retry_after, + category, ); } - anyhow!("completion request failed, code: {code}, message: {message}").into() + let category = category_from_cloud_failure(&code, &message); + Self::from_provider_response( + upstream_provider, + None, + Some(code), + message, + retry_after, + category, + ) } else if let Some(status_code) = code .strip_prefix("upstream_http_") .and_then(|code| StatusCode::from_str(code).ok()) { - Self::from_http_status(upstream_provider, status_code, message, retry_after) + let category = ProviderErrorCategory::from_http_status(status_code, &message); + Self::from_provider_response( + upstream_provider, + Some(status_code), + Some(code), + message, + retry_after, + category, + ) } else if let Some(status_code) = code .strip_prefix("http_") .and_then(|code| StatusCode::from_str(code).ok()) { - Self::from_http_status(ZED_CLOUD_PROVIDER_NAME, status_code, message, retry_after) + let category = ProviderErrorCategory::from_http_status(status_code, &message); + Self::from_provider_response( + ZED_CLOUD_PROVIDER_NAME, + Some(status_code), + Some(code), + message, + retry_after, + category, + ) } else { - anyhow!("completion request failed, code: {code}, message: {message}").into() + let category = category_from_cloud_failure(&code, &message); + Self::from_provider_response( + upstream_provider, + None, + Some(code), + message, + retry_after, + category, + ) } } @@ -263,42 +329,129 @@ impl LanguageModelCompletionError { message: String, retry_after: Option, ) -> Self { - match status_code { - StatusCode::BAD_REQUEST => { - if is_invalid_encrypted_content_message(&message) { - Self::InvalidEncryptedContent { provider, message } - } else if is_context_window_exceeded_message(&message) { - Self::PromptTooLarge { tokens: None } - } else { - Self::BadRequestFormat { provider, message } - } - } - StatusCode::UNAUTHORIZED => Self::AuthenticationError { provider, message }, - StatusCode::FORBIDDEN => Self::PermissionError { provider, message }, - StatusCode::NOT_FOUND => Self::ApiEndpointNotFound { provider }, - StatusCode::PAYLOAD_TOO_LARGE => Self::PromptTooLarge { - tokens: parse_prompt_too_long(&message), - }, - StatusCode::TOO_MANY_REQUESTS => Self::RateLimitExceeded { - provider, - retry_after, - }, - StatusCode::INTERNAL_SERVER_ERROR => Self::ApiInternalServerError { provider, message }, - StatusCode::SERVICE_UNAVAILABLE => Self::ServerOverloaded { - provider, - retry_after, - }, - _ if status_code.as_u16() == 529 => Self::ServerOverloaded { - provider, - retry_after, - }, - _ => Self::HttpResponseError { - provider, - status_code, - message, - }, + let category = ProviderErrorCategory::from_http_status(status_code, &message); + Self::from_provider_response( + provider, + Some(status_code), + None, + message, + retry_after, + category, + ) + } + + /// Builds a [`LanguageModelCompletionError::ProviderRejection`] from raw + /// provider response fields and the category assigned by its adapter. + pub fn from_provider_response( + provider: LanguageModelProviderName, + status: Option, + code: Option, + message: String, + retry_after: Option, + category: ProviderErrorCategory, + ) -> Self { + Self::ProviderRejection { + provider, + status, + code, + message, + retry_after, + category, } } + + /// Returns the delay before a retry attempt, honoring a provider-supplied + /// delay before falling back to exponential backoff from five to forty + /// seconds. + /// + /// `attempt` is one-based. Provider rejections that are not classified as + /// transient, and error kinds without shared retry semantics, return + /// `None`. + pub fn retry_delay(&self, attempt: usize) -> Option { + if attempt == 0 { + 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() => + { + (*retry_after).or_else(|| exponential_backoff(attempt)) + } + Self::ApiReadResponseError { .. } | Self::HttpSend { .. } => { + exponential_backoff(attempt) + } + Self::DataRetentionConsentRequired { .. } + | Self::NoApiKey { .. } + | Self::ProviderRejection { .. } + | Self::SerializeRequest { .. } + | Self::BuildRequestBody { .. } + | Self::DeserializeResponse { .. } + | Self::StreamEndedUnexpectedly { .. } + | Self::Other(_) => None, + } + } +} + +fn category_from_cloud_failure(code: &str, message: &str) -> ProviderErrorCategory { + if let Some(tokens) = parse_prompt_too_long(message) { + return ProviderErrorCategory::PromptTooLarge { + tokens: Some(tokens), + }; + } + if is_context_window_exceeded_message(message) { + return ProviderErrorCategory::PromptTooLarge { tokens: None }; + } + if code == "invalid_encrypted_content" || is_invalid_encrypted_content_message(message) { + return ProviderErrorCategory::InvalidEncryptedContent; + } + + match code { + "context_length_exceeded" | "request_too_large" => { + ProviderErrorCategory::PromptTooLarge { tokens: None } + } + "invalid_request_error" => ProviderErrorCategory::InvalidRequest, + "authentication_error" => ProviderErrorCategory::Authentication, + "billing_error" | "payment_required_error" => ProviderErrorCategory::PaymentRequired, + "permission_error" => ProviderErrorCategory::Permission, + "not_found_error" => ProviderErrorCategory::EndpointNotFound, + "conflict_error" => ProviderErrorCategory::Conflict, + "rate_limit_error" | "rate_limit_exceeded" => ProviderErrorCategory::RateLimit, + "timeout_error" | "request_timed_out" => ProviderErrorCategory::Timeout, + "api_error" | "internal_server_error" => ProviderErrorCategory::InternalServer, + "overloaded_error" => ProviderErrorCategory::Overloaded, + _ => ProviderErrorCategory::Other, + } +} + +fn is_retryable_provider_status(status: StatusCode) -> bool { + status.is_server_error() || matches!(status.as_u16(), 408 | 425 | 429) +} + +fn exponential_backoff(attempt: usize) -> Option { + const INITIAL_BACKOFF: Duration = Duration::from_secs(5); + const MAXIMUM_BACKOFF: Duration = Duration::from_secs(40); + + let exponent = u32::try_from(attempt.checked_sub(1)?).unwrap_or(u32::MAX); + let multiplier = 2_u32.checked_pow(exponent).unwrap_or(u32::MAX); + Some( + INITIAL_BACKOFF + .checked_mul(multiplier) + .unwrap_or(MAXIMUM_BACKOFF) + .min(MAXIMUM_BACKOFF), + ) } fn is_invalid_encrypted_content_message(message: &str) -> bool { @@ -662,11 +815,15 @@ mod tests { ); match error { - LanguageModelCompletionError::ServerOverloaded { provider, .. } => { + LanguageModelCompletionError::ProviderRejection { + provider, + category: ProviderErrorCategory::Overloaded, + .. + } => { assert_eq!(provider.0, "anthropic"); } _ => panic!( - "Expected ServerOverloaded error for 503 status, got: {:?}", + "Expected Overloaded category for 503 status, got: {:?}", error ), } @@ -679,12 +836,22 @@ mod tests { ); match error { - LanguageModelCompletionError::ApiInternalServerError { provider, message } => { + LanguageModelCompletionError::ProviderRejection { + provider, + status, + code, + message, + category, + .. + } => { assert_eq!(provider.0, "anthropic"); + assert_eq!(status, Some(StatusCode::INTERNAL_SERVER_ERROR)); + assert_eq!(code.as_deref(), Some("upstream_http_error")); assert_eq!(message, "Internal server error"); + assert_eq!(category, ProviderErrorCategory::InternalServer); } _ => panic!( - "Expected ApiInternalServerError for 500 status, got: {:?}", + "Expected ProviderRejection for 500 status, got: {:?}", error ), } @@ -701,7 +868,10 @@ mod tests { assert!(matches!( error, - LanguageModelCompletionError::PromptTooLarge { tokens: None } + LanguageModelCompletionError::ProviderRejection { + category: ProviderErrorCategory::PromptTooLarge { tokens: None }, + .. + } )); let error = LanguageModelCompletionError::from_http_status( @@ -713,7 +883,11 @@ mod tests { assert!(matches!( error, - LanguageModelCompletionError::BadRequestFormat { .. } + LanguageModelCompletionError::ProviderRejection { + status: Some(StatusCode::BAD_REQUEST), + category: ProviderErrorCategory::InvalidRequest, + .. + } )); } @@ -729,9 +903,11 @@ mod tests { assert!(matches!( error, - LanguageModelCompletionError::InvalidEncryptedContent { + LanguageModelCompletionError::ProviderRejection { provider, message: error_message, + category: ProviderErrorCategory::InvalidEncryptedContent, + .. } if provider.0 == "OpenAI" && error_message == message )); } @@ -746,13 +922,120 @@ mod tests { ); match error { - LanguageModelCompletionError::ServerOverloaded { provider, .. } => { + LanguageModelCompletionError::ProviderRejection { + provider, + category: ProviderErrorCategory::Overloaded, + .. + } => { assert_eq!(provider.0, "anthropic"); } - _ => panic!("Expected ServerOverloaded error for upstream_http_503"), + _ => panic!("Expected Overloaded category for upstream_http_503"), } } + #[test] + fn test_from_cloud_failure_preserves_unknown_provider_rejection() { + let error = LanguageModelCompletionError::from_cloud_failure( + OPEN_AI_PROVIDER_NAME, + "cyber_policy".to_string(), + "This content was flagged as potentially violating our terms of use.".to_string(), + None, + ); + + assert!(matches!( + error, + LanguageModelCompletionError::ProviderRejection { + provider, + status: None, + code: Some(code), + message, + retry_after: None, + category: ProviderErrorCategory::Other, + } if provider == OPEN_AI_PROVIDER_NAME + && code == "cyber_policy" + && message == "This content was flagged as potentially violating our terms of use." + )); + } + + #[test] + fn test_provider_category_preserves_status_less_rejection_code() { + let error = LanguageModelCompletionError::from_provider_response( + ANTHROPIC_PROVIDER_NAME, + None, + Some("rate_limit_error".to_string()), + "Rate limit exceeded".to_string(), + None, + ProviderErrorCategory::RateLimit, + ); + + assert!(matches!( + error, + LanguageModelCompletionError::ProviderRejection { + status: None, + code: Some(code), + category: ProviderErrorCategory::RateLimit, + .. + } if code == "rate_limit_error" + )); + } + + #[test] + fn test_retry_delay_uses_provider_delay_or_bounded_exponential_backoff() { + let retry_after = Duration::from_secs(17); + let error = LanguageModelCompletionError::from_http_status( + ANTHROPIC_PROVIDER_NAME, + StatusCode::TOO_MANY_REQUESTS, + "Rate limit exceeded".to_string(), + Some(retry_after), + ); + assert_eq!(error.retry_delay(1), Some(retry_after)); + + let error = LanguageModelCompletionError::from_http_status( + ANTHROPIC_PROVIDER_NAME, + StatusCode::INTERNAL_SERVER_ERROR, + "Internal server error".to_string(), + None, + ); + 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))); + assert_eq!(error.retry_delay(4), Some(Duration::from_secs(40))); + assert_eq!(error.retry_delay(20), Some(Duration::from_secs(40))); + } + + #[test] + fn test_retry_delay_retries_statusless_transient_provider_errors() { + for category in [ + ProviderErrorCategory::Timeout, + ProviderErrorCategory::InternalServer, + ] { + let error = LanguageModelCompletionError::from_provider_response( + ANTHROPIC_PROVIDER_NAME, + None, + Some("provider_error".to_string()), + "Transient provider error".to_string(), + None, + category, + ); + + assert_eq!(error.retry_delay(1), Some(Duration::from_secs(5))); + } + } + + #[test] + fn test_retry_delay_rejects_permanent_provider_error() { + let error = LanguageModelCompletionError::from_provider_response( + OPEN_AI_PROVIDER_NAME, + None, + Some("cyber_policy".to_string()), + "This content was flagged as potentially violating our terms of use.".to_string(), + None, + ProviderErrorCategory::Other, + ); + + assert_eq!(error.retry_delay(1), None); + } + #[test] fn test_upstream_http_error_connection_timeout() { let error = LanguageModelCompletionError::from_cloud_failure( @@ -763,11 +1046,15 @@ mod tests { ); match error { - LanguageModelCompletionError::ServerOverloaded { provider, .. } => { + LanguageModelCompletionError::ProviderRejection { + provider, + category: ProviderErrorCategory::Overloaded, + .. + } => { assert_eq!(provider.0, "anthropic"); } _ => panic!( - "Expected ServerOverloaded error for connection timeout with 503 status, got: {:?}", + "Expected Overloaded category for connection timeout with 503 status, got: {:?}", error ), } @@ -780,15 +1067,25 @@ mod tests { ); match error { - LanguageModelCompletionError::ApiInternalServerError { provider, message } => { + LanguageModelCompletionError::ProviderRejection { + provider, + status, + code, + message, + category, + .. + } => { assert_eq!(provider.0, "anthropic"); + assert_eq!(status, Some(StatusCode::INTERNAL_SERVER_ERROR)); + assert_eq!(code.as_deref(), Some("upstream_http_error")); assert_eq!( message, "Received an error from the Anthropic API: upstream connect error or disconnect/reset before headers. reset reason: connection timeout" ); + assert_eq!(category, ProviderErrorCategory::InternalServer); } _ => panic!( - "Expected ApiInternalServerError for connection timeout with 500 status, got: {:?}", + "Expected ProviderRejection for connection timeout with 500 status, got: {:?}", error ), } diff --git a/crates/language_models/src/provider/bedrock.rs b/crates/language_models/src/provider/bedrock.rs index b4b503076d..e72880f3ae 100644 --- a/crates/language_models/src/provider/bedrock.rs +++ b/crates/language_models/src/provider/bedrock.rs @@ -45,8 +45,8 @@ use language_model::{ LanguageModelId, LanguageModelName, LanguageModelProvider, LanguageModelProviderId, LanguageModelProviderName, LanguageModelProviderState, LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolResultContent, LanguageModelToolSchemaFormat, - LanguageModelToolUse, MessageContent, ProviderSettingsView, RateLimiter, Role, - SubPageProviderSettings, TokenUsage, env_var, + LanguageModelToolUse, MessageContent, ProviderErrorCategory, ProviderSettingsView, RateLimiter, + Role, SubPageProviderSettings, TokenUsage, env_var, }; use open_ai::responses::Request as OpenAiResponseRequest; use open_ai::responses::{ResponseOutputItem, StreamEvent as OpenAiResponseStreamEvent}; @@ -981,36 +981,65 @@ impl LanguageModel for BedrockModel { let response = request.await.map_err(|err| match err { BedrockError::Validation(ref msg) => { if msg.contains("model identifier is invalid") { - LanguageModelCompletionError::Other(anyhow!( - "{display_name} is not available in {region}. \ + LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + None, + Some("ValidationException".to_string()), + format!( + "{display_name} is not available in {region}. \ Try switching to a region where this model is supported." - )) + ), + None, + ProviderErrorCategory::InvalidRequest, + ) } else { - LanguageModelCompletionError::BadRequestFormat { - provider: PROVIDER_NAME, - message: msg.clone(), - } + LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + None, + Some("ValidationException".to_string()), + msg.clone(), + None, + ProviderErrorCategory::InvalidRequest, + ) } } - BedrockError::RateLimited => LanguageModelCompletionError::RateLimitExceeded { - provider: PROVIDER_NAME, - retry_after: None, - }, + BedrockError::RateLimited => LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + None, + Some("ThrottlingException".to_string()), + "Bedrock request was throttled".to_string(), + None, + ProviderErrorCategory::RateLimit, + ), BedrockError::ServiceUnavailable => { - LanguageModelCompletionError::ServerOverloaded { - provider: PROVIDER_NAME, - retry_after: None, - } + LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + None, + Some("ServiceUnavailableException".to_string()), + "Bedrock service is temporarily unavailable".to_string(), + None, + ProviderErrorCategory::Overloaded, + ) + } + BedrockError::AccessDenied(msg) => { + LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + None, + Some("AccessDeniedException".to_string()), + msg, + None, + ProviderErrorCategory::Permission, + ) } - BedrockError::AccessDenied(msg) => LanguageModelCompletionError::PermissionError { - provider: PROVIDER_NAME, - message: msg, - }, BedrockError::InternalServer(msg) => { - LanguageModelCompletionError::ApiInternalServerError { - provider: PROVIDER_NAME, - message: msg, - } + LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + None, + Some("InternalServerException".to_string()), + msg, + None, + ProviderErrorCategory::InternalServer, + ) } other => LanguageModelCompletionError::Other(anyhow!(other)), })?; @@ -1081,16 +1110,20 @@ fn map_mantle_error(model: &MantleModel, error: RequestError) -> LanguageModelCo if let RequestError::HttpResponseError { status_code, .. } = &error && *status_code == http_client::http::StatusCode::FORBIDDEN { - return LanguageModelCompletionError::PermissionError { - provider: PROVIDER_NAME, - message: format!( + return LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + Some(http_client::http::StatusCode::FORBIDDEN), + None, + format!( "Bedrock Mantle denied this request for {}. Mantle-only models require IAM \ permissions for the `bedrock-mantle` endpoint (for example via the \ `AmazonBedrockMantleInferenceAccess` managed policy) in addition to whatever \ permissions your existing Bedrock credentials already have.", model.display_name() ), - }; + None, + ProviderErrorCategory::Permission, + ); } error.into() } diff --git a/crates/language_models_cloud/src/language_models_cloud.rs b/crates/language_models_cloud/src/language_models_cloud.rs index 9e41180e2e..c37d1144c8 100644 --- a/crates/language_models_cloud/src/language_models_cloud.rs +++ b/crates/language_models_cloud/src/language_models_cloud.rs @@ -24,8 +24,8 @@ use language_model::{ LanguageModelCompletionError, LanguageModelCompletionEvent, LanguageModelEffortLevel, LanguageModelId, LanguageModelName, LanguageModelProviderId, LanguageModelProviderName, LanguageModelRequest, LanguageModelToolChoice, LanguageModelToolSchemaFormat, - OPEN_AI_PROVIDER_ID, OPEN_AI_PROVIDER_NAME, RateLimiter, X_AI_PROVIDER_ID, X_AI_PROVIDER_NAME, - ZED_CLOUD_PROVIDER_ID, ZED_CLOUD_PROVIDER_NAME, + OPEN_AI_PROVIDER_ID, OPEN_AI_PROVIDER_NAME, ProviderErrorCategory, RateLimiter, + X_AI_PROVIDER_ID, X_AI_PROVIDER_NAME, ZED_CLOUD_PROVIDER_ID, ZED_CLOUD_PROVIDER_NAME, }; use schemars::JsonSchema; @@ -216,7 +216,15 @@ impl CloudLanguageModel { } if status == StatusCode::PAYMENT_REQUIRED { - return Err(LanguageModelCompletionError::PaymentRequired); + return Err(LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + Some(status), + None, + "payment required to use this language model; please upgrade your account" + .to_string(), + None, + ProviderErrorCategory::PaymentRequired, + )); } let mut body = String::new(); @@ -372,18 +380,27 @@ impl From for LanguageModelCompletionError { .unwrap_or(error.status) }; - return LanguageModelCompletionError::UpstreamProviderError { - message: cloud_error.message, - status, - retry_after: cloud_error.retry_after.map(Duration::from_secs_f64), - }; + let category = + ProviderErrorCategory::from_http_status(status, &cloud_error.message); + return LanguageModelCompletionError::from_provider_response( + PROVIDER_NAME, + Some(status), + Some(cloud_error.code), + cloud_error.message, + cloud_error.retry_after.map(Duration::from_secs_f64), + category, + ); } - return LanguageModelCompletionError::from_http_status( + let category = + ProviderErrorCategory::from_http_status(error.status, &cloud_error.message); + return LanguageModelCompletionError::from_provider_response( PROVIDER_NAME, - error.status, + Some(error.status), + Some(cloud_error.code), cloud_error.message, None, + category, ); } @@ -1236,7 +1253,8 @@ mod tests { use http_client::FakeHttpClient; use http_client::http::{HeaderMap, StatusCode}; use language_model::{ - LanguageModelCompletionError, LanguageModelRequestMessage, MessageContent, Role, Speed, + LanguageModelCompletionError, LanguageModelRequestMessage, MessageContent, + ProviderErrorCategory, Role, Speed, }; use serde_json::json; use std::sync::Mutex; @@ -1542,7 +1560,7 @@ mod tests { #[test] fn test_api_error_conversion_with_upstream_http_error() { - // upstream_http_error with 503 status should become ServerOverloaded + // upstream_http_error with 503 status should become an Overloaded rejection let error_body = r#"{"code":"upstream_http_error","message":"Received an error from the Anthropic API: upstream connect error or disconnect/reset before headers, reset reason: connection timeout","upstream_status":503}"#; let api_error = ApiError { @@ -1554,19 +1572,22 @@ mod tests { let completion_error: LanguageModelCompletionError = api_error.into(); match completion_error { - LanguageModelCompletionError::UpstreamProviderError { message, .. } => { - assert_eq!( - message, - "Received an error from the Anthropic API: upstream connect error or disconnect/reset before headers, reset reason: connection timeout" - ); + LanguageModelCompletionError::ProviderRejection { + provider, + retry_after, + category: ProviderErrorCategory::Overloaded, + .. + } => { + assert_eq!(provider, PROVIDER_NAME); + assert_eq!(retry_after, None); } _ => panic!( - "Expected UpstreamProviderError for upstream 503, got: {:?}", + "Expected Overloaded rejection for upstream 503, got: {:?}", completion_error ), } - // upstream_http_error with 500 status should become ApiInternalServerError + // upstream_http_error with 500 status should preserve the rejection let error_body = r#"{"code":"upstream_http_error","message":"Received an error from the OpenAI API: internal server error","upstream_status":500}"#; let api_error = ApiError { @@ -1578,19 +1599,31 @@ mod tests { let completion_error: LanguageModelCompletionError = api_error.into(); match completion_error { - LanguageModelCompletionError::UpstreamProviderError { message, .. } => { + LanguageModelCompletionError::ProviderRejection { + provider, + status, + code, + message, + retry_after, + category, + } => { + assert_eq!(provider, PROVIDER_NAME); + assert_eq!(status, Some(StatusCode::INTERNAL_SERVER_ERROR)); + assert_eq!(code.as_deref(), Some("upstream_http_error")); assert_eq!( message, "Received an error from the OpenAI API: internal server error" ); + assert_eq!(retry_after, None); + assert_eq!(category, ProviderErrorCategory::InternalServer); } _ => panic!( - "Expected UpstreamProviderError for upstream 500, got: {:?}", + "Expected ProviderRejection for upstream 500, got: {:?}", completion_error ), } - // upstream_http_error with 429 status should become RateLimitExceeded + // upstream_http_error with 429 status should become a RateLimit rejection let error_body = r#"{"code":"upstream_http_error","message":"Received an error from the Google API: rate limit exceeded","upstream_status":429}"#; let api_error = ApiError { @@ -1602,19 +1635,22 @@ mod tests { let completion_error: LanguageModelCompletionError = api_error.into(); match completion_error { - LanguageModelCompletionError::UpstreamProviderError { message, .. } => { - assert_eq!( - message, - "Received an error from the Google API: rate limit exceeded" - ); + LanguageModelCompletionError::ProviderRejection { + provider, + retry_after, + category: ProviderErrorCategory::RateLimit, + .. + } => { + assert_eq!(provider, PROVIDER_NAME); + assert_eq!(retry_after, None); } _ => panic!( - "Expected UpstreamProviderError for upstream 429, got: {:?}", + "Expected RateLimit rejection for upstream 429, got: {:?}", completion_error ), } - // Regular 500 error without upstream_http_error should remain ApiInternalServerError for Zed + // Regular 500 error without upstream_http_error should preserve the Zed rejection let error_body = "Regular internal server error"; let api_error = ApiError { @@ -1626,17 +1662,28 @@ mod tests { let completion_error: LanguageModelCompletionError = api_error.into(); match completion_error { - LanguageModelCompletionError::ApiInternalServerError { provider, message } => { + LanguageModelCompletionError::ProviderRejection { + provider, + status, + code, + message, + retry_after, + category, + } => { assert_eq!(provider, PROVIDER_NAME); + assert_eq!(status, Some(StatusCode::INTERNAL_SERVER_ERROR)); + assert_eq!(code, None); assert_eq!(message, "Regular internal server error"); + assert_eq!(retry_after, None); + assert_eq!(category, ProviderErrorCategory::InternalServer); } _ => panic!( - "Expected ApiInternalServerError for regular 500, got: {:?}", + "Expected ProviderRejection for regular 500, got: {:?}", completion_error ), } - // upstream_http_429 format should be converted to UpstreamProviderError + // upstream_http_429 format should be converted to a RateLimit rejection let error_body = r#"{"code":"upstream_http_429","message":"Upstream Anthropic rate limit exceeded.","retry_after":30.5}"#; let api_error = ApiError { @@ -1648,17 +1695,17 @@ mod tests { let completion_error: LanguageModelCompletionError = api_error.into(); match completion_error { - LanguageModelCompletionError::UpstreamProviderError { - message, - status, + LanguageModelCompletionError::ProviderRejection { + provider, retry_after, + category: ProviderErrorCategory::RateLimit, + .. } => { - assert_eq!(message, "Upstream Anthropic rate limit exceeded."); - assert_eq!(status, StatusCode::TOO_MANY_REQUESTS); + assert_eq!(provider, PROVIDER_NAME); assert_eq!(retry_after, Some(Duration::from_secs_f64(30.5))); } _ => panic!( - "Expected UpstreamProviderError for upstream_http_429, got: {:?}", + "Expected RateLimit rejection for upstream_http_429, got: {:?}", completion_error ), } @@ -1675,11 +1722,20 @@ mod tests { let completion_error: LanguageModelCompletionError = api_error.into(); match completion_error { - LanguageModelCompletionError::ApiInternalServerError { provider, .. } => { + LanguageModelCompletionError::ProviderRejection { + provider, + status, + code, + message, + .. + } => { assert_eq!(provider, PROVIDER_NAME); + assert_eq!(status, Some(StatusCode::INTERNAL_SERVER_ERROR)); + assert_eq!(code, None); + assert_eq!(message, "Not JSON at all"); } _ => panic!( - "Expected ApiInternalServerError for invalid JSON, got: {:?}", + "Expected ProviderRejection for invalid JSON, got: {:?}", completion_error ), } diff --git a/crates/open_ai/src/completion.rs b/crates/open_ai/src/completion.rs index d858fc419d..893a14ca5e 100644 --- a/crates/open_ai/src/completion.rs +++ b/crates/open_ai/src/completion.rs @@ -6,9 +6,9 @@ use language_model_core::{ LanguageModelCustomToolFormat, LanguageModelCustomToolGrammarSyntax, LanguageModelImage, LanguageModelProviderId, LanguageModelRequest, LanguageModelRequestMessage, LanguageModelRequestToolInput, LanguageModelToolChoice, LanguageModelToolResultContent, - LanguageModelToolUse, LanguageModelToolUseId, LanguageModelToolUseInput, MessageContent, Role, - StopReason, TokenUsage, - util::{fix_streamed_json, is_context_window_exceeded_message, parse_tool_arguments}, + LanguageModelToolUse, LanguageModelToolUseId, LanguageModelToolUseInput, MessageContent, + ProviderErrorCategory, Role, StopReason, TokenUsage, provider_name_for_id, + util::{fix_streamed_json, parse_tool_arguments}, }; use std::pin::Pin; use std::sync::Arc; @@ -1273,17 +1273,31 @@ impl OpenAiResponseEventMapper { events } ResponsesStreamEvent::Failed { response } => match response.error.as_ref() { - Some(error) => vec![Err(completion_error_from_response_error(error))], - None => vec![Err(LanguageModelCompletionError::Other(anyhow!( - response_failure_message(&response) - )))], + Some(error) => vec![Err(completion_error_from_response_error( + error, + provider_name_for_id(&self.compaction_state_owner), + ))], + None => vec![Err(LanguageModelCompletionError::from_provider_response( + provider_name_for_id(&self.compaction_state_owner), + None, + Some("response.failed".to_string()), + response_failure_message(&response), + None, + ProviderErrorCategory::Other, + ))], }, ResponsesStreamEvent::Error { error } => { - vec![Err(completion_error_from_response_error(&error))] + vec![Err(completion_error_from_response_error( + &error, + provider_name_for_id(&self.compaction_state_owner), + ))] } ResponsesStreamEvent::GenericError { error } => { let error = error.into_response_error(); - vec![Err(completion_error_from_response_error(&error))] + vec![Err(completion_error_from_response_error( + &error, + provider_name_for_id(&self.compaction_state_owner), + ))] } ResponsesStreamEvent::ReasoningSummaryPartAdded { item_id, @@ -1610,13 +1624,37 @@ fn response_failure_message(response: &ResponsesSummary) -> String { .unwrap_or_else(|| "response.failed".to_string()) } -fn completion_error_from_response_error(error: &ResponseError) -> LanguageModelCompletionError { - let message = response_error_message(error); - if is_context_window_exceeded_message(&message) { - LanguageModelCompletionError::PromptTooLarge { tokens: None } - } else { - LanguageModelCompletionError::Other(anyhow!(message)) - } +fn completion_error_from_response_error( + error: &ResponseError, + provider: language_model_core::LanguageModelProviderName, +) -> LanguageModelCompletionError { + let category = match error.code.as_deref() { + Some("context_length_exceeded" | "request_too_large") => { + ProviderErrorCategory::PromptTooLarge { tokens: None } + } + Some("invalid_encrypted_content") => ProviderErrorCategory::InvalidEncryptedContent, + Some("invalid_request_error") => ProviderErrorCategory::InvalidRequest, + Some("authentication_error") => ProviderErrorCategory::Authentication, + Some("billing_error" | "payment_required_error") => ProviderErrorCategory::PaymentRequired, + Some("permission_error") => ProviderErrorCategory::Permission, + Some("not_found_error") => ProviderErrorCategory::EndpointNotFound, + Some("conflict_error") => ProviderErrorCategory::Conflict, + Some("rate_limit_error" | "rate_limit_exceeded") => ProviderErrorCategory::RateLimit, + Some("timeout_error" | "request_timed_out") => ProviderErrorCategory::Timeout, + Some("api_error" | "internal_server_error" | "server_error") => { + ProviderErrorCategory::InternalServer + } + Some("overloaded_error") => ProviderErrorCategory::Overloaded, + Some(_) | None => ProviderErrorCategory::Other, + }; + LanguageModelCompletionError::from_provider_response( + provider, + None, + error.code.clone(), + error.message.clone(), + None, + category, + ) } fn response_error_message(error: &ResponseError) -> String { @@ -1705,8 +1743,8 @@ mod tests { LanguageModelCustomToolFormat, LanguageModelCustomToolGrammarSyntax, LanguageModelImage, LanguageModelRequestMessage, LanguageModelRequestTool, LanguageModelRequestToolInput, LanguageModelToolResult, LanguageModelToolResultContent, LanguageModelToolUse, - LanguageModelToolUseId, LanguageModelToolUseInput, OPEN_AI_PROVIDER_ID, SharedString, - Speed, + LanguageModelToolUseId, LanguageModelToolUseInput, OPEN_AI_PROVIDER_ID, + OPEN_AI_PROVIDER_NAME, ProviderErrorCategory, SharedString, Speed, }; use pretty_assertions::assert_eq; use serde_json::json; @@ -3075,7 +3113,7 @@ mod tests { } #[test] - fn responses_stream_failed_uses_response_error_message() { + fn responses_stream_failed_preserves_provider_rejection() { let mut mapper = OpenAiResponseEventMapper::new(OPEN_AI_PROVIDER_ID); let mapped = mapper.map_event(ResponsesStreamEvent::Failed { response: ResponseSummary { @@ -3091,10 +3129,19 @@ mod tests { assert_eq!(mapped.len(), 1); let error = mapped.into_iter().next().unwrap().unwrap_err(); - assert_eq!( - error.to_string(), - "server_error: The model failed to generate a response." - ); + assert!(matches!( + error, + LanguageModelCompletionError::ProviderRejection { + provider, + status: None, + code: Some(code), + message, + retry_after: None, + .. + } if provider == OPEN_AI_PROVIDER_NAME + && code == "server_error" + && message == "The model failed to generate a response." + )); } #[test] @@ -3113,11 +3160,23 @@ mod tests { assert_eq!(mapped.len(), 1); let error = mapped.into_iter().next().unwrap().unwrap_err(); - assert_eq!(error.to_string(), "ERR_SOMETHING: Something went wrong"); + assert!(matches!( + error, + LanguageModelCompletionError::ProviderRejection { + provider, + status: None, + code: Some(code), + message, + retry_after: None, + .. + } if provider == OPEN_AI_PROVIDER_NAME + && code == "ERR_SOMETHING" + && message == "Something went wrong" + )); } #[test] - fn responses_stream_deserializes_nested_error_event() { + fn responses_stream_preserves_nested_cyber_policy_rejection() { // In practice the Responses API often nests error fields under an // `error` object even though the public spec documents them at the top // level. Make sure we don't lose the message and code in that case. @@ -3125,8 +3184,8 @@ mod tests { "type": "error", "error": { "type": "invalid_request_error", - "code": "invalid_prompt", - "message": "Your prompt was flagged.", + "code": "cyber_policy", + "message": "This content was flagged as potentially violating our terms of use.", "param": "input" }, "sequence_number": 2 @@ -3138,10 +3197,19 @@ mod tests { assert_eq!(mapped.len(), 1); let error = mapped.into_iter().next().unwrap().unwrap_err(); - assert_eq!( - error.to_string(), - "invalid_prompt: Your prompt was flagged." - ); + assert!(matches!( + error, + LanguageModelCompletionError::ProviderRejection { + provider, + status: None, + code: Some(code), + message, + retry_after: None, + category: ProviderErrorCategory::Other, + } if provider == OPEN_AI_PROVIDER_NAME + && code == "cyber_policy" + && message == "This content was flagged as potentially violating our terms of use." + )); } #[test] @@ -3165,7 +3233,10 @@ mod tests { let error = mapped.into_iter().next().unwrap().unwrap_err(); assert!(matches!( error, - LanguageModelCompletionError::PromptTooLarge { tokens: None } + LanguageModelCompletionError::ProviderRejection { + category: ProviderErrorCategory::PromptTooLarge { tokens: None }, + .. + } )); } @@ -3188,7 +3259,10 @@ mod tests { let error = mapped.into_iter().next().unwrap().unwrap_err(); assert!(matches!( error, - LanguageModelCompletionError::PromptTooLarge { tokens: None } + LanguageModelCompletionError::ProviderRejection { + category: ProviderErrorCategory::PromptTooLarge { tokens: None }, + .. + } )); } @@ -3208,7 +3282,19 @@ mod tests { assert_eq!(mapped.len(), 1); let error = mapped.into_iter().next().unwrap().unwrap_err(); - assert_eq!(error.to_string(), "invalid_request_error: Invalid request."); + assert!(matches!( + error, + LanguageModelCompletionError::ProviderRejection { + provider, + status: None, + code: Some(code), + message, + retry_after: None, + .. + } if provider == OPEN_AI_PROVIDER_NAME + && code == "invalid_request_error" + && message == "Invalid request." + )); } #[test] diff --git a/crates/open_router/Cargo.toml b/crates/open_router/Cargo.toml index 84b08e3cf6..76e9a0d213 100644 --- a/crates/open_router/Cargo.toml +++ b/crates/open_router/Cargo.toml @@ -25,7 +25,6 @@ schemars = { workspace = true, optional = true } serde.workspace = true serde_json.workspace = true settings.workspace = true -strum.workspace = true thiserror.workspace = true [dev-dependencies] diff --git a/crates/open_router/src/open_router.rs b/crates/open_router/src/open_router.rs index 9add2fc672..36ac81a2b6 100644 --- a/crates/open_router/src/open_router.rs +++ b/crates/open_router/src/open_router.rs @@ -11,7 +11,6 @@ pub use settings::ModelMode; pub use settings::OpenRouterAvailableModel as AvailableModel; pub use settings::OpenRouterProvider as Provider; use std::{convert::TryFrom, io, time::Duration}; -use strum::EnumString; use thiserror::Error; pub const OPEN_ROUTER_API_URL: &str = "https://openrouter.ai/api/v1"; @@ -515,8 +514,10 @@ pub async fn stream_completion( Ok(ResponseStreamResult::Response(response)) => Some(Ok(response)), Ok(ResponseStreamResult::Error(OpenRouterErrorResponse { error })) => { Some(Err(OpenRouterError::ApiError(ApiError { - code: ApiErrorCode::from_status(error.code), + status: None, + code: error.code, message: error.message, + retry_after: None, }))) } Err(error) => Some(Err(OpenRouterError::DeserializeResponse(error))), @@ -621,33 +622,22 @@ pub async fn list_models( Ok(models) } else { - let code = ApiErrorCode::from_status(response.status().as_u16()); - + let status = response.status(); let error_response = match serde_json::from_str::(&body) { Ok(OpenRouterErrorResponse { error }) => error, Err(_) => OpenRouterErrorBody { - code: response.status().as_u16(), + code: status.as_u16(), message: body, metadata: None, }, }; - match code { - ApiErrorCode::RateLimitError => { - let retry_after = extract_retry_after(response.headers()); - Err(OpenRouterError::RateLimit { - retry_after: retry_after.unwrap_or_else(|| std::time::Duration::from_secs(60)), - }) - } - ApiErrorCode::OverloadedError => { - let retry_after = extract_retry_after(response.headers()); - Err(OpenRouterError::ServerOverloaded { retry_after }) - } - _ => Err(OpenRouterError::ApiError(ApiError { - code: code, - message: error_response.message, - })), - } + Err(OpenRouterError::ApiError(ApiError { + status: Some(status.as_u16()), + code: error_response.code, + message: error_response.message, + retry_after: retry_after_with_rate_limit_default(status, response.headers()), + })) } } @@ -665,12 +655,6 @@ pub enum OpenRouterError { /// Failed to read from response stream ReadResponse(io::Error), - /// Rate limit exceeded - RateLimit { retry_after: Duration }, - - /// Server overloaded - ServerOverloaded { retry_after: Option }, - /// API returned an error response ApiError(ApiError), @@ -689,7 +673,6 @@ impl OpenRouterError { else { return Self::ChatCompletion(error); }; - let code = ApiErrorCode::from_status(status_code.as_u16()); let error_response = match serde_json::from_str::(&body) { Ok(OpenRouterErrorResponse { error }) => error, Err(_) => OpenRouterErrorBody { @@ -698,23 +681,27 @@ impl OpenRouterError { metadata: None, }, }; - - match code { - ApiErrorCode::RateLimitError => Self::RateLimit { - retry_after: extract_retry_after(&headers) - .unwrap_or_else(|| std::time::Duration::from_secs(60)), - }, - ApiErrorCode::OverloadedError => Self::ServerOverloaded { - retry_after: extract_retry_after(&headers), - }, - _ => Self::ApiError(ApiError { - code, - message: error_response.message, - }), - } + Self::ApiError(ApiError { + status: Some(status_code.as_u16()), + code: error_response.code, + message: error_response.message, + retry_after: retry_after_with_rate_limit_default(status_code, &headers), + }) } } +/// OpenRouter reports a rate limit's reset time via `X-RateLimit-Reset` when +/// present, but omits it on some rate-limited responses; a minute is a +/// reasonable default backoff for those. +fn retry_after_with_rate_limit_default( + status: http_client::StatusCode, + headers: &http::HeaderMap, +) -> Option { + extract_retry_after(headers).or_else(|| { + (status == http_client::StatusCode::TOO_MANY_REQUESTS).then(|| Duration::from_secs(60)) + }) +} + #[derive(Debug, Serialize, Deserialize)] pub struct OpenRouterErrorBody { pub code: u16, @@ -731,63 +718,13 @@ pub struct OpenRouterErrorResponse { #[derive(Debug, Serialize, Deserialize, Error)] #[error("OpenRouter API Error: {code}: {message}")] pub struct ApiError { - pub code: ApiErrorCode, + /// The HTTP status remains distinct from the provider's numeric error code + /// because OpenRouter can report different values for them, and streaming + /// errors do not have their own HTTP response. + pub status: Option, + pub code: u16, pub message: String, -} - -/// An OpenROuter API error code. -/// -#[derive(Debug, PartialEq, Eq, Clone, Copy, EnumString, Serialize, Deserialize)] -#[strum(serialize_all = "snake_case")] -pub enum ApiErrorCode { - /// 400: Bad Request (invalid or missing params, CORS) - InvalidRequestError, - /// 401: Invalid credentials (OAuth session expired, disabled/invalid API key) - AuthenticationError, - /// 402: Your account or API key has insufficient credits. Add more credits and retry the request. - PaymentRequiredError, - /// 403: Your chosen model requires moderation and your input was flagged - PermissionError, - /// 408: Your request timed out - RequestTimedOut, - /// 429: You are being rate limited - RateLimitError, - /// 502: Your chosen model is down or we received an invalid response from it - ApiError, - /// 503: There is no available model provider that meets your routing requirements - OverloadedError, -} - -impl std::fmt::Display for ApiErrorCode { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let s = match self { - ApiErrorCode::InvalidRequestError => "invalid_request_error", - ApiErrorCode::AuthenticationError => "authentication_error", - ApiErrorCode::PaymentRequiredError => "payment_required_error", - ApiErrorCode::PermissionError => "permission_error", - ApiErrorCode::RequestTimedOut => "request_timed_out", - ApiErrorCode::RateLimitError => "rate_limit_error", - ApiErrorCode::ApiError => "api_error", - ApiErrorCode::OverloadedError => "overloaded_error", - }; - write!(f, "{s}") - } -} - -impl ApiErrorCode { - pub fn from_status(status: u16) -> Self { - match status { - 400 => ApiErrorCode::InvalidRequestError, - 401 => ApiErrorCode::AuthenticationError, - 402 => ApiErrorCode::PaymentRequiredError, - 403 => ApiErrorCode::PermissionError, - 408 => ApiErrorCode::RequestTimedOut, - 429 => ApiErrorCode::RateLimitError, - 502 => ApiErrorCode::ApiError, - 503 => ApiErrorCode::OverloadedError, - _ => ApiErrorCode::ApiError, - } - } + pub retry_after: Option, } // -- Conversions to `language_model_core` types -- @@ -806,14 +743,6 @@ impl From for language_model_core::LanguageModelCompletionError Self::DeserializeResponse { provider, error } } OpenRouterError::ReadResponse(error) => Self::ApiReadResponseError { provider, error }, - OpenRouterError::RateLimit { retry_after } => Self::RateLimitExceeded { - provider, - retry_after: Some(retry_after), - }, - OpenRouterError::ServerOverloaded { retry_after } => Self::ServerOverloaded { - provider, - retry_after, - }, OpenRouterError::ApiError(api_error) => api_error.into(), OpenRouterError::ChatCompletion(error) => error.into(), } @@ -822,43 +751,28 @@ impl From for language_model_core::LanguageModelCompletionError impl From for language_model_core::LanguageModelCompletionError { fn from(error: ApiError) -> Self { - use ApiErrorCode::*; + use language_model_core::ProviderErrorCategory; + let provider = language_model_core::LanguageModelProviderName::new("OpenRouter"); - match error.code { - InvalidRequestError => Self::BadRequestFormat { - provider, - message: error.message, - }, - AuthenticationError => Self::AuthenticationError { - provider, - message: error.message, - }, - PaymentRequiredError => Self::AuthenticationError { - provider, - message: format!("Payment required: {}", error.message), - }, - PermissionError => Self::PermissionError { - provider, - message: error.message, - }, - RequestTimedOut => Self::HttpResponseError { - provider, - status_code: http_client::StatusCode::REQUEST_TIMEOUT, - message: error.message, - }, - RateLimitError => Self::RateLimitExceeded { - provider, - retry_after: None, - }, - ApiError => Self::ApiInternalServerError { - provider, - message: error.message, - }, - OverloadedError => Self::ServerOverloaded { - provider, - retry_after: None, - }, - } + let status = error + .status + .and_then(|status| http_client::StatusCode::from_u16(status).ok()); + let category = http_client::StatusCode::from_u16(error.code) + .ok() + .map(|status| ProviderErrorCategory::from_http_status(status, &error.message)) + .filter(|category| *category != ProviderErrorCategory::Other) + .or_else(|| { + status.map(|status| ProviderErrorCategory::from_http_status(status, &error.message)) + }) + .unwrap_or(ProviderErrorCategory::Other); + Self::from_provider_response( + provider, + status, + Some(error.code.to_string()), + error.message, + error.retry_after, + category, + ) } } @@ -957,4 +871,76 @@ mod tests { } if provider.0 == "OpenRouter" && host == "openrouter.ai" )); } + + #[test] + fn provider_code_remains_distinct_from_http_status() { + let error = OpenRouterError::from_chat_completion_request_error( + open_ai::RequestError::HttpResponseError { + provider: "OpenRouter".to_string(), + status_code: http_client::StatusCode::INTERNAL_SERVER_ERROR, + body: r#"{"error":{"code":499,"message":"upstream provider failed"}}"#.to_string(), + headers: Box::default(), + }, + ); + let OpenRouterError::ApiError(api_error) = &error else { + panic!("expected ApiError, got {error:?}"); + }; + assert_eq!(api_error.status, Some(500)); + assert_eq!(api_error.code, 499); + + let completion_error = language_model_core::LanguageModelCompletionError::from(error); + assert!(matches!( + completion_error, + language_model_core::LanguageModelCompletionError::ProviderRejection { + status: Some(status), + code: Some(code), + category: language_model_core::ProviderErrorCategory::InternalServer, + .. + } if status == http_client::StatusCode::INTERNAL_SERVER_ERROR && code == "499" + )); + } + + #[test] + fn streaming_provider_code_does_not_become_http_status() { + let completion_error = language_model_core::LanguageModelCompletionError::from(ApiError { + status: None, + code: 402, + message: "Insufficient credits".to_string(), + retry_after: None, + }); + + assert!(matches!( + completion_error, + language_model_core::LanguageModelCompletionError::ProviderRejection { + status: None, + code: Some(code), + category: language_model_core::ProviderErrorCategory::PaymentRequired, + .. + } if code == "402" + )); + } + + #[test] + fn streaming_server_error_remains_retryable_without_http_status() { + let completion_error = language_model_core::LanguageModelCompletionError::from(ApiError { + status: None, + code: 502, + message: "Upstream provider failed".to_string(), + retry_after: None, + }); + + assert!(matches!( + &completion_error, + language_model_core::LanguageModelCompletionError::ProviderRejection { + status: None, + code: Some(code), + category: language_model_core::ProviderErrorCategory::InternalServer, + .. + } if code == "502" + )); + assert_eq!( + completion_error.retry_delay(1), + Some(Duration::from_secs(5)) + ); + } }