From 4bc83aa5f7ebfec8492bb5747affe0fc8433fcad Mon Sep 17 00:00:00 2001 From: Abhinav Date: Fri, 9 Oct 2026 21:19:27 +0000 Subject: [PATCH] Retry bootstrap GETs through the system proxy after request failures (#52702) ## Why Account discovery and cloud configuration GETs can fail after connecting but before receiving response headers. These request failures previously bypassed system proxy fallback. ## What changed Allow bootstrap GETs to retry through the system proxy on request errors, alongside the existing connection failure and timeout cases, when fallback is enabled. ## Testing Add regression coverage for connections closed before response headers on `accounts/check` and `config/bundle`, verifying successful proxy retries with the original request headers. Extend coverage to ensure HTTP responses with status `401`, `403`, `407`, `429`, or `503` do not trigger fallback, and network policy denials prevent requests to both the origin and proxy. GitOrigin-RevId: ac34c3bcfe3bd81c905a7e616d8b821879fbe2c9 --- codex-rs/backend-client/src/client.rs | 4 +- .../src/client_request_tests.rs | 92 ++++++++++++++++++- 2 files changed, 93 insertions(+), 3 deletions(-) diff --git a/codex-rs/backend-client/src/client.rs b/codex-rs/backend-client/src/client.rs index 54bf0ff48d..21ca66cceb 100644 --- a/codex-rs/backend-client/src/client.rs +++ b/codex-rs/backend-client/src/client.rs @@ -360,7 +360,9 @@ impl Client { Ok(Err(RequestError::Other(error))) if error .downcast_ref::() - .is_some_and(|error| error.is_connect() || error.is_timeout()) => {} + .is_some_and(|error| { + error.is_connect() || error.is_timeout() || error.is_request() + }) => {} Err(_) => {} Ok(response) => return response, } diff --git a/codex-rs/backend-client/src/client_request_tests.rs b/codex-rs/backend-client/src/client_request_tests.rs index 0d699587e7..0c0943699c 100644 --- a/codex-rs/backend-client/src/client_request_tests.rs +++ b/codex-rs/backend-client/src/client_request_tests.rs @@ -244,7 +244,7 @@ async fn bootstrap_gets_keep_default_responses_without_proxy_retry() { ) { return; } - for status in [200, 401] { + for status in [200, 401, 403, 407, 429, 503] { let issuer = MockServer::start().await; let proxy = MockServer::start().await; for endpoint in ["config/bundle", "accounts/check"] { @@ -270,7 +270,7 @@ async fn bootstrap_gets_keep_default_responses_without_proxy_retry() { bundle.expect_err("cloud status error").status(), accounts.expect_err("accounts status error").status(), ], - [Some(http::StatusCode::UNAUTHORIZED); 2] + [Some(http::StatusCode::from_u16(status).expect("valid test status")); 2] ); } issuer.verify().await; @@ -299,6 +299,94 @@ async fn bootstrap_gets_honor_disabled_fallback() { assert_eq!(proxy.received_requests().await.unwrap_or_default().len(), 0); } +#[tokio::test] +async fn bootstrap_gets_preserve_network_policy_with_fallback_enabled() { + let issuer = MockServer::start().await; + let proxy = MockServer::start().await; + for endpoint in ["config/bundle", "accounts/check"] { + cache_system_proxy_route_for_test( + &format!("{}/api/codex/{endpoint}", issuer.uri()), + proxy.uri(), + ); + } + let controller = codex_http_client::NetworkPolicyController::default(); + let client = Client::new( + issuer.uri(), + HttpClientFactory::new(OutboundProxyPolicy::ReqwestDefault) + .with_system_proxy_fallback() + .with_network_policy(controller.policy()), + ); + assert!(matches!( + client.get_config_bundle().await, + Err(RequestError::Policy(_)) + )); + assert!(matches!( + client.get_accounts_check().await, + Err(RequestError::Policy(_)) + )); + assert_eq!( + issuer.received_requests().await.unwrap_or_default().len(), + 0 + ); + assert_eq!(proxy.received_requests().await.unwrap_or_default().len(), 0); +} + +#[tokio::test] +async fn bootstrap_gets_retry_post_connect_failure_with_system_proxy() { + if run_without_environment_proxies( + "client::request_tests::bootstrap_gets_retry_post_connect_failure_with_system_proxy", + ) { + return; + } + for endpoint in ["accounts/check", "config/bundle"] { + let issuer = TcpListener::bind(("127.0.0.1", 0)).expect("bind dropping issuer"); + let issuer_url = format!("http://{}", issuer.local_addr().expect("issuer address")); + let issuer_thread = std::thread::spawn(move || { + let (stream, _) = issuer.accept().expect("accept bootstrap GET"); + stream + .set_read_timeout(Some(Duration::from_secs(5))) + .expect("set read timeout"); + let mut reader = std::io::BufReader::new(&stream); + let mut lines = Vec::new(); + loop { + let mut line = String::new(); + reader.read_line(&mut line).expect("read request headers"); + if line == "\r\n" || line.is_empty() { + break; + } + lines.push(line); + } + stream + .shutdown(std::net::Shutdown::Both) + .expect("drop connection before response headers"); + lines + }); + let proxy = MockServer::start().await; + let endpoint_path = format!("/api/codex/{endpoint}"); + cache_system_proxy_route_for_test(&format!("{issuer_url}{endpoint_path}"), proxy.uri()); + Mock::given(method("GET")) + .and(path(&endpoint_path)) + .and(header("authorization", "Bearer bootstrap-token")) + .and(header("chatgpt-account-id", "workspace-123")) + .and(header("user-agent", "bootstrap-test")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({}))) + .expect(1) + .mount(&proxy) + .await; + let client = fallback_client(&issuer_url); + let result = client + .exec_bootstrap_get(&format!("{issuer_url}{endpoint_path}")) + .await; + let requests = issuer_thread.join().expect("dropping issuer finishes"); + assert_eq!(requests[0], format!("GET {endpoint_path} HTTP/1.1\r\n")); + assert_eq!( + result.expect("retry the idempotent bootstrap GET through the permitted system route"), + ("{}".to_string(), "application/json".to_string()) + ); + proxy.verify().await; + } +} + #[tokio::test] async fn bootstrap_get_recovers_from_stalled_body_before_cloud_startup_timeout() { if run_without_environment_proxies(