mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
feat(sandbox): add retry logic for transient container failures (#1232)
* feat(sandbox): add retry logic for transient container failures (#1224) SandboxManager::execute_with_policy() had no retry logic. Transient Docker errors (daemon temporarily unavailable, container creation race conditions, container start failures) caused immediate job failure. Adds up to 2 retries (3 total attempts) with exponential backoff (2s, 4s) for transient error types only: - DockerNotAvailable - ContainerCreationFailed - ContainerStartFailed Non-transient errors (Timeout, ExecutionFailed, NetworkBlocked, Config) are returned immediately without retry. Container cleanup on retry is safe: ContainerRunner::execute() always force-removes the container before returning. Closes #1224 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * style: fix formatting Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
596d17f04b
commit
0c31da46e7
+100
-3
@@ -236,14 +236,59 @@ impl SandboxManager {
|
||||
self.initialize().await?;
|
||||
}
|
||||
|
||||
// Get proxy port if running
|
||||
// Retry transient container failures (Docker daemon glitches, container
|
||||
// creation races) up to MAX_SANDBOX_RETRIES times with exponential backoff.
|
||||
const MAX_SANDBOX_RETRIES: u32 = 2;
|
||||
let mut last_err: Option<SandboxError> = None;
|
||||
|
||||
for attempt in 0..=MAX_SANDBOX_RETRIES {
|
||||
if attempt > 0 {
|
||||
let delay = std::time::Duration::from_secs(1 << attempt); // 2s, 4s
|
||||
tracing::warn!(
|
||||
attempt = attempt + 1,
|
||||
max_attempts = MAX_SANDBOX_RETRIES + 1,
|
||||
delay_secs = delay.as_secs(),
|
||||
"Retrying sandbox execution after transient failure"
|
||||
);
|
||||
tokio::time::sleep(delay).await;
|
||||
}
|
||||
|
||||
match self
|
||||
.try_execute_in_container(command, cwd, policy, env.clone())
|
||||
.await
|
||||
{
|
||||
Ok(output) => return Ok(output),
|
||||
Err(e) if is_transient_sandbox_error(&e) => {
|
||||
tracing::warn!(
|
||||
attempt = attempt + 1,
|
||||
error = %e,
|
||||
"Transient sandbox error, will retry"
|
||||
);
|
||||
last_err = Some(e);
|
||||
}
|
||||
Err(e) => return Err(e),
|
||||
}
|
||||
}
|
||||
|
||||
Err(last_err.unwrap_or_else(|| SandboxError::ExecutionFailed {
|
||||
reason: "all retry attempts exhausted".to_string(),
|
||||
}))
|
||||
}
|
||||
|
||||
/// Single attempt at container execution (no retry logic).
|
||||
async fn try_execute_in_container(
|
||||
&self,
|
||||
command: &str,
|
||||
cwd: &Path,
|
||||
policy: SandboxPolicy,
|
||||
env: HashMap<String, String>,
|
||||
) -> Result<ExecOutput> {
|
||||
let proxy_port = if let Some(proxy) = self.proxy.read().await.as_ref() {
|
||||
proxy.addr().await.map(|a| a.port()).unwrap_or(0)
|
||||
} else {
|
||||
0
|
||||
};
|
||||
|
||||
// Reuse the stored Docker connection, create a runner with the current proxy port
|
||||
let docker =
|
||||
self.docker
|
||||
.read()
|
||||
@@ -262,7 +307,6 @@ impl SandboxManager {
|
||||
};
|
||||
|
||||
let container_output = runner.execute(command, cwd, policy, &limits, env).await?;
|
||||
|
||||
Ok(container_output.into())
|
||||
}
|
||||
|
||||
@@ -373,6 +417,20 @@ impl Drop for SandboxManager {
|
||||
}
|
||||
}
|
||||
|
||||
/// Check whether a sandbox error is transient and worth retrying.
|
||||
///
|
||||
/// Transient errors are those caused by Docker daemon glitches, container
|
||||
/// creation race conditions, or container start failures — not by command
|
||||
/// execution failures, timeouts, or policy violations.
|
||||
fn is_transient_sandbox_error(err: &SandboxError) -> bool {
|
||||
matches!(
|
||||
err,
|
||||
SandboxError::DockerNotAvailable { .. }
|
||||
| SandboxError::ContainerCreationFailed { .. }
|
||||
| SandboxError::ContainerStartFailed { .. }
|
||||
)
|
||||
}
|
||||
|
||||
/// Builder for creating a sandbox manager.
|
||||
pub struct SandboxManagerBuilder {
|
||||
config: SandboxConfig,
|
||||
@@ -597,4 +655,43 @@ mod tests {
|
||||
assert!(output.truncated);
|
||||
assert!(output.stdout.len() <= 32 * 1024);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transient_errors_are_retryable() {
|
||||
assert!(super::is_transient_sandbox_error(
|
||||
&SandboxError::DockerNotAvailable {
|
||||
reason: "daemon restarting".to_string()
|
||||
}
|
||||
));
|
||||
assert!(super::is_transient_sandbox_error(
|
||||
&SandboxError::ContainerCreationFailed {
|
||||
reason: "image pull glitch".to_string()
|
||||
}
|
||||
));
|
||||
assert!(super::is_transient_sandbox_error(
|
||||
&SandboxError::ContainerStartFailed {
|
||||
reason: "cgroup race".to_string()
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_transient_errors_are_not_retryable() {
|
||||
assert!(!super::is_transient_sandbox_error(&SandboxError::Timeout(
|
||||
std::time::Duration::from_secs(30)
|
||||
)));
|
||||
assert!(!super::is_transient_sandbox_error(
|
||||
&SandboxError::ExecutionFailed {
|
||||
reason: "exit code 1".to_string()
|
||||
}
|
||||
));
|
||||
assert!(!super::is_transient_sandbox_error(
|
||||
&SandboxError::NetworkBlocked {
|
||||
reason: "policy violation".to_string()
|
||||
}
|
||||
));
|
||||
assert!(!super::is_transient_sandbox_error(&SandboxError::Config {
|
||||
reason: "bad config".to_string()
|
||||
}));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user