From 061862a0e20d54741000f534c3ea47d2fd08ee6a Mon Sep 17 00:00:00 2001 From: pakrym-oai Date: Fri, 24 Oct 2025 09:47:52 -0700 Subject: [PATCH] Add CodexHttpClient wrapper with request logging (#5564) ## Summary - wrap the default reqwest::Client inside a new CodexHttpClient/CodexRequestBuilder pair and log the HTTP method, URL, and status for each request - update the auth/model/provider plumbing to use the new builder helpers so headers and bearer auth continue to be applied consistently - add the shared `http` dependency that backs the header conversion helpers ## Testing - `CODEX_SANDBOX=seatbelt CODEX_SANDBOX_NETWORK_DISABLED=1 cargo test -p codex-core` - `CODEX_SANDBOX=seatbelt CODEX_SANDBOX_NETWORK_DISABLED=1 cargo test -p codex-chatgpt` - `CODEX_SANDBOX=seatbelt CODEX_SANDBOX_NETWORK_DISABLED=1 cargo test -p codex-tui` ------ https://chatgpt.com/codex/tasks/task_i_68fa5038c17483208b1148661c5873be --- codex-rs/Cargo.lock | 1 + codex-rs/Cargo.toml | 1 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/auth.rs | 7 +- codex-rs/core/src/chat_completions.rs | 3 +- codex-rs/core/src/client.rs | 10 +- codex-rs/core/src/default_client.rs | 133 ++++++++++++++++++++++- codex-rs/core/src/model_provider_info.rs | 12 +- 8 files changed, 148 insertions(+), 20 deletions(-) diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 449774bf..0d8522a6 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1075,6 +1075,7 @@ dependencies = [ "escargot", "eventsource-stream", "futures", + "http", "indexmap 2.10.0", "landlock", "libc", diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index c6e75dca..46a46e23 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -116,6 +116,7 @@ env_logger = "0.11.5" escargot = "0.5" eventsource-stream = "0.2.3" futures = { version = "0.3", default-features = false } +http = "1.3.1" icu_decimal = "2.0.0" icu_locale_core = "2.0.0" ignore = "0.4.23" diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index b05e4885..9b0e4339 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -34,6 +34,7 @@ dunce = { workspace = true } env-flags = { workspace = true } eventsource-stream = { workspace = true } futures = { workspace = true } +http = { workspace = true } indexmap = { workspace = true } libc = { workspace = true } mcp-types = { workspace = true } diff --git a/codex-rs/core/src/auth.rs b/codex-rs/core/src/auth.rs index 3dcc037b..30394e79 100644 --- a/codex-rs/core/src/auth.rs +++ b/codex-rs/core/src/auth.rs @@ -21,6 +21,7 @@ use codex_app_server_protocol::AuthMode; use codex_protocol::config_types::ForcedLoginMethod; use crate::config::Config; +use crate::default_client::CodexHttpClient; use crate::token_data::PlanType; use crate::token_data::TokenData; use crate::token_data::parse_id_token; @@ -32,7 +33,7 @@ pub struct CodexAuth { pub(crate) api_key: Option, pub(crate) auth_dot_json: Arc>>, pub(crate) auth_file: PathBuf, - pub(crate) client: reqwest::Client, + pub(crate) client: CodexHttpClient, } impl PartialEq for CodexAuth { @@ -180,7 +181,7 @@ impl CodexAuth { } } - fn from_api_key_with_client(api_key: &str, client: reqwest::Client) -> Self { + fn from_api_key_with_client(api_key: &str, client: CodexHttpClient) -> Self { Self { api_key: Some(api_key.to_owned()), mode: AuthMode::ApiKey, @@ -400,7 +401,7 @@ async fn update_tokens( async fn try_refresh_token( refresh_token: String, - client: &reqwest::Client, + client: &CodexHttpClient, ) -> std::io::Result { let refresh_request = RefreshRequest { client_id: CLIENT_ID, diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index 27937a48..ac1d7f9c 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -4,6 +4,7 @@ use crate::ModelProviderInfo; use crate::client_common::Prompt; use crate::client_common::ResponseEvent; use crate::client_common::ResponseStream; +use crate::default_client::CodexHttpClient; use crate::error::CodexErr; use crate::error::ConnectionFailedError; use crate::error::ResponseStreamFailed; @@ -36,7 +37,7 @@ use tracing::trace; pub(crate) async fn stream_chat_completions( prompt: &Prompt, model_family: &ModelFamily, - client: &reqwest::Client, + client: &CodexHttpClient, provider: &ModelProviderInfo, otel_event_manager: &OtelEventManager, ) -> Result { diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index 1bf1ad0d..1708df6a 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -39,6 +39,7 @@ use crate::client_common::ResponsesApiRequest; use crate::client_common::create_reasoning_param_for_request; use crate::client_common::create_text_param_for_request; use crate::config::Config; +use crate::default_client::CodexHttpClient; use crate::default_client::create_client; use crate::error::CodexErr; use crate::error::ConnectionFailedError; @@ -81,7 +82,7 @@ pub struct ModelClient { config: Arc, auth_manager: Option>, otel_event_manager: OtelEventManager, - client: reqwest::Client, + client: CodexHttpClient, provider: ModelProviderInfo, conversation_id: ConversationId, effort: Option, @@ -335,13 +336,6 @@ impl ModelClient { .headers() .get("cf-ray") .map(|v| v.to_str().unwrap_or_default().to_string()); - - debug!( - "Response status: {}, cf-ray: {:?}, version: {:?}", - resp.status(), - request_id, - resp.version() - ); } match res { diff --git a/codex-rs/core/src/default_client.rs b/codex-rs/core/src/default_client.rs index 42690e04..821a4c80 100644 --- a/codex-rs/core/src/default_client.rs +++ b/codex-rs/core/src/default_client.rs @@ -1,5 +1,12 @@ use crate::spawn::CODEX_SANDBOX_ENV_VAR; +use http::Error as HttpError; +use reqwest::IntoUrl; +use reqwest::Method; +use reqwest::Response; +use reqwest::header::HeaderName; use reqwest::header::HeaderValue; +use serde::Serialize; +use std::fmt::Display; use std::sync::LazyLock; use std::sync::Mutex; use std::sync::OnceLock; @@ -22,6 +29,125 @@ use std::sync::OnceLock; pub static USER_AGENT_SUFFIX: LazyLock>> = LazyLock::new(|| Mutex::new(None)); pub const DEFAULT_ORIGINATOR: &str = "codex_cli_rs"; pub const CODEX_INTERNAL_ORIGINATOR_OVERRIDE_ENV_VAR: &str = "CODEX_INTERNAL_ORIGINATOR_OVERRIDE"; + +#[derive(Clone, Debug)] +pub struct CodexHttpClient { + inner: reqwest::Client, +} + +impl CodexHttpClient { + fn new(inner: reqwest::Client) -> Self { + Self { inner } + } + + pub fn get(&self, url: U) -> CodexRequestBuilder + where + U: IntoUrl, + { + self.request(Method::GET, url) + } + + pub fn post(&self, url: U) -> CodexRequestBuilder + where + U: IntoUrl, + { + self.request(Method::POST, url) + } + + pub fn request(&self, method: Method, url: U) -> CodexRequestBuilder + where + U: IntoUrl, + { + let url_str = url.as_str().to_string(); + CodexRequestBuilder::new(self.inner.request(method.clone(), url), method, url_str) + } +} + +#[must_use = "requests are not sent unless `send` is awaited"] +#[derive(Debug)] +pub struct CodexRequestBuilder { + builder: reqwest::RequestBuilder, + method: Method, + url: String, +} + +impl CodexRequestBuilder { + fn new(builder: reqwest::RequestBuilder, method: Method, url: String) -> Self { + Self { + builder, + method, + url, + } + } + + fn map(self, f: impl FnOnce(reqwest::RequestBuilder) -> reqwest::RequestBuilder) -> Self { + Self { + builder: f(self.builder), + method: self.method, + url: self.url, + } + } + + pub fn header(self, key: K, value: V) -> Self + where + HeaderName: TryFrom, + >::Error: Into, + HeaderValue: TryFrom, + >::Error: Into, + { + self.map(|builder| builder.header(key, value)) + } + + pub fn bearer_auth(self, token: T) -> Self + where + T: Display, + { + self.map(|builder| builder.bearer_auth(token)) + } + + pub fn json(self, value: &T) -> Self + where + T: ?Sized + Serialize, + { + self.map(|builder| builder.json(value)) + } + + pub async fn send(self) -> Result { + match self.builder.send().await { + Ok(response) => { + let request_id = response + .headers() + .get("cf-ray") + .map(|v| v.to_str().unwrap_or_default().to_string()) + .unwrap_or_default(); + + let version = format!("{:?}", response.version()); + + tracing::debug!( + method = %self.method, + url = %self.url, + status = %response.status(), + request_id = %request_id, + version = %version, + "Request completed" + ); + + Ok(response) + } + Err(error) => { + let status = error.status(); + tracing::debug!( + method = %self.method, + url = %self.url, + status = status.map(|s| s.as_u16()), + error = %error, + "Request failed" + ); + Err(error) + } + } + } +} #[derive(Debug, Clone)] pub struct Originator { pub value: String, @@ -124,8 +250,8 @@ fn sanitize_user_agent(candidate: String, fallback: &str) -> String { } } -/// Create a reqwest client with default `originator` and `User-Agent` headers set. -pub fn create_client() -> reqwest::Client { +/// Create an HTTP client with default `originator` and `User-Agent` headers set. +pub fn create_client() -> CodexHttpClient { use reqwest::header::HeaderMap; let mut headers = HeaderMap::new(); @@ -140,7 +266,8 @@ pub fn create_client() -> reqwest::Client { builder = builder.no_proxy(); } - builder.build().unwrap_or_else(|_| reqwest::Client::new()) + let inner = builder.build().unwrap_or_else(|_| reqwest::Client::new()); + CodexHttpClient::new(inner) } fn is_sandboxed() -> bool { diff --git a/codex-rs/core/src/model_provider_info.rs b/codex-rs/core/src/model_provider_info.rs index 429a2606..8dc252aa 100644 --- a/codex-rs/core/src/model_provider_info.rs +++ b/codex-rs/core/src/model_provider_info.rs @@ -6,6 +6,8 @@ //! key. These override or extend the defaults at runtime. use crate::CodexAuth; +use crate::default_client::CodexHttpClient; +use crate::default_client::CodexRequestBuilder; use codex_app_server_protocol::AuthMode; use serde::Deserialize; use serde::Serialize; @@ -95,7 +97,7 @@ pub struct ModelProviderInfo { impl ModelProviderInfo { /// Construct a `POST` RequestBuilder for the given URL using the provided - /// reqwest Client applying: + /// [`CodexHttpClient`] applying: /// • provider-specific headers (static + env based) /// • Bearer auth header when an API key is available. /// • Auth token for OAuth. @@ -104,9 +106,9 @@ impl ModelProviderInfo { /// one produced by [`ModelProviderInfo::api_key`]. pub async fn create_request_builder<'a>( &'a self, - client: &'a reqwest::Client, + client: &'a CodexHttpClient, auth: &Option, - ) -> crate::error::Result { + ) -> crate::error::Result { let effective_auth = if let Some(secret_key) = &self.experimental_bearer_token { Some(CodexAuth::from_api_key(secret_key)) } else { @@ -187,9 +189,9 @@ impl ModelProviderInfo { } /// Apply provider-specific HTTP headers (both static and environment-based) - /// onto an existing `reqwest::RequestBuilder` and return the updated + /// onto an existing [`CodexRequestBuilder`] and return the updated /// builder. - fn apply_http_headers(&self, mut builder: reqwest::RequestBuilder) -> reqwest::RequestBuilder { + fn apply_http_headers(&self, mut builder: CodexRequestBuilder) -> CodexRequestBuilder { if let Some(extra) = &self.http_headers { for (k, v) in extra { builder = builder.header(k, v);