mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Add timeout for remote compaction requests (#23451)
## Why Remote compaction currently sends a unary `POST /responses/compact` and waits for the full response before replacing history or emitting the completed `ContextCompaction` item. Unlike normal `/responses` streaming requests, this unary compact request had no timeout boundary. If the backend accepts the request and then stalls before returning a body, the existing request retry policy never sees a transport error, so the compact turn can remain stuck after the started item with no completion or actionable error. That matches the reported hang shape in issues such as #18363, where logs show `responses/compact` was posted but no corresponding compact completion followed. A bounded request timeout gives the existing retry policy a concrete timeout error to retry instead of letting the user sit indefinitely on automatic context compaction. ## What - Add a request timeout to legacy `/responses/compact` calls. - Size that timeout from the provider stream idle timeout with a conservative multiplier, so the default compact attempt gets 20 minutes rather than the 5 minute stream idle window. - Map API transport timeouts to a request timeout error instead of the child-process timeout message. ## Testing - Not run (per request; CI will cover).
This commit is contained in:
committed by
GitHub
Unverified
parent
000bf5ce6d
commit
18cefba922
@@ -123,7 +123,7 @@ pub fn map_api_error(err: ApiError) -> CodexErr {
|
||||
status: http::StatusCode::INTERNAL_SERVER_ERROR,
|
||||
request_id: None,
|
||||
}),
|
||||
TransportError::Timeout => CodexErr::Timeout,
|
||||
TransportError::Timeout => CodexErr::RequestTimeout,
|
||||
TransportError::Network(msg) | TransportError::Build(msg) => {
|
||||
CodexErr::Stream(msg, None)
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ use http::Method;
|
||||
use serde::Deserialize;
|
||||
use serde_json::to_value;
|
||||
use std::sync::Arc;
|
||||
use std::time::Duration;
|
||||
|
||||
pub struct CompactClient<T: HttpTransport> {
|
||||
session: EndpointSession<T>,
|
||||
@@ -37,10 +38,19 @@ impl<T: HttpTransport> CompactClient<T> {
|
||||
&self,
|
||||
body: serde_json::Value,
|
||||
extra_headers: HeaderMap,
|
||||
request_timeout: Duration,
|
||||
) -> Result<Vec<ResponseItem>, ApiError> {
|
||||
let resp = self
|
||||
.session
|
||||
.execute(Method::POST, Self::path(), extra_headers, Some(body))
|
||||
.execute_with(
|
||||
Method::POST,
|
||||
Self::path(),
|
||||
extra_headers,
|
||||
Some(body),
|
||||
|req| {
|
||||
req.timeout = Some(request_timeout);
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
let parsed: CompactHistoryResponse =
|
||||
serde_json::from_slice(&resp.body).map_err(|e| ApiError::Stream(e.to_string()))?;
|
||||
@@ -51,10 +61,11 @@ impl<T: HttpTransport> CompactClient<T> {
|
||||
&self,
|
||||
input: &CompactionInput<'_>,
|
||||
extra_headers: HeaderMap,
|
||||
request_timeout: Duration,
|
||||
) -> Result<Vec<ResponseItem>, ApiError> {
|
||||
let body = to_value(input)
|
||||
.map_err(|e| ApiError::Stream(format!("failed to encode compaction input: {e}")))?;
|
||||
self.compact(body, extra_headers).await
|
||||
self.compact(body, extra_headers, request_timeout).await
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -146,6 +146,9 @@ const X_CODEX_WS_STREAM_REQUEST_START_MS_CLIENT_METADATA_KEY: &str =
|
||||
const RESPONSES_WEBSOCKETS_V2_BETA_HEADER_VALUE: &str = "responses_websockets=2026-02-06";
|
||||
const RESPONSES_ENDPOINT: &str = "/responses";
|
||||
const RESPONSES_COMPACT_ENDPOINT: &str = "/responses/compact";
|
||||
// `/responses/compact` is unary, so the timeout covers the full response rather than one idle
|
||||
// period between stream events.
|
||||
const COMPACT_REQUEST_TIMEOUT_IDLE_MULTIPLIER: u32 = 4;
|
||||
const MEMORIES_SUMMARIZE_ENDPOINT: &str = "/memories/trace_summarize";
|
||||
#[cfg(test)]
|
||||
pub(crate) const WEBSOCKET_CONNECT_TIMEOUT: Duration =
|
||||
@@ -502,12 +505,16 @@ impl ModelClient {
|
||||
if let Some(header_value) = self.generate_attestation_header_for().await {
|
||||
extra_headers.insert(X_OAI_ATTESTATION_HEADER, header_value);
|
||||
}
|
||||
let compact_request_timeout = client_setup
|
||||
.api_provider
|
||||
.stream_idle_timeout
|
||||
.saturating_mul(COMPACT_REQUEST_TIMEOUT_IDLE_MULTIPLIER);
|
||||
let client =
|
||||
ApiCompactClient::new(transport, client_setup.api_provider, client_setup.api_auth)
|
||||
.with_telemetry(Some(request_telemetry));
|
||||
let trace_attempt = compaction_trace.start_attempt(&payload);
|
||||
let result = client
|
||||
.compact_input(&payload, extra_headers)
|
||||
.compact_input(&payload, extra_headers, compact_request_timeout)
|
||||
.await
|
||||
.map_err(map_api_error);
|
||||
trace_attempt.record_result(result.as_deref());
|
||||
|
||||
@@ -89,6 +89,8 @@ pub enum CodexErr {
|
||||
/// Returned by run_command_stream when the spawned child process timed out (10s).
|
||||
#[error("timeout waiting for child process to exit")]
|
||||
Timeout,
|
||||
#[error("request timed out")]
|
||||
RequestTimeout,
|
||||
/// Returned by run_command_stream when the child could not be spawned (its stdout/stderr pipes
|
||||
/// could not be captured). Analogous to the previous `CodexError::Spawn` variant.
|
||||
#[error("spawn failed: child stdout/stderr not captured")]
|
||||
@@ -192,6 +194,7 @@ impl CodexErr {
|
||||
| CodexErr::CyberPolicy { .. } => false,
|
||||
CodexErr::Stream(..)
|
||||
| CodexErr::Timeout
|
||||
| CodexErr::RequestTimeout
|
||||
| CodexErr::UnexpectedStatus(_)
|
||||
| CodexErr::ResponseStreamFailed(_)
|
||||
| CodexErr::ConnectionFailed(_)
|
||||
|
||||
Reference in New Issue
Block a user