From 9e5625d9cf636f4ef35aac7ca321a8be96f561ec Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 21 Jul 2026 07:03:12 +0000 Subject: [PATCH] Separate HTTP execution from request logging (#34476) ## What changed - Add an execution path that injects trace headers and sends a request without logging its result. - Extract response and error logging into reusable helpers while preserving the existing behavior of `HttpClient::execute`. GitOrigin-RevId: 3feebe3da041dd08f5033dd319233d8afe9054ec --- codex-rs/http-client/src/default_client.rs | 59 ++++++++++++++-------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/codex-rs/http-client/src/default_client.rs b/codex-rs/http-client/src/default_client.rs index 6a312b0e2f..153f9372d7 100644 --- a/codex-rs/http-client/src/default_client.rs +++ b/codex-rs/http-client/src/default_client.rs @@ -69,40 +69,55 @@ impl HttpClient { pub(crate) async fn execute( &self, - mut request: reqwest::Request, + request: reqwest::Request, ) -> Result { - request.headers_mut().extend(trace_headers()); let method = request.method().clone(); let url = request.url().to_string(); - match self.inner.execute(request).await { + match self.execute_without_request_logging(request).await { Ok(response) => { - if self.request_logging == RequestLogging::Enabled { - tracing::debug!( - method = %method, - url = %url, - status = %response.status(), - headers = ?response.headers(), - version = ?response.version(), - "Request completed" - ); - } + self.log_response(&method, &url, &response); Ok(response) } Err(error) => { - if self.request_logging == RequestLogging::Enabled { - tracing::debug!( - method = %method, - url = %url, - status = error.status().map(|status| status.as_u16()), - error = %error, - "Request failed" - ); - } + self.log_error(&method, &url, &error); Err(error) } } } + + pub(crate) async fn execute_without_request_logging( + &self, + mut request: reqwest::Request, + ) -> Result { + request.headers_mut().extend(trace_headers()); + self.inner.execute(request).await + } + + pub(crate) fn log_response(&self, method: &Method, url: &str, response: &reqwest::Response) { + if self.request_logging == RequestLogging::Enabled { + tracing::debug!( + method = %method, + url = %url, + status = %response.status(), + headers = ?response.headers(), + version = ?response.version(), + "Request completed" + ); + } + } + + pub(crate) fn log_error(&self, method: &Method, url: &str, error: &reqwest::Error) { + if self.request_logging == RequestLogging::Enabled { + tracing::debug!( + method = %method, + url = %url, + status = error.status().map(|status| status.as_u16()), + error = %error, + "Request failed" + ); + } + } } #[derive(Clone, Copy, Debug, PartialEq, Eq)]