From c8518b331739f072b6bbfc7cc8a4cb3bc6e990d9 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 24 Aug 2026 19:36:04 +0000 Subject: [PATCH] Preserve language model provider rejection details (#63095) Language model providers can reject a request inside an otherwise successful response stream. Those failures were represented by overlapping completion-error variants and provider-specific code enums, and OpenAI Responses errors such as `cyber_policy` fell through to `Other`. Agent sessions consequently retried permanent provider rejections and user interfaces lost the provider's actionable message. This gives every provider-originated failure one shared representation. `ProviderRejection` preserves the provider, exact message, retry delay, raw provider code, and an HTTP status only when one was actually reported. `ProviderErrorCategory` supplies the normalized meaning used by shared UI and policy code. Provider adapters map their native wire errors into that category explicitly, so typed SDK exceptions and errors embedded in successful streams no longer fabricate HTTP statuses. Retry selection is independent from error construction. `LanguageModelCompletionError::retry_delay` owns transient classification and a bounded schedule of 5, 10, 20, then 40 seconds. A provider-supplied `retry_after` overrides that schedule. The scheduling layer adds positive jitter of up to 10%, while interactive requests and evaluation runs retain their own attempt budgets. Permanent status-less errors such as `cyber_policy` do not retry. OpenRouter also keeps its numeric body code distinct from an optional HTTP status, including for errors embedded in a stream. Testing performed: - `corgi test -p language_model_core` - `corgi test -p anthropic` (33 tests) - `corgi test -p open_ai` (77 tests) - `cargo nextest run -p open_router -p copilot_chat -p language_models_cloud -p language_models` (125 tests) - `cargo nextest run -p agent -p agent_ui` (1,153 tests; 11 skipped) - `cargo check -p open_router -p copilot_chat -p language_models_cloud -p language_models -p agent -p agent_ui` - `./script/clippy -p language_model_core -p anthropic -p open_ai -p open_router -p copilot_chat -p language_models_cloud -p language_models -p agent -p agent_ui` completed the release Clippy phase; Cargo Shear then reported the repository's existing warnings - `cargo fmt --all --check` Suggested .rules additions None. Release Notes: - Fixed permanent language model provider rejections being retried and displayed without their actionable provider message. --- Cargo.lock | 1 - crates/agent/Cargo.toml | 2 +- crates/agent/src/agent.rs | 15 + crates/agent/src/tests/mod.rs | 121 +++-- crates/agent/src/thread.rs | 304 +++++++---- crates/agent/src/tools/evals.rs | 10 +- crates/agent/src/tools/evals/edit_file.rs | 64 +-- crates/agent/src/tools/evals/terminal_tool.rs | 62 +-- crates/agent/src/tools/evals/write_file.rs | 64 +-- crates/agent_ui/src/conversation_view.rs | 80 ++- .../src/conversation_view/thread_view.rs | 20 +- crates/anthropic/src/anthropic.rs | 216 +++++--- crates/copilot_chat/src/model.rs | 63 ++- .../src/language_model_core.rs | 511 ++++++++++++++---- .../language_models/src/provider/bedrock.rs | 91 +++- .../src/language_models_cloud.rs | 136 +++-- crates/open_ai/src/completion.rs | 154 ++++-- crates/open_router/Cargo.toml | 1 - crates/open_router/src/open_router.rs | 270 +++++---- 19 files changed, 1392 insertions(+), 793 deletions(-) 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)) + ); + } }