From e65e480e0d67ce5a32667ff3e89143be035a0606 Mon Sep 17 00:00:00 2001 From: Celia Chen Date: Mon, 22 Jun 2026 17:53:09 -0700 Subject: [PATCH] chore: improve expired Bedrock credential errors (#28992) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Amazon Bedrock returns a `401 Unauthorized` response containing `Signature expired:` when an AWS credential, including a short-lived `AWS_BEARER_TOKEN_BEDROCK`, has expired. Codex currently surfaces that response as a generic `unexpected status` error, which does not explain how to recover. Environment-provided bearer tokens cannot be refreshed automatically, so the error should direct users to refresh their AWS credentials or replace or remove the environment token and restart Codex. This classification belongs to the Amazon Bedrock provider so similar responses from other providers retain their existing behavior. ## What changed - Add a synchronous `ModelProvider::map_api_error` hook that defaults to the existing provider-neutral API error mapping, and route model request, stream, WebSocket, and terminal unauthorized errors through the active provider. - Override the hook for Amazon Bedrock. After preserving the structured status, body, URL, and request metadata, recognize `401` responses containing `Signature expired:` and attach actionable credential guidance. - Keep `codex-protocol` provider-neutral by representing the guidance as an optional `user_message`. Error rendering prefers this message while continuing to append the URL, request ID, Cloudflare ray, and authorization diagnostics. - Add model-provider coverage for expired signatures and negative cases, core coverage for provider dispatch after unauthorized recovery, and a TUI snapshot for the rendered error. ## Testing Tested with a real request with expired bedrock key: Screenshot 2026-06-22 at 3 56 51 PM --- codex-rs/codex-api/src/api_bridge.rs | 36 ++++++--- codex-rs/codex-api/src/api_bridge_tests.rs | 28 +++++++ codex-rs/core/src/client.rs | 27 ++++--- codex-rs/core/src/client_tests.rs | 43 +++++++++++ .../src/amazon_bedrock/error.rs | 20 +++++ .../src/amazon_bedrock/error_tests.rs | 77 +++++++++++++++++++ .../model-provider/src/amazon_bedrock/mod.rs | 11 +++ codex-rs/model-provider/src/provider.rs | 7 ++ codex-rs/protocol/src/error.rs | 55 +++---------- codex-rs/protocol/src/error_tests.rs | 44 ++++++----- ...nt_bedrock_expired_signature_snapshot.snap | 5 ++ codex-rs/tui/src/history_cell/tests.rs | 27 +++++++ 12 files changed, 298 insertions(+), 82 deletions(-) create mode 100644 codex-rs/model-provider/src/amazon_bedrock/error.rs create mode 100644 codex-rs/model-provider/src/amazon_bedrock/error_tests.rs create mode 100644 codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__error_event_bedrock_expired_signature_snapshot.snap diff --git a/codex-rs/codex-api/src/api_bridge.rs b/codex-rs/codex-api/src/api_bridge.rs index 1c34d8bbf..ab1b409eb 100644 --- a/codex-rs/codex-api/src/api_bridge.rs +++ b/codex-rs/codex-api/src/api_bridge.rs @@ -23,15 +23,19 @@ pub fn map_api_error(err: ApiError) -> CodexErr { ApiError::Retryable { message, delay } => CodexErr::Stream(message, delay), ApiError::Stream(msg) => CodexErr::Stream(msg, None), ApiError::ServerOverloaded => CodexErr::ServerOverloaded, - ApiError::Api { status, message } => CodexErr::UnexpectedStatus(UnexpectedResponseError { - status, - body: message, - url: None, - cf_ray: None, - request_id: None, - identity_authorization_error: None, - identity_error_code: None, - }), + ApiError::Api { status, message } => { + let user_message = api_error_user_message(status, &message); + CodexErr::UnexpectedStatus(UnexpectedResponseError { + status, + body: message, + user_message, + url: None, + cf_ray: None, + request_id: None, + identity_authorization_error: None, + identity_error_code: None, + }) + } ApiError::InvalidRequest { message } => CodexErr::InvalidRequest(message), ApiError::CyberPolicy { message } => CodexErr::CyberPolicy { message }, ApiError::Transport(transport) => match transport { @@ -111,6 +115,7 @@ pub fn map_api_error(err: ApiError) -> CodexErr { } else { CodexErr::UnexpectedStatus(UnexpectedResponseError { status, + user_message: api_error_user_message(status, &body_text), body: body_text, url, cf_ray: extract_header(headers.as_ref(), CF_RAY_HEADER), @@ -145,6 +150,8 @@ const X_ERROR_JSON_HEADER: &str = "x-error-json"; const CYBER_POLICY_ERROR_CODE: &str = "cyber_policy"; const CYBER_POLICY_FALLBACK_MESSAGE: &str = "This request has been flagged for possible cybersecurity risk."; +const CLOUDFLARE_BLOCKED_MESSAGE: &str = + "Access blocked by Cloudflare. This usually happens when connecting from a restricted region"; #[cfg(test)] #[path = "api_bridge_tests.rs"] @@ -154,6 +161,17 @@ fn extract_request_tracking_id(headers: Option<&HeaderMap>) -> Option { extract_request_id(headers).or_else(|| extract_header(headers, CF_RAY_HEADER)) } +fn api_error_user_message(status: http::StatusCode, body: &str) -> Option { + if status == http::StatusCode::FORBIDDEN + && body.contains("Cloudflare") + && body.contains("blocked") + { + Some(format!("{CLOUDFLARE_BLOCKED_MESSAGE} (status {status})")) + } else { + None + } +} + fn extract_request_id(headers: Option<&HeaderMap>) -> Option { extract_header(headers, REQUEST_ID_HEADER) .or_else(|| extract_header(headers, OAI_REQUEST_ID_HEADER)) diff --git a/codex-rs/codex-api/src/api_bridge_tests.rs b/codex-rs/codex-api/src/api_bridge_tests.rs index 101e5566f..5812fc19f 100644 --- a/codex-rs/codex-api/src/api_bridge_tests.rs +++ b/codex-rs/codex-api/src/api_bridge_tests.rs @@ -26,6 +26,34 @@ fn map_api_error_maps_server_overloaded_from_503_body() { assert!(matches!(err, CodexErr::ServerOverloaded)); } +#[test] +fn map_api_error_maps_cloudflare_blocked_response_to_user_message() { + let mut headers = HeaderMap::new(); + headers.insert(CF_RAY_HEADER, http::HeaderValue::from_static("ray-id")); + let err = map_api_error(ApiError::Transport(TransportError::Http { + status: http::StatusCode::FORBIDDEN, + url: Some("http://example.com/blocked".to_string()), + headers: Some(headers), + body: Some( + "Cloudflare error: Sorry, you have been blocked".to_string(), + ), + })); + + let CodexErr::UnexpectedStatus(err) = err else { + panic!("expected CodexErr::UnexpectedStatus, got {err:?}"); + }; + assert_eq!( + err.user_message.as_deref(), + Some( + "Access blocked by Cloudflare. This usually happens when connecting from a restricted region (status 403 Forbidden)" + ) + ); + assert_eq!( + err.to_string(), + "Access blocked by Cloudflare. This usually happens when connecting from a restricted region (status 403 Forbidden), url: http://example.com/blocked, cf-ray: ray-id" + ); +} + #[test] fn map_api_error_maps_cyber_policy_from_400_body() { let body = serde_json::json!({ diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index c6cb92907..8ebea984d 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -112,7 +112,6 @@ use crate::feedback_tags; use crate::responses_metadata::CodexResponsesMetadata; use crate::responses_metadata::subagent_header_value; use crate::util::emit_feedback_auth_recovery_tags; -use codex_api::map_api_error; use codex_feedback::FeedbackRequestTags; use codex_feedback::emit_feedback_request_tags_with_auth_env; use codex_login::auth_env_telemetry::AuthEnvTelemetry; @@ -577,7 +576,7 @@ impl ModelClient { turn_state.as_deref(), ) .await - .map_err(map_api_error); + .map_err(|error| self.state.provider.map_api_error(error)); trace_attempt.record_result(result.as_deref()); result } @@ -604,7 +603,7 @@ impl ModelClient { let response = ApiRealtimeCallClient::new(transport, api_provider, client_setup.api_auth) .create_with_session_and_headers(sdp, session_config, extra_headers) .await - .map_err(map_api_error)?; + .map_err(|error| self.state.provider.map_api_error(error))?; Ok(RealtimeWebrtcCallStart { sdp: response.sdp, call_id: response.call_id, @@ -658,7 +657,7 @@ impl ModelClient { client .summarize_input(&payload, self.build_subagent_headers()) .await - .map_err(map_api_error) + .map_err(|error| self.state.provider.map_api_error(error)) } fn build_subagent_headers(&self) -> ApiHeaderMap { @@ -1337,6 +1336,7 @@ impl ModelClientSession { stream, session_telemetry.clone(), inference_trace_attempt, + Arc::clone(&self.client.state.provider), ); return Ok(stream); } @@ -1355,6 +1355,7 @@ impl ModelClientSession { unauthorized_transport, &mut auth_recovery, session_telemetry, + &self.client.state.provider, ) .await?, ); @@ -1363,7 +1364,7 @@ impl ModelClientSession { Err(err) => { let response_debug_context = extract_response_debug_context_from_api_error(&err); - let err = map_api_error(err); + let err = self.client.state.provider.map_api_error(err); inference_trace_attempt.record_failed( &err, response_debug_context.request_id.as_deref(), @@ -1469,12 +1470,13 @@ impl ModelClientSession { unauthorized_transport, &mut auth_recovery, session_telemetry, + &self.client.state.provider, ) .await?, ); continue; } - Err(err) => return Err(map_api_error(err)), + Err(err) => return Err(self.client.state.provider.map_api_error(err)), } let (mut ws_request, previous_response_id_from_untraced_warmup) = @@ -1503,7 +1505,7 @@ impl ModelClientSession { self.websocket_session.last_response_from_untraced_warmup = warmup; let websocket_connection = self.websocket_session.connection.as_ref().ok_or_else(|| { - map_api_error(ApiError::Stream( + self.client.state.provider.map_api_error(ApiError::Stream( "websocket connection is unavailable".to_string(), )) })?; @@ -1517,7 +1519,7 @@ impl ModelClientSession { .map_err(|err| { let response_debug_context = extract_response_debug_context_from_api_error(&err); - let err = map_api_error(err); + let err = self.client.state.provider.map_api_error(err); inference_trace_attempt.record_failed( &err, response_debug_context.request_id.as_deref(), @@ -1529,6 +1531,7 @@ impl ModelClientSession { stream_result, session_telemetry.clone(), inference_trace_attempt, + Arc::clone(&self.client.state.provider), ); self.websocket_session.last_response_rx = Some(last_request_rx); return Ok(WebsocketStreamOutcome::Stream(stream)); @@ -1761,6 +1764,7 @@ fn map_response_stream( api_stream: codex_api::ResponseStream, session_telemetry: SessionTelemetry, inference_trace_attempt: InferenceTraceAttempt, + provider: SharedModelProvider, ) -> (ResponseStream, oneshot::Receiver) { let codex_api::ResponseStream { rx_event, @@ -1775,6 +1779,7 @@ fn map_response_stream( api_stream, session_telemetry, inference_trace_attempt, + provider, ) } @@ -1783,6 +1788,7 @@ fn map_response_events( api_stream: S, session_telemetry: SessionTelemetry, inference_trace_attempt: InferenceTraceAttempt, + provider: SharedModelProvider, ) -> (ResponseStream, oneshot::Receiver) where S: futures::Stream> @@ -1893,7 +1899,7 @@ where if let Some(upstream_request_id) = upstream_request_id { feedback_tags!(last_model_request_id = upstream_request_id); } - let mapped = map_api_error(err); + let mapped = provider.map_api_error(err); inference_trace_attempt.record_failed( &mapped, upstream_request_id, @@ -1999,6 +2005,7 @@ async fn handle_unauthorized( transport: TransportError, auth_recovery: &mut Option, session_telemetry: &SessionTelemetry, + provider: &SharedModelProvider, ) -> Result { let debug = extract_response_debug_context(&transport); if let Some(recovery) = auth_recovery @@ -2108,7 +2115,7 @@ async fn handle_unauthorized( debug.auth_error_code.as_deref(), ); - Err(map_api_error(ApiError::Transport(transport))) + Err(provider.map_api_error(ApiError::Transport(transport))) } fn api_error_http_status(error: &ApiError) -> Option { diff --git a/codex-rs/core/src/client_tests.rs b/codex-rs/core/src/client_tests.rs index 0584e290f..9df7d4d01 100644 --- a/codex-rs/core/src/client_tests.rs +++ b/codex-rs/core/src/client_tests.rs @@ -15,10 +15,13 @@ use crate::test_support::TestCodexResponsesRequestKind; use crate::test_support::responses_metadata as test_responses_metadata; use codex_api::ApiError; use codex_api::ResponseEvent; +use codex_api::TransportError; use codex_app_server_protocol::AuthMode; use codex_login::AuthManager; use codex_login::CodexAuth; use codex_model_provider::BearerAuthProvider; +use codex_model_provider::SharedModelProvider; +use codex_model_provider::create_model_provider; use codex_model_provider_info::CHATGPT_CODEX_BASE_URL; use codex_model_provider_info::ModelProviderInfo; use codex_model_provider_info::WireApi; @@ -81,6 +84,10 @@ fn test_model_client(session_source: SessionSource) -> ModelClient { ) } +fn test_model_provider() -> SharedModelProvider { + test_model_client(SessionSource::Cli).state.provider.clone() +} + fn test_responses_metadata_for_client( client: &ModelClient, turn_id: Option<&str>, @@ -391,6 +398,7 @@ async fn dropped_response_stream_traces_cancelled_partial_output() -> anyhow::Re api_stream, test_session_telemetry(), attempt, + test_model_provider(), ); let observed = stream @@ -440,6 +448,7 @@ async fn response_stream_records_last_model_feedback_ids() { api_stream, test_session_telemetry(), InferenceTraceAttempt::disabled(), + test_model_provider(), ); while stream.next().await.is_some() {} @@ -455,6 +464,39 @@ async fn response_stream_records_last_model_feedback_ids() { ); } +#[tokio::test] +async fn bedrock_unauthorized_error_uses_provider_mapping() { + let provider = create_model_provider( + ModelProviderInfo::create_amazon_bedrock_provider(/*aws*/ None), + /*auth_manager*/ None, + ); + let mut auth_recovery = None; + let url = "https://bedrock-mantle.us-east-2.api.aws/openai/v1/responses"; + let error = super::handle_unauthorized( + TransportError::Http { + status: http::StatusCode::UNAUTHORIZED, + url: Some(url.to_string()), + headers: None, + body: Some( + "Signature expired: 20260609T133205Z is now earlier than 20260614T062525Z" + .to_string(), + ), + }, + &mut auth_recovery, + &test_session_telemetry(), + &provider, + ) + .await + .expect_err("expired Bedrock signature should fail"); + + assert_eq!( + error.to_string(), + format!( + "Amazon Bedrock rejected the request because its AWS signature has expired. Refresh your AWS credentials and retry. If `AWS_BEARER_TOKEN_BEDROCK` is set, update or unset it, then restart Codex, url: {url}" + ) + ); +} + #[tokio::test] async fn dropped_backpressured_response_stream_traces_cancelled_partial_output() -> anyhow::Result<()> { @@ -481,6 +523,7 @@ async fn dropped_backpressured_response_stream_traces_cancelled_partial_output() api_stream, test_session_telemetry(), attempt, + test_model_provider(), ); // Fill the mapper channel with non-terminal events, then yield one output diff --git a/codex-rs/model-provider/src/amazon_bedrock/error.rs b/codex-rs/model-provider/src/amazon_bedrock/error.rs new file mode 100644 index 000000000..1e489352b --- /dev/null +++ b/codex-rs/model-provider/src/amazon_bedrock/error.rs @@ -0,0 +1,20 @@ +use codex_api::ApiError; +use codex_protocol::error::CodexErr; +use http::StatusCode; + +pub(super) const BEDROCK_EXPIRED_SIGNATURE_MESSAGE: &str = concat!( + "Amazon Bedrock rejected the request because its AWS signature has expired. ", + "Refresh your AWS credentials and retry. If `AWS_BEARER_TOKEN_BEDROCK` is set, ", + "update or unset it, then restart Codex", +); + +pub(super) fn map_api_error(error: ApiError) -> CodexErr { + let mut error = codex_api::map_api_error(error); + if let CodexErr::UnexpectedStatus(response) = &mut error + && response.status == StatusCode::UNAUTHORIZED + && response.body.contains("Signature expired:") + { + response.user_message = Some(BEDROCK_EXPIRED_SIGNATURE_MESSAGE.to_string()); + } + error +} diff --git a/codex-rs/model-provider/src/amazon_bedrock/error_tests.rs b/codex-rs/model-provider/src/amazon_bedrock/error_tests.rs new file mode 100644 index 000000000..50291c3ee --- /dev/null +++ b/codex-rs/model-provider/src/amazon_bedrock/error_tests.rs @@ -0,0 +1,77 @@ +use codex_api::ApiError; +use codex_api::TransportError; +use codex_protocol::error::CodexErr; +use http::HeaderMap; +use http::HeaderValue; +use http::StatusCode; +use pretty_assertions::assert_eq; + +use super::error::BEDROCK_EXPIRED_SIGNATURE_MESSAGE; +use super::error::map_api_error; + +const BEDROCK_RESPONSES_URL: &str = "https://bedrock-mantle.us-east-2.api.aws/openai/v1/responses"; + +fn http_error(status: StatusCode, body: &str) -> ApiError { + let mut headers = HeaderMap::new(); + headers.insert("x-request-id", HeaderValue::from_static("req-bedrock")); + ApiError::Transport(TransportError::Http { + status, + url: Some(BEDROCK_RESPONSES_URL.to_string()), + headers: Some(headers), + body: Some(body.to_string()), + }) +} + +#[test] +fn expired_signature_has_actionable_guidance() { + let error = map_api_error(http_error( + StatusCode::UNAUTHORIZED, + "Signature expired: 20260609T133205Z is now earlier than 20260614T062525Z", + )); + + let CodexErr::UnexpectedStatus(response) = &error else { + panic!("expected unexpected status error, got {error:?}"); + }; + assert_eq!( + response.user_message.as_deref(), + Some(BEDROCK_EXPIRED_SIGNATURE_MESSAGE) + ); + assert_eq!( + error.to_string(), + format!( + "{BEDROCK_EXPIRED_SIGNATURE_MESSAGE}, url: {BEDROCK_RESPONSES_URL}, request id: req-bedrock" + ) + ); +} + +#[test] +fn other_unauthorized_errors_remain_generic() { + let error = map_api_error(http_error( + StatusCode::UNAUTHORIZED, + "The security token included in the request is invalid", + )); + + let CodexErr::UnexpectedStatus(response) = &error else { + panic!("expected unexpected status error, got {error:?}"); + }; + assert_eq!(response.user_message, None); + assert_eq!( + error.to_string(), + format!( + "unexpected status 401 Unauthorized: The security token included in the request is invalid, url: {BEDROCK_RESPONSES_URL}, request id: req-bedrock" + ) + ); +} + +#[test] +fn signature_errors_with_other_statuses_remain_generic() { + let error = map_api_error(http_error( + StatusCode::FORBIDDEN, + "Signature expired: old is now earlier than new", + )); + + let CodexErr::UnexpectedStatus(response) = &error else { + panic!("expected unexpected status error, got {error:?}"); + }; + assert_eq!(response.user_message, None); +} diff --git a/codex-rs/model-provider/src/amazon_bedrock/mod.rs b/codex-rs/model-provider/src/amazon_bedrock/mod.rs index 68c94fe4d..108308c1b 100644 --- a/codex-rs/model-provider/src/amazon_bedrock/mod.rs +++ b/codex-rs/model-provider/src/amazon_bedrock/mod.rs @@ -1,10 +1,12 @@ mod auth; mod catalog; +mod error; mod mantle; use std::path::PathBuf; use std::sync::Arc; +use codex_api::ApiError; use codex_api::Provider; use codex_api::SharedAuthProvider; use codex_login::AuthManager; @@ -17,6 +19,7 @@ use codex_models_manager::manager::SharedModelsManager; use codex_models_manager::manager::StaticModelsManager; use codex_protocol::account::AmazonBedrockCredentialSource; use codex_protocol::account::ProviderAccount; +use codex_protocol::error::CodexErr; use codex_protocol::error::Result; use codex_protocol::openai_models::ModelsResponse; @@ -142,6 +145,10 @@ impl ModelProvider for AmazonBedrockModelProvider { }) } + fn map_api_error(&self, error: ApiError) -> CodexErr { + error::map_api_error(error) + } + fn api_provider(&self) -> ModelProviderFuture<'_, Result> { Box::pin(AmazonBedrockModelProvider::api_provider(self)) } @@ -166,6 +173,10 @@ impl ModelProvider for AmazonBedrockModelProvider { } } +#[cfg(test)] +#[path = "error_tests.rs"] +mod error_tests; + #[cfg(test)] mod tests { use http::HeaderValue; diff --git a/codex-rs/model-provider/src/provider.rs b/codex-rs/model-provider/src/provider.rs index 3bd503136..5df5a88ea 100644 --- a/codex-rs/model-provider/src/provider.rs +++ b/codex-rs/model-provider/src/provider.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; use std::pin::Pin; use std::sync::Arc; +use codex_api::ApiError; use codex_api::Provider; use codex_api::SharedAuthProvider; use codex_login::AuthManager; @@ -13,6 +14,7 @@ use codex_models_manager::manager::OpenAiModelsManager; use codex_models_manager::manager::SharedModelsManager; use codex_models_manager::manager::StaticModelsManager; use codex_protocol::account::ProviderAccount; +use codex_protocol::error::CodexErr; use codex_protocol::openai_models::ModelsResponse; use crate::amazon_bedrock::AmazonBedrockModelProvider; @@ -142,6 +144,11 @@ pub trait ModelProvider: fmt::Debug + Send + Sync { /// Returns the current app-visible account state for this provider. fn account_state(&self) -> ProviderAccountResult; + /// Maps an API client error into the provider's user-facing error representation. + fn map_api_error(&self, error: ApiError) -> CodexErr { + codex_api::map_api_error(error) + } + /// Returns provider configuration adapted for the API client. fn api_provider(&self) -> ModelProviderFuture<'_, codex_protocol::error::Result> { Box::pin(async move { diff --git a/codex-rs/protocol/src/error.rs b/codex-rs/protocol/src/error.rs index d7e953af0..a2ea0eb69 100644 --- a/codex-rs/protocol/src/error.rs +++ b/codex-rs/protocol/src/error.rs @@ -305,6 +305,7 @@ impl std::fmt::Display for ResponseStreamFailed { pub struct UnexpectedResponseError { pub status: StatusCode, pub body: String, + pub user_message: Option, pub url: Option, pub cf_ray: Option, pub request_id: Option, @@ -312,8 +313,6 @@ pub struct UnexpectedResponseError { pub identity_error_code: Option, } -const CLOUDFLARE_BLOCKED_MESSAGE: &str = - "Access blocked by Cloudflare. This usually happens when connecting from a restricted region"; const UNEXPECTED_RESPONSE_BODY_MAX_BYTES: usize = 1000; impl UnexpectedResponseError { @@ -343,18 +342,17 @@ impl UnexpectedResponseError { Some(message.to_string()) } } +} - fn friendly_message(&self) -> Option { - if self.status != StatusCode::FORBIDDEN { - return None; - } - - if !self.body.contains("Cloudflare") || !self.body.contains("blocked") { - return None; - } - - let status = self.status; - let mut message = format!("{CLOUDFLARE_BLOCKED_MESSAGE} (status {status})"); +impl std::fmt::Display for UnexpectedResponseError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let mut message = if let Some(user_message) = &self.user_message { + user_message.clone() + } else { + let status = self.status; + let body = self.display_body(); + format!("unexpected status {status}: {body}") + }; if let Some(url) = &self.url { message.push_str(&format!(", url: {url}")); } @@ -370,36 +368,7 @@ impl UnexpectedResponseError { if let Some(error_code) = &self.identity_error_code { message.push_str(&format!(", auth error code: {error_code}")); } - - Some(message) - } -} - -impl std::fmt::Display for UnexpectedResponseError { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - if let Some(friendly) = self.friendly_message() { - write!(f, "{friendly}") - } else { - let status = self.status; - let body = self.display_body(); - let mut message = format!("unexpected status {status}: {body}"); - if let Some(url) = &self.url { - message.push_str(&format!(", url: {url}")); - } - if let Some(cf_ray) = &self.cf_ray { - message.push_str(&format!(", cf-ray: {cf_ray}")); - } - if let Some(id) = &self.request_id { - message.push_str(&format!(", request id: {id}")); - } - if let Some(auth_error) = &self.identity_authorization_error { - message.push_str(&format!(", auth error: {auth_error}")); - } - if let Some(error_code) = &self.identity_error_code { - message.push_str(&format!(", auth error code: {error_code}")); - } - write!(f, "{message}") - } + write!(f, "{message}") } } diff --git a/codex-rs/protocol/src/error_tests.rs b/codex-rs/protocol/src/error_tests.rs index bd7a5f005..0adbd029c 100644 --- a/codex-rs/protocol/src/error_tests.rs +++ b/codex-rs/protocol/src/error_tests.rs @@ -400,31 +400,12 @@ fn usage_limit_reached_includes_minutes_when_available() { }); } -#[test] -fn unexpected_status_cloudflare_html_is_simplified() { - let err = UnexpectedResponseError { - status: StatusCode::FORBIDDEN, - body: "Cloudflare error: Sorry, you have been blocked" - .to_string(), - url: Some("http://example.com/blocked".to_string()), - cf_ray: Some("ray-id".to_string()), - request_id: None, - identity_authorization_error: None, - identity_error_code: None, - }; - let status = StatusCode::FORBIDDEN.to_string(); - let url = "http://example.com/blocked"; - assert_eq!( - err.to_string(), - format!("{CLOUDFLARE_BLOCKED_MESSAGE} (status {status}), url: {url}, cf-ray: ray-id") - ); -} - #[test] fn unexpected_status_non_html_is_unchanged() { let err = UnexpectedResponseError { status: StatusCode::FORBIDDEN, body: "plain text error".to_string(), + user_message: None, url: Some("http://example.com/plain".to_string()), cf_ray: None, request_id: None, @@ -439,12 +420,32 @@ fn unexpected_status_non_html_is_unchanged() { ); } +#[test] +fn unexpected_status_uses_user_message_and_preserves_response_context() { + let err = UnexpectedResponseError { + status: StatusCode::UNAUTHORIZED, + body: "provider-specific response".to_string(), + user_message: Some("Provider-specific guidance".to_string()), + url: Some("https://example.com/v1/responses".to_string()), + cf_ray: None, + request_id: Some("req-provider".to_string()), + identity_authorization_error: None, + identity_error_code: None, + }; + + assert_eq!( + err.to_string(), + "Provider-specific guidance, url: https://example.com/v1/responses, request id: req-provider" + ); +} + #[test] fn unexpected_status_prefers_error_message_when_present() { let err = UnexpectedResponseError { status: StatusCode::UNAUTHORIZED, body: r#"{"error":{"message":"Workspace is not authorized in this region."},"status":401}"# .to_string(), + user_message: None, url: Some("https://chatgpt.com/backend-api/codex/responses".to_string()), cf_ray: None, request_id: Some("req-123".to_string()), @@ -466,6 +467,7 @@ fn unexpected_status_truncates_long_body_with_ellipsis() { let err = UnexpectedResponseError { status: StatusCode::BAD_GATEWAY, body: long_body, + user_message: None, url: Some("http://example.com/long".to_string()), cf_ray: None, request_id: Some("req-long".to_string()), @@ -487,6 +489,7 @@ fn unexpected_status_includes_cf_ray_and_request_id() { let err = UnexpectedResponseError { status: StatusCode::UNAUTHORIZED, body: "plain text error".to_string(), + user_message: None, url: Some("https://chatgpt.com/backend-api/codex/responses".to_string()), cf_ray: Some("9c81f9f18f2fa49d-LHR".to_string()), request_id: Some("req-xyz".to_string()), @@ -507,6 +510,7 @@ fn unexpected_status_includes_identity_auth_details() { let err = UnexpectedResponseError { status: StatusCode::UNAUTHORIZED, body: "plain text error".to_string(), + user_message: None, url: Some("https://chatgpt.com/backend-api/codex/models".to_string()), cf_ray: Some("cf-ray-auth-401-test".to_string()), request_id: Some("req-auth".to_string()), diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__error_event_bedrock_expired_signature_snapshot.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__error_event_bedrock_expired_signature_snapshot.snap new file mode 100644 index 000000000..3652ad442 --- /dev/null +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__error_event_bedrock_expired_signature_snapshot.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/history_cell/tests.rs +expression: rendered +--- +■ Amazon Bedrock rejected the request because its AWS signature has expired. Refresh your AWS credentials and retry. If `AWS_BEARER_TOKEN_BEDROCK` is set, update or unset it, then restart Codex, url: https://bedrock-mantle.us-east-2.api.aws/openai/v1/responses diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 4d2339212..99f7a3a93 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -15,11 +15,13 @@ use codex_otel::RuntimeMetricTotals; use codex_otel::RuntimeMetricsSummary; use codex_protocol::ThreadId; use codex_protocol::account::PlanType; +use codex_protocol::error::UnexpectedResponseError; use codex_protocol::parse_command::ParsedCommand; use dirs::home_dir; use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; use ratatui::layout::Rect; +use reqwest::StatusCode; use serde_json::json; use std::collections::HashMap; use std::path::PathBuf; @@ -752,6 +754,31 @@ fn error_event_oversized_input_snapshot() { insta::assert_snapshot!(rendered); } +#[test] +fn error_event_bedrock_expired_signature_snapshot() { + let error = UnexpectedResponseError { + status: StatusCode::UNAUTHORIZED, + body: "Signature expired: 20260609T133205Z is now earlier than 20260614T062525Z \ +(20260614T063025Z - 5 min.)" + .to_string(), + user_message: Some( + "Amazon Bedrock rejected the request because its AWS signature has expired. \ +Refresh your AWS credentials and retry. If `AWS_BEARER_TOKEN_BEDROCK` is set, update or \ +unset it, then restart Codex" + .to_string(), + ), + url: Some("https://bedrock-mantle.us-east-2.api.aws/openai/v1/responses".to_string()), + cf_ray: None, + request_id: None, + identity_authorization_error: None, + identity_error_code: None, + }; + let cell = new_error_event(error.to_string()); + let rendered = render_lines(&cell.display_lines(/*width*/ 100)).join("\n"); + + insta::assert_snapshot!(rendered); +} + #[tokio::test] async fn mcp_tools_output_masks_sensitive_values() { let mut config = test_config().await;