mirror of
https://github.com/openai/codex.git
synced 2026-10-20 08:19:50 +00:00
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
This commit is contained in:
@@ -360,7 +360,9 @@ impl Client {
|
||||
Ok(Err(RequestError::Other(error)))
|
||||
if error
|
||||
.downcast_ref::<RouteAwareRequestError>()
|
||||
.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,
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user