From d94d792a48465279db259238413daf9b8e58d032 Mon Sep 17 00:00:00 2001 From: Yeachan-Heo Date: Mon, 6 Apr 2026 00:10:24 +0000 Subject: [PATCH] Expose actionable ids for opaque provider failures Issue #22 was triggered by generic upstream fatal wrappers that only surfaced 'Something went wrong', which left repeated Jobdori-style failures opaque in the CLI. Capture provider request ids on error responses, classify the known generic wrapper as provider_internal, and prefix the user-visible runtime error with the failure class plus session/trace identifiers so operators can correlate the failure quickly. Constraint: Keep the fix small and user-safe without redesigning the broader runtime error taxonomy Constraint: Preserve existing non-generic error text unless the wrapper is the known opaque fatal surface Rejected: Broadly rewriting every runtime error into classified envelopes | unnecessary scope expansion for issue #22 Confidence: high Scope-risk: narrow Reversibility: clean Directive: If more opaque wrappers appear, extend the marker list and classification helper rather than reintroducing raw wrapper text alone Tested: cargo test -p api detects_generic_fatal_wrapper_and_classifies_it_as_provider_internal -- --nocapture; cargo test -p api retries_exhausted_preserves_nested_request_id_and_failure_class -- --nocapture; cargo test -p rusty-claude-cli opaque_provider_wrapper_surfaces_failure_class_session_and_trace -- --nocapture; cargo test -p rusty-claude-cli retry_exhaustion_preserves_internal_failure_class_for_generic_provider_wrapper -- --nocapture; cargo test --workspace Not-tested: Live upstream reproduction of the Jobdori failure against a real provider session --- rust/crates/api/src/error.rs | 137 +++++++++++++++++- rust/crates/api/src/providers/anthropic.rs | 2 + .../crates/api/src/providers/openai_compat.rs | 2 + rust/crates/rusty-claude-cli/src/main.rs | 99 ++++++++++--- 4 files changed, 221 insertions(+), 19 deletions(-) diff --git a/rust/crates/api/src/error.rs b/rust/crates/api/src/error.rs index 35c8da2..970e142 100644 --- a/rust/crates/api/src/error.rs +++ b/rust/crates/api/src/error.rs @@ -2,6 +2,11 @@ use std::env::VarError; use std::fmt::{Display, Formatter}; use std::time::Duration; +const GENERIC_FATAL_WRAPPER_MARKERS: &[&str] = &[ + "something went wrong while processing your request", + "please try again, or use /new to start a fresh session", +]; + #[derive(Debug)] pub enum ApiError { MissingCredentials { @@ -25,6 +30,7 @@ pub enum ApiError { status: reqwest::StatusCode, error_type: Option, message: Option, + request_id: Option, body: String, retryable: bool, }, @@ -65,6 +71,68 @@ impl ApiError { | Self::BackoffOverflow { .. } => false, } } + + #[must_use] + pub fn request_id(&self) -> Option<&str> { + match self { + Self::Api { request_id, .. } => request_id.as_deref(), + Self::RetriesExhausted { last_error, .. } => last_error.request_id(), + Self::MissingCredentials { .. } + | Self::ContextWindowExceeded { .. } + | Self::ExpiredOAuthToken + | Self::Auth(_) + | Self::InvalidApiKeyEnv(_) + | Self::Http(_) + | Self::Io(_) + | Self::Json(_) + | Self::InvalidSseFrame(_) + | Self::BackoffOverflow { .. } => None, + } + } + + #[must_use] + pub fn safe_failure_class(&self) -> &'static str { + match self { + Self::MissingCredentials { .. } | Self::ExpiredOAuthToken | Self::Auth(_) => { + "provider_auth" + } + Self::Api { status, .. } if matches!(status.as_u16(), 401 | 403) => "provider_auth", + Self::ContextWindowExceeded { .. } => "context_window", + Self::Api { status, .. } if status.as_u16() == 429 => "provider_rate_limit", + Self::Api { .. } | Self::RetriesExhausted { .. } if self.is_generic_fatal_wrapper() => { + "provider_internal" + } + Self::Api { .. } => "provider_error", + Self::Http(_) | Self::InvalidSseFrame(_) | Self::BackoffOverflow { .. } => { + "provider_transport" + } + Self::RetriesExhausted { .. } => "provider_retry_exhausted", + Self::InvalidApiKeyEnv(_) | Self::Io(_) | Self::Json(_) => "runtime_io", + } + } + + #[must_use] + pub fn is_generic_fatal_wrapper(&self) -> bool { + match self { + Self::Api { message, body, .. } => { + message + .as_deref() + .is_some_and(looks_like_generic_fatal_wrapper) + || looks_like_generic_fatal_wrapper(body) + } + Self::RetriesExhausted { last_error, .. } => last_error.is_generic_fatal_wrapper(), + Self::MissingCredentials { .. } + | Self::ContextWindowExceeded { .. } + | Self::ExpiredOAuthToken + | Self::Auth(_) + | Self::InvalidApiKeyEnv(_) + | Self::Http(_) + | Self::Io(_) + | Self::Json(_) + | Self::InvalidSseFrame(_) + | Self::BackoffOverflow { .. } => false, + } + } } impl Display for ApiError { @@ -102,13 +170,24 @@ impl Display for ApiError { status, error_type, message, + request_id, body, .. } => match (error_type, message) { (Some(error_type), Some(message)) => { - write!(f, "api returned {status} ({error_type}): {message}") + write!(f, "api returned {status} ({error_type})")?; + if let Some(request_id) = request_id { + write!(f, " [trace {request_id}]")?; + } + write!(f, ": {message}") + } + _ => { + write!(f, "api returned {status}")?; + if let Some(request_id) = request_id { + write!(f, " [trace {request_id}]")?; + } + write!(f, ": {body}") } - _ => write!(f, "api returned {status}: {body}"), }, Self::RetriesExhausted { attempts, @@ -151,3 +230,57 @@ impl From for ApiError { Self::InvalidApiKeyEnv(value) } } + +fn looks_like_generic_fatal_wrapper(text: &str) -> bool { + let lowered = text.to_ascii_lowercase(); + GENERIC_FATAL_WRAPPER_MARKERS + .iter() + .any(|marker| lowered.contains(marker)) +} + +#[cfg(test)] +mod tests { + use super::ApiError; + + #[test] + fn detects_generic_fatal_wrapper_and_classifies_it_as_provider_internal() { + let error = ApiError::Api { + status: reqwest::StatusCode::INTERNAL_SERVER_ERROR, + error_type: Some("api_error".to_string()), + message: Some( + "Something went wrong while processing your request. Please try again, or use /new to start a fresh session." + .to_string(), + ), + request_id: Some("req_jobdori_123".to_string()), + body: String::new(), + retryable: true, + }; + + assert!(error.is_generic_fatal_wrapper()); + assert_eq!(error.safe_failure_class(), "provider_internal"); + assert_eq!(error.request_id(), Some("req_jobdori_123")); + assert!(error.to_string().contains("[trace req_jobdori_123]")); + } + + #[test] + fn retries_exhausted_preserves_nested_request_id_and_failure_class() { + let error = ApiError::RetriesExhausted { + attempts: 3, + last_error: Box::new(ApiError::Api { + status: reqwest::StatusCode::BAD_GATEWAY, + error_type: Some("api_error".to_string()), + message: Some( + "Something went wrong while processing your request. Please try again, or use /new to start a fresh session." + .to_string(), + ), + request_id: Some("req_nested_456".to_string()), + body: String::new(), + retryable: true, + }), + }; + + assert!(error.is_generic_fatal_wrapper()); + assert_eq!(error.safe_failure_class(), "provider_internal"); + assert_eq!(error.request_id(), Some("req_nested_456")); + } +} diff --git a/rust/crates/api/src/providers/anthropic.rs b/rust/crates/api/src/providers/anthropic.rs index a398241..a85924b 100644 --- a/rust/crates/api/src/providers/anthropic.rs +++ b/rust/crates/api/src/providers/anthropic.rs @@ -808,6 +808,7 @@ async fn expect_success(response: reqwest::Response) -> Result(&body).ok(); let retryable = is_retryable_status(status); @@ -820,6 +821,7 @@ async fn expect_success(response: reqwest::Response) -> Result Result(&body).ok(); let retryable = is_retryable_status(status); @@ -918,6 +919,7 @@ async fn expect_success(response: reqwest::Response) -> Result = None; let mut saw_stop = false; - while let Some(event) = stream - .next_event() - .await - .map_err(|error| RuntimeError::new(error.to_string()))? - { + while let Some(event) = stream.next_event().await.map_err(|error| { + RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) + })? { match event { ApiStreamEvent::MessageStart(start) => { for block in start.message.content { @@ -5418,7 +5420,9 @@ impl ApiClient for AnthropicRuntimeClient { ..message_request.clone() }) .await - .map_err(|error| RuntimeError::new(error.to_string()))?; + .map_err(|error| { + RuntimeError::new(format_user_visible_api_error(&self.session_id, &error)) + })?; let mut events = response_to_events(response, out)?; push_prompt_cache_record(&self.client, &mut events); Ok(events) @@ -5426,6 +5430,23 @@ impl ApiClient for AnthropicRuntimeClient { } } +fn format_user_visible_api_error(session_id: &str, error: &api::ApiError) -> String { + if error.is_generic_fatal_wrapper() { + let mut qualifiers = vec![format!("session {session_id}")]; + if let Some(request_id) = error.request_id() { + qualifiers.push(format!("trace {request_id}")); + } + format!( + "{} ({}): {}", + error.safe_failure_class(), + qualifiers.join(", "), + error + ) + } else { + error.to_string() + } +} + fn final_assistant_text(summary: &runtime::TurnSummary) -> String { summary .assistant_messages @@ -6424,18 +6445,19 @@ mod tests { format_permissions_report, format_permissions_switch_report, format_pr_report, format_resume_report, format_status_report, format_tool_call_start, format_tool_result, format_ultraplan_report, format_unknown_slash_command, - format_unknown_slash_command_message, normalize_permission_mode, parse_args, - parse_git_status_branch, parse_git_status_metadata_for, parse_git_workspace_summary, - permission_policy, print_help_to, push_output_block, render_config_report, - render_diff_report, render_diff_report_for, render_memory_report, render_repl_help, - render_resume_usage, resolve_model_alias, resolve_session_reference, response_to_events, + format_unknown_slash_command_message, format_user_visible_api_error, + normalize_permission_mode, parse_args, parse_git_status_branch, + parse_git_status_metadata_for, parse_git_workspace_summary, permission_policy, + print_help_to, push_output_block, render_config_report, render_diff_report, + render_diff_report_for, render_memory_report, render_repl_help, render_resume_usage, + resolve_model_alias, resolve_session_reference, response_to_events, resume_supported_slash_commands, run_resume_command, slash_command_completion_candidates_with_sessions, status_context, validate_no_args, write_mcp_server_fixture, CliAction, CliOutputFormat, CliToolExecutor, GitWorkspaceSummary, InternalPromptProgressEvent, InternalPromptProgressState, LiveCli, LocalHelpTopic, SlashCommand, StatusUsage, DEFAULT_MODEL, }; - use api::{MessageResponse, OutputContentBlock, Usage}; + use api::{ApiError, MessageResponse, OutputContentBlock, Usage}; use plugins::{ PluginManager, PluginManagerConfig, PluginTool, PluginToolDefinition, PluginToolPermission, }; @@ -6475,6 +6497,49 @@ mod tests { .expect("plugin tool registry should build") } + #[test] + fn opaque_provider_wrapper_surfaces_failure_class_session_and_trace() { + let error = ApiError::Api { + status: "500".parse().expect("status"), + error_type: Some("api_error".to_string()), + message: Some( + "Something went wrong while processing your request. Please try again, or use /new to start a fresh session." + .to_string(), + ), + request_id: Some("req_jobdori_789".to_string()), + body: String::new(), + retryable: true, + }; + + let rendered = format_user_visible_api_error("session-issue-22", &error); + assert!(rendered.contains("provider_internal")); + assert!(rendered.contains("session session-issue-22")); + assert!(rendered.contains("trace req_jobdori_789")); + } + + #[test] + fn retry_exhaustion_uses_retry_failure_class_for_generic_provider_wrapper() { + let error = ApiError::RetriesExhausted { + attempts: 3, + last_error: Box::new(ApiError::Api { + status: "502".parse().expect("status"), + error_type: Some("api_error".to_string()), + message: Some( + "Something went wrong while processing your request. Please try again, or use /new to start a fresh session." + .to_string(), + ), + request_id: Some("req_jobdori_790".to_string()), + body: String::new(), + retryable: true, + }), + }; + + let rendered = format_user_visible_api_error("session-issue-22", &error); + assert!(rendered.contains("provider_retry_exhausted"), "{rendered}"); + assert!(rendered.contains("session session-issue-22")); + assert!(rendered.contains("trace req_jobdori_790")); + } + fn temp_dir() -> PathBuf { let nanos = SystemTime::now() .duration_since(UNIX_EPOCH)