mirror of
https://github.com/openai/codex.git
synced 2026-09-20 12:47:38 +00:00
## Why Transient rate limits can end automatic approval reviews prematurely, and review failures currently report high risk even when no assessment completed. ## What changed - Retry rate limits and recoverable exhausted-stream errors, while excluding non-transient HTTP failures. - Preserve server retry delays after stream retries are exhausted and honor them within the review deadline. Scope retry advice to the current turn so reused sessions cannot apply stale delays. - Keep failed reviews denied, but leave risk and authorization unset and explain that the review could not complete without declaring the action unsafe. ## Testing Add an integration test covering rate-limit recovery through approval and tool execution, asserting that the action executes exactly once after approval. Update failure assertions to check absent assessment fields and the review-failure explanation. GitOrigin-RevId: 1163cfde35c6b8eb23b6f24f1f24461b86ded838
179 lines
5.9 KiB
Rust
179 lines
5.9 KiB
Rust
//! Shared retry and transport fallback decisions for Responses requests.
|
|
|
|
use std::time::Duration;
|
|
|
|
use crate::client::ModelClientSession;
|
|
use crate::session::session::Session;
|
|
use crate::session::turn_context::TurnContext;
|
|
use crate::util::backoff;
|
|
use codex_client::RetryOperation;
|
|
use codex_features::Feature;
|
|
use codex_protocol::error::CodexErr;
|
|
use codex_protocol::error::CodexErrorDetails;
|
|
use codex_protocol::protocol::EventMsg;
|
|
use codex_protocol::protocol::WarningEvent;
|
|
use tracing::warn;
|
|
|
|
const INITIAL_CONNECTION_RETRY_DELAY: Duration = Duration::from_secs(5);
|
|
const MAX_CONNECTION_RETRY_DELAY: Duration = Duration::from_secs(60);
|
|
|
|
#[derive(Debug, Clone, Copy)]
|
|
pub(crate) enum ResponsesStreamRequest {
|
|
Sampling,
|
|
RemoteCompactionV2,
|
|
}
|
|
|
|
pub(crate) struct ResponsesStreamRetryState {
|
|
retries: u64,
|
|
connection_retries: u64,
|
|
connection_retry_delay: Duration,
|
|
}
|
|
|
|
impl Default for ResponsesStreamRetryState {
|
|
fn default() -> Self {
|
|
Self {
|
|
retries: 0,
|
|
connection_retries: 0,
|
|
connection_retry_delay: INITIAL_CONNECTION_RETRY_DELAY,
|
|
}
|
|
}
|
|
}
|
|
|
|
/// Server retry advice retained after stream retries are exhausted. The turn ID
|
|
/// prevents a reused Guardian session from applying advice from an earlier review.
|
|
pub(crate) struct ExhaustedResponseRetry {
|
|
pub(crate) turn_id: String,
|
|
pub(crate) retry_at: Option<tokio::time::Instant>,
|
|
}
|
|
|
|
/// Handles a retryable stream error and returns `Ok(())` when the caller should
|
|
/// retry the request loop.
|
|
pub(crate) async fn handle_retryable_response_stream_error(
|
|
retry_state: &mut ResponsesStreamRetryState,
|
|
max_retries: u64,
|
|
err: CodexErr,
|
|
client_session: &mut ModelClientSession,
|
|
sess: &Session,
|
|
turn_context: &TurnContext,
|
|
request: ResponsesStreamRequest,
|
|
) -> Result<(), CodexErr> {
|
|
let operation = match request {
|
|
ResponsesStreamRequest::Sampling => RetryOperation::Sampling,
|
|
ResponsesStreamRequest::RemoteCompactionV2 => RetryOperation::RemoteCompactionV2,
|
|
};
|
|
|
|
if turn_context
|
|
.config
|
|
.features
|
|
.enabled(Feature::UnboundedConnectionRetries)
|
|
&& matches!(request, ResponsesStreamRequest::Sampling)
|
|
&& matches!(err.details(), CodexErrorDetails::ConnectionFailed(_))
|
|
&& !turn_context.session_source.is_internal()
|
|
&& !turn_context.provider.info().is_amazon_bedrock()
|
|
{
|
|
let retry_delay = retry_state.connection_retry_delay;
|
|
warn!(
|
|
turn_id = %turn_context.sub_id,
|
|
error = %err,
|
|
?retry_delay,
|
|
"stream connection failed; waiting to retry"
|
|
);
|
|
sess.notify_stream_error(turn_context, "Reconnecting... waiting for network", err)
|
|
.await;
|
|
retry_state.connection_retries = retry_state.connection_retries.saturating_add(1);
|
|
codex_client::record_retry!(retry_state.connection_retries, retry_delay, operation);
|
|
tokio::time::sleep(retry_delay).await;
|
|
retry_state.connection_retry_delay = retry_delay
|
|
.saturating_mul(2)
|
|
.min(MAX_CONNECTION_RETRY_DELAY);
|
|
return Ok(());
|
|
}
|
|
|
|
if retry_state.retries >= max_retries
|
|
&& client_session.try_switch_fallback_transport(
|
|
&turn_context.session_telemetry,
|
|
turn_context.model_info(),
|
|
)
|
|
{
|
|
sess.send_event(
|
|
turn_context,
|
|
EventMsg::Warning(WarningEvent {
|
|
message: format!("Falling back from WebSockets to HTTPS transport. {err:#}"),
|
|
}),
|
|
)
|
|
.await;
|
|
retry_state.retries = 0;
|
|
return Ok(());
|
|
}
|
|
|
|
if retry_state.retries < max_retries {
|
|
retry_state.retries += 1;
|
|
let retry_count = retry_state.retries;
|
|
let delay = err.retry_delay().unwrap_or_else(|| backoff(retry_count));
|
|
log_retry(request, turn_context, &err, retry_count, max_retries, delay);
|
|
|
|
// In release builds, hide the first websocket retry notification to reduce noisy
|
|
// transient reconnect messages. In debug builds, keep full visibility for diagnosis.
|
|
let report_error = retry_count > 1
|
|
|| cfg!(debug_assertions)
|
|
|| !sess.services.model_client.responses_websocket_enabled();
|
|
if report_error {
|
|
// Surface retry information to any UI/front-end so the user understands what is
|
|
// happening instead of staring at a seemingly frozen screen.
|
|
sess.notify_stream_error(
|
|
turn_context,
|
|
format!("Reconnecting... {retry_count}/{max_retries}"),
|
|
err,
|
|
)
|
|
.await;
|
|
}
|
|
codex_client::record_retry!(retry_count, delay, operation);
|
|
tokio::time::sleep(delay).await;
|
|
return Ok(());
|
|
}
|
|
|
|
sess.services
|
|
.thread_extension_data
|
|
.insert(ExhaustedResponseRetry {
|
|
turn_id: turn_context.sub_id.clone(),
|
|
retry_at: err
|
|
.retry_delay()
|
|
.and_then(|delay| tokio::time::Instant::now().checked_add(delay)),
|
|
});
|
|
Err(err)
|
|
}
|
|
|
|
fn log_retry(
|
|
request: ResponsesStreamRequest,
|
|
turn_context: &TurnContext,
|
|
err: &CodexErr,
|
|
retries: u64,
|
|
max_retries: u64,
|
|
delay: Duration,
|
|
) {
|
|
match request {
|
|
ResponsesStreamRequest::Sampling => {
|
|
warn!(
|
|
turn_id = %turn_context.sub_id,
|
|
retries,
|
|
max_retries,
|
|
sampling_error = %err,
|
|
"stream disconnected - retrying sampling request ({retries}/{max_retries} in {delay:?})...",
|
|
);
|
|
}
|
|
ResponsesStreamRequest::RemoteCompactionV2 => {
|
|
warn!(
|
|
turn_id = %turn_context.sub_id,
|
|
retries,
|
|
max_retries,
|
|
compact_error = %err,
|
|
"remote compaction v2 stream failed; retrying request after delay"
|
|
);
|
|
}
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
#[path = "responses_retry_tests.rs"]
|
|
mod tests;
|