diff --git a/codex-rs/core/src/api_bridge.rs b/codex-rs/core/src/api_bridge.rs index 86aeaedda0a..f7aeb570bde 100644 --- a/codex-rs/core/src/api_bridge.rs +++ b/codex-rs/core/src/api_bridge.rs @@ -28,6 +28,7 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { status, body: message, url: None, + cf_ray: None, request_id: None, }), ApiError::InvalidRequest { message } => CodexErr::InvalidRequest(message), @@ -89,13 +90,14 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { CodexErr::RetryLimit(RetryLimitReachedError { status, - request_id: extract_request_id(headers.as_ref()), + request_id: extract_request_tracking_id(headers.as_ref()), }) } else { CodexErr::UnexpectedStatus(UnexpectedResponseError { status, body: body_text, url, + cf_ray: extract_header(headers.as_ref(), CF_RAY_HEADER), request_id: extract_request_id(headers.as_ref()), }) } @@ -115,6 +117,9 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { const MODEL_CAP_MODEL_HEADER: &str = "x-codex-model-cap-model"; const MODEL_CAP_RESET_AFTER_HEADER: &str = "x-codex-model-cap-reset-after-seconds"; +const REQUEST_ID_HEADER: &str = "x-request-id"; +const OAI_REQUEST_ID_HEADER: &str = "x-oai-request-id"; +const CF_RAY_HEADER: &str = "cf-ray"; #[cfg(test)] mod tests { @@ -149,15 +154,20 @@ mod tests { } } +fn extract_request_tracking_id(headers: Option<&HeaderMap>) -> Option { + extract_request_id(headers).or_else(|| extract_header(headers, CF_RAY_HEADER)) +} + fn extract_request_id(headers: Option<&HeaderMap>) -> Option { + extract_header(headers, REQUEST_ID_HEADER) + .or_else(|| extract_header(headers, OAI_REQUEST_ID_HEADER)) +} + +fn extract_header(headers: Option<&HeaderMap>, name: &str) -> Option { headers.and_then(|map| { - ["cf-ray", "x-request-id", "x-oai-request-id"] - .iter() - .find_map(|name| { - map.get(*name) - .and_then(|v| v.to_str().ok()) - .map(str::to_string) - }) + map.get(name) + .and_then(|value| value.to_str().ok()) + .map(str::to_string) }) } diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 953b11b8f87..fcc308f3801 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -567,10 +567,10 @@ impl ModelClientSession { Ok(stream) => { return Ok(map_response_stream(stream, self.state.otel_manager.clone())); } - Err(ApiError::Transport(TransportError::Http { status, .. })) - if status == StatusCode::UNAUTHORIZED => - { - handle_unauthorized(status, &mut auth_recovery).await?; + Err(ApiError::Transport( + unauthorized_transport @ TransportError::Http { status, .. }, + )) if status == StatusCode::UNAUTHORIZED => { + handle_unauthorized(unauthorized_transport, &mut auth_recovery).await?; continue; } Err(err) => return Err(map_api_error(err)), @@ -606,10 +606,10 @@ impl ModelClientSession { .await { Ok(connection) => connection, - Err(ApiError::Transport(TransportError::Http { status, .. })) - if status == StatusCode::UNAUTHORIZED => - { - handle_unauthorized(status, &mut auth_recovery).await?; + Err(ApiError::Transport( + unauthorized_transport @ TransportError::Http { status, .. }, + )) if status == StatusCode::UNAUTHORIZED => { + handle_unauthorized(unauthorized_transport, &mut auth_recovery).await?; continue; } Err(err) => return Err(map_api_error(err)), @@ -780,7 +780,7 @@ where /// When refresh succeeds, the caller should retry the API call; otherwise /// the mapped `CodexErr` is returned to the caller. async fn handle_unauthorized( - status: StatusCode, + transport: TransportError, auth_recovery: &mut Option, ) -> Result<()> { if let Some(recovery) = auth_recovery @@ -793,16 +793,7 @@ async fn handle_unauthorized( }; } - Err(map_unauthorized_status(status)) -} - -fn map_unauthorized_status(status: StatusCode) -> CodexErr { - map_api_error(ApiError::Transport(TransportError::Http { - status, - url: None, - headers: None, - body: None, - })) + Err(map_api_error(ApiError::Transport(transport))) } struct ApiTelemetry { diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 684d968162d..889335824ec 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -286,13 +286,42 @@ pub struct UnexpectedResponseError { pub status: StatusCode, pub body: String, pub url: Option, + pub cf_ray: Option, pub request_id: 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 { + fn display_body(&self) -> String { + if let Some(message) = self.extract_error_message() { + return message; + } + + let trimmed_body = self.body.trim(); + if trimmed_body.is_empty() { + return "Unknown error".to_string(); + } + + truncate_with_ellipsis(trimmed_body, UNEXPECTED_RESPONSE_BODY_MAX_BYTES) + } + + fn extract_error_message(&self) -> Option { + let json = serde_json::from_str::(&self.body).ok()?; + let message = json + .get("error") + .and_then(|error| error.get("message")) + .and_then(serde_json::Value::as_str)?; + let message = message.trim(); + if message.is_empty() { + None + } else { + Some(message.to_string()) + } + } + fn friendly_message(&self) -> Option { if self.status != StatusCode::FORBIDDEN { return None; @@ -307,6 +336,9 @@ impl UnexpectedResponseError { 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}")); } @@ -321,11 +353,14 @@ impl std::fmt::Display for UnexpectedResponseError { write!(f, "{friendly}") } else { let status = self.status; - let body = &self.body; + 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}")); } @@ -335,6 +370,21 @@ impl std::fmt::Display for UnexpectedResponseError { } impl std::error::Error for UnexpectedResponseError {} + +fn truncate_with_ellipsis(text: &str, max_bytes: usize) -> String { + if text.len() <= max_bytes { + return text.to_string(); + } + + let mut cut = max_bytes; + while !text.is_char_boundary(cut) { + cut = cut.saturating_sub(1); + } + let mut truncated = text[..cut].to_string(); + truncated.push_str("..."); + truncated +} + #[derive(Debug)] pub struct RetryLimitReachedError { pub status: StatusCode, @@ -952,15 +1002,14 @@ mod tests { body: "Cloudflare error: Sorry, you have been blocked" .to_string(), url: Some("http://example.com/blocked".to_string()), - request_id: Some("ray-id".to_string()), + cf_ray: Some("ray-id".to_string()), + request_id: 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}, request id: ray-id" - ) + format!("{CLOUDFLARE_BLOCKED_MESSAGE} (status {status}), url: {url}, cf-ray: ray-id") ); } @@ -970,6 +1019,7 @@ mod tests { status: StatusCode::FORBIDDEN, body: "plain text error".to_string(), url: Some("http://example.com/plain".to_string()), + cf_ray: None, request_id: None, }; let status = StatusCode::FORBIDDEN.to_string(); @@ -980,6 +1030,63 @@ mod tests { ); } + #[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(), + url: Some("https://chatgpt.com/backend-api/codex/responses".to_string()), + cf_ray: None, + request_id: Some("req-123".to_string()), + }; + let status = StatusCode::UNAUTHORIZED.to_string(); + assert_eq!( + err.to_string(), + format!( + "unexpected status {status}: Workspace is not authorized in this region., url: https://chatgpt.com/backend-api/codex/responses, request id: req-123" + ) + ); + } + + #[test] + fn unexpected_status_truncates_long_body_with_ellipsis() { + let long_body = "x".repeat(UNEXPECTED_RESPONSE_BODY_MAX_BYTES + 10); + let err = UnexpectedResponseError { + status: StatusCode::BAD_GATEWAY, + body: long_body, + url: Some("http://example.com/long".to_string()), + cf_ray: None, + request_id: Some("req-long".to_string()), + }; + let status = StatusCode::BAD_GATEWAY.to_string(); + let expected_body = format!("{}...", "x".repeat(UNEXPECTED_RESPONSE_BODY_MAX_BYTES)); + assert_eq!( + err.to_string(), + format!( + "unexpected status {status}: {expected_body}, url: http://example.com/long, request id: req-long" + ) + ); + } + + #[test] + fn unexpected_status_includes_cf_ray_and_request_id() { + let err = UnexpectedResponseError { + status: StatusCode::UNAUTHORIZED, + body: "plain text error".to_string(), + 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()), + }; + let status = StatusCode::UNAUTHORIZED.to_string(); + assert_eq!( + err.to_string(), + format!( + "unexpected status {status}: plain text error, url: https://chatgpt.com/backend-api/codex/responses, cf-ray: 9c81f9f18f2fa49d-LHR, request id: req-xyz" + ) + ); + } + #[test] fn usage_limit_reached_includes_hours_and_minutes() { let base = Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap();