diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 7494796a54..626411dbb3 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -3509,7 +3509,7 @@ impl CodexMessageProcessor { INVALID_REQUEST_ERROR_CODE, format!("failed to load rollout `{}`: {err}", rollout_path.display()), ), - CodexErr::InvalidRequest(message) => (INVALID_REQUEST_ERROR_CODE, message), + CodexErr::InvalidRequest(err) => (INVALID_REQUEST_ERROR_CODE, err.message), _ => (INTERNAL_ERROR_CODE, format!("error forking thread: {err}")), }; let error = JSONRPCErrorError { diff --git a/codex-rs/core/src/api_bridge.rs b/codex-rs/core/src/api_bridge.rs index 3b2024f58b..54db6dcb6a 100644 --- a/codex-rs/core/src/api_bridge.rs +++ b/codex-rs/core/src/api_bridge.rs @@ -10,6 +10,7 @@ use serde::Deserialize; use crate::auth::CodexAuth; use crate::error::CodexErr; +use crate::error::InvalidRequestError; use crate::error::RetryLimitReachedError; use crate::error::UnexpectedResponseError; use crate::error::UsageLimitReachedError; @@ -31,7 +32,7 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { cf_ray: None, request_id: None, }), - ApiError::InvalidRequest { message } => CodexErr::InvalidRequest(message), + ApiError::InvalidRequest { message } => CodexErr::InvalidRequest(message.into()), ApiError::Transport(transport) => match transport { TransportError::Http { status, @@ -60,7 +61,10 @@ pub(crate) fn map_api_error(err: ApiError) -> CodexErr { { CodexErr::InvalidImageRequest() } else { - CodexErr::InvalidRequest(body_text) + CodexErr::InvalidRequest(InvalidRequestError { + message: body_text, + request_id: extract_request_id(headers.as_ref()), + }) } } else if status == http::StatusCode::INTERNAL_SERVER_ERROR { CodexErr::InternalServerError @@ -217,6 +221,37 @@ mod tests { None ); } + + #[test] + fn map_api_error_preserves_request_id_for_bad_request() { + let mut headers = HeaderMap::new(); + headers.insert( + REQUEST_ID_HEADER, + http::HeaderValue::from_static("req_bad_123"), + ); + let body = serde_json::json!({ + "error": { + "message": "bad request" + } + }) + .to_string(); + + let err = map_api_error(ApiError::Transport(TransportError::Http { + status: http::StatusCode::BAD_REQUEST, + url: Some("http://example.com/v1/responses".to_string()), + headers: Some(headers), + body: Some(body), + })); + + let CodexErr::InvalidRequest(invalid_request) = err else { + panic!("expected CodexErr::InvalidRequest, got {err:?}"); + }; + assert_eq!( + invalid_request.message, + r#"{"error":{"message":"bad request"}}"# + ); + assert_eq!(invalid_request.request_id.as_deref(), Some("req_bad_123")); + } } fn extract_request_tracking_id(headers: Option<&HeaderMap>) -> Option { diff --git a/codex-rs/core/src/error.rs b/codex-rs/core/src/error.rs index 84a91fe47b..baa1cbdcd6 100644 --- a/codex-rs/core/src/error.rs +++ b/codex-rs/core/src/error.rs @@ -108,7 +108,7 @@ pub enum CodexErr { /// Invalid request. #[error("{0}")] - InvalidRequest(String), + InvalidRequest(InvalidRequestError), /// Invalid image. #[error("Image poisoning")] @@ -191,6 +191,22 @@ impl From for CodexErr { } } +#[derive(Debug, Error)] +#[error("{message}")] +pub struct InvalidRequestError { + pub message: String, + pub request_id: Option, +} + +impl From for InvalidRequestError { + fn from(message: String) -> Self { + Self { + message, + request_id: None, + } + } +} + impl CodexErr { pub fn is_retryable(&self) -> bool { match self { @@ -234,6 +250,7 @@ impl CodexErr { CodexErr::UnexpectedStatus(err) => err.request_id.as_deref(), CodexErr::RetryLimit(err) => err.request_id.as_deref(), CodexErr::ResponseStreamFailed(err) => err.request_id.as_deref(), + CodexErr::InvalidRequest(err) => err.request_id.as_deref(), CodexErr::TurnAborted | CodexErr::Interrupted | CodexErr::Stream(..) @@ -249,7 +266,6 @@ impl CodexErr { | CodexErr::UsageNotIncluded | CodexErr::QuotaExceeded | CodexErr::InvalidImageRequest() - | CodexErr::InvalidRequest(_) | CodexErr::RefreshTokenFailed(_) | CodexErr::UnsupportedOperation(_) | CodexErr::Sandbox(_) @@ -1106,6 +1122,21 @@ mod tests { ); } + #[test] + fn invalid_request_request_id_defaults_to_none() { + let err = CodexErr::InvalidRequest("plain invalid request".to_string().into()); + assert_eq!(err.request_id(), None); + } + + #[test] + fn invalid_request_request_id_is_preserved() { + let err = CodexErr::InvalidRequest(InvalidRequestError { + message: "invalid request".to_string(), + request_id: Some("req_invalid_123".to_string()), + }); + assert_eq!(err.request_id(), Some("req_invalid_123")); + } + #[test] fn usage_limit_reached_includes_hours_and_minutes() { let base = Utc.with_ymd_and_hms(2024, 1, 1, 0, 0, 0).unwrap(); diff --git a/codex-rs/core/src/memories/phase1.rs b/codex-rs/core/src/memories/phase1.rs index 7e1aa46136..db3d27af0f 100644 --- a/codex-rs/core/src/memories/phase1.rs +++ b/codex-rs/core/src/memories/phase1.rs @@ -454,7 +454,7 @@ mod job { }) .collect::>(); serde_json::to_string(&filtered).map_err(|err| { - CodexErr::InvalidRequest(format!("failed to serialize rollout memory: {err}")) + CodexErr::InvalidRequest(format!("failed to serialize rollout memory: {err}").into()) }) } } diff --git a/codex-rs/core/src/memory_trace.rs b/codex-rs/core/src/memory_trace.rs index 2e114b9aa4..3275884f60 100644 --- a/codex-rs/core/src/memory_trace.rs +++ b/codex-rs/core/src/memory_trace.rs @@ -54,11 +54,14 @@ pub async fn build_memories_from_trace_files( .summarize_memories(raw_memories, model_info, effort, otel_manager) .await?; if output.len() != prepared.len() { - return Err(CodexErr::InvalidRequest(format!( - "unexpected memory summarize output length: expected {}, got {}", - prepared.len(), - output.len() - ))); + return Err(CodexErr::InvalidRequest( + format!( + "unexpected memory summarize output length: expected {}, got {}", + prepared.len(), + output.len() + ) + .into(), + )); } Ok(prepared @@ -116,10 +119,9 @@ fn load_trace_items(path: &Path, text: &str) -> Result> { .filter(serde_json::Value::is_object) .collect::>(); if dict_items.is_empty() { - return Err(CodexErr::InvalidRequest(format!( - "no object items found in trace file: {}", - path.display() - ))); + return Err(CodexErr::InvalidRequest( + format!("no object items found in trace file: {}", path.display()).into(), + )); } return normalize_trace_items(dict_items, path); } @@ -145,10 +147,9 @@ fn load_trace_items(path: &Path, text: &str) -> Result> { } if parsed_items.is_empty() { - return Err(CodexErr::InvalidRequest(format!( - "no JSON items parsed from trace file: {}", - path.display() - ))); + return Err(CodexErr::InvalidRequest( + format!("no JSON items parsed from trace file: {}", path.display()).into(), + )); } normalize_trace_items(parsed_items, path) @@ -193,10 +194,13 @@ fn normalize_trace_items(items: Vec, path: &Path) -> Result> { } if normalized.is_empty() { - return Err(CodexErr::InvalidRequest(format!( - "no valid trace items after normalization: {}", - path.display() - ))); + return Err(CodexErr::InvalidRequest( + format!( + "no valid trace items after normalization: {}", + path.display() + ) + .into(), + )); } Ok(normalized) } diff --git a/codex-rs/core/src/realtime_conversation.rs b/codex-rs/core/src/realtime_conversation.rs index 656b590e57..49e00a2e7e 100644 --- a/codex-rs/core/src/realtime_conversation.rs +++ b/codex-rs/core/src/realtime_conversation.rs @@ -79,7 +79,9 @@ impl RealtimeHandoffState { output_text, }) .await - .map_err(|_| CodexErr::InvalidRequest("conversation is not running".to_string()))?; + .map_err(|_| { + CodexErr::InvalidRequest("conversation is not running".to_string().into()) + })?; Ok(()) } } @@ -183,7 +185,7 @@ impl RealtimeConversationManager { let Some(sender) = sender else { return Err(CodexErr::InvalidRequest( - "conversation is not running".to_string(), + "conversation is not running".to_string().into(), )); }; @@ -194,7 +196,7 @@ impl RealtimeConversationManager { Ok(()) } Err(TrySendError::Closed(_)) => Err(CodexErr::InvalidRequest( - "conversation is not running".to_string(), + "conversation is not running".to_string().into(), )), } } @@ -207,14 +209,13 @@ impl RealtimeConversationManager { let Some(sender) = sender else { return Err(CodexErr::InvalidRequest( - "conversation is not running".to_string(), + "conversation is not running".to_string().into(), )); }; - sender - .send(text) - .await - .map_err(|_| CodexErr::InvalidRequest("conversation is not running".to_string()))?; + sender.send(text).await.map_err(|_| { + CodexErr::InvalidRequest("conversation is not running".to_string().into()) + })?; Ok(()) } @@ -223,7 +224,7 @@ impl RealtimeConversationManager { let guard = self.state.lock().await; let Some(state) = guard.as_ref() else { return Err(CodexErr::InvalidRequest( - "conversation is not running".to_string(), + "conversation is not running".to_string().into(), )); }; state.handoff.clone() @@ -415,7 +416,9 @@ fn realtime_api_key( } Err(CodexErr::InvalidRequest( - "realtime conversation requires API key auth".to_string(), + "realtime conversation requires API key auth" + .to_string() + .into(), )) } @@ -432,7 +435,7 @@ fn realtime_request_headers( } let auth_value = HeaderValue::from_str(&format!("Bearer {api_key}")).map_err(|err| { - CodexErr::InvalidRequest(format!("invalid realtime api key header: {err}")) + CodexErr::InvalidRequest(format!("invalid realtime api key header: {err}").into()) })?; headers.insert(AUTHORIZATION, auth_value); diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 2c5333b09b..4a5a4edf23 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -212,11 +212,11 @@ async fn rollout_persists_request_id_when_responses_request_errors() -> anyhow:: skip_if_no_network!(Ok(())); let server = MockServer::start().await; - let response = ResponseTemplate::new(418) + let response = ResponseTemplate::new(400) .insert_header("x-request-id", "req_error_123") .set_body_json(json!({ "error": { - "message": "teapot" + "message": "bad request" } })); Mock::given(method("POST"))