diff --git a/crates/goose/src/providers/bedrock.rs b/crates/goose/src/providers/bedrock.rs index 72f07850ce..a8b4fc3c36 100644 --- a/crates/goose/src/providers/bedrock.rs +++ b/crates/goose/src/providers/bedrock.rs @@ -181,12 +181,12 @@ impl BedrockProvider { .get_param::("BEDROCK_MAX_RETRY_INTERVAL_MS") .unwrap_or(BEDROCK_DEFAULT_MAX_RETRY_INTERVAL_MS); - RetryConfig { + RetryConfig::new( max_retries, initial_interval_ms, backoff_multiplier, max_interval_ms, - } + ) } fn should_enable_caching(&self) -> bool { diff --git a/crates/goose/src/providers/databricks.rs b/crates/goose/src/providers/databricks.rs index 5b68accde8..652ff5ad20 100644 --- a/crates/goose/src/providers/databricks.rs +++ b/crates/goose/src/providers/databricks.rs @@ -217,12 +217,12 @@ impl DatabricksProvider { .and_then(|v: String| v.parse::().ok()) .unwrap_or(DEFAULT_MAX_RETRY_INTERVAL_MS); - RetryConfig { + RetryConfig::new( max_retries, initial_interval_ms, backoff_multiplier, max_interval_ms, - } + ) } fn load_fast_retry_config(_config: &crate::config::Config) -> RetryConfig { diff --git a/crates/goose/src/providers/ollama.rs b/crates/goose/src/providers/ollama.rs index 49e0c82a91..84bc18676f 100644 --- a/crates/goose/src/providers/ollama.rs +++ b/crates/goose/src/providers/ollama.rs @@ -2,7 +2,7 @@ use super::api_client::{ApiClient, AuthMethod}; use super::base::{ConfigKey, MessageStream, Provider, ProviderDef, ProviderMetadata}; use super::errors::ProviderError; use super::openai_compatible::handle_status_openai_compat; -use super::retry::ProviderRetry; +use super::retry::{ProviderRetry, RetryConfig}; use super::utils::{ImageFormat, RequestLog}; use crate::config::declarative_providers::DeclarativeProviderConfig; use crate::conversation::message::Message; @@ -36,6 +36,14 @@ pub const OLLAMA_KNOWN_MODELS: &[&str] = &[ ]; pub const OLLAMA_DOC_URL: &str = "https://ollama.com/library"; +// Ollama-specific retry config: large models can take 30-120s to load into memory, +// during which Ollama returns 500 errors. Use more retries with gradual backoff +// to wait for the model to become ready. +const OLLAMA_MAX_RETRIES: usize = 10; +const OLLAMA_INITIAL_RETRY_INTERVAL_MS: u64 = 2000; +const OLLAMA_BACKOFF_MULTIPLIER: f64 = 1.5; +const OLLAMA_MAX_RETRY_INTERVAL_MS: u64 = 15_000; + #[derive(serde::Serialize)] pub struct OllamaProvider { #[serde(skip)] @@ -211,6 +219,16 @@ impl Provider for OllamaProvider { self.model.clone() } + fn retry_config(&self) -> RetryConfig { + RetryConfig::new( + OLLAMA_MAX_RETRIES, + OLLAMA_INITIAL_RETRY_INTERVAL_MS, + OLLAMA_BACKOFF_MULTIPLIER, + OLLAMA_MAX_RETRY_INTERVAL_MS, + ) + .transient_only() + } + async fn stream( &self, model_config: &ModelConfig, @@ -340,4 +358,37 @@ mod tests { apply_ollama_options(&mut payload, &model_config); assert!(payload.get("options").is_none()); } + + #[test] + fn test_ollama_retry_config_is_transient_only() { + let config = RetryConfig::new( + OLLAMA_MAX_RETRIES, + OLLAMA_INITIAL_RETRY_INTERVAL_MS, + OLLAMA_BACKOFF_MULTIPLIER, + OLLAMA_MAX_RETRY_INTERVAL_MS, + ) + .transient_only(); + + assert!(config.transient_only); + + use super::super::errors::ProviderError; + use super::super::retry::should_retry; + + assert!(!should_retry( + &ProviderError::RequestFailed("Resource not found (404)".into()), + &config + )); + assert!(!should_retry( + &ProviderError::RequestFailed("Bad request (400)".into()), + &config + )); + assert!(should_retry( + &ProviderError::ServerError("500 model loading".into()), + &config + )); + assert!(should_retry( + &ProviderError::NetworkError("connection refused".into()), + &config + )); + } } diff --git a/crates/goose/src/providers/retry.rs b/crates/goose/src/providers/retry.rs index 0ddd093f39..f27f322dc1 100644 --- a/crates/goose/src/providers/retry.rs +++ b/crates/goose/src/providers/retry.rs @@ -20,6 +20,9 @@ pub struct RetryConfig { pub(crate) backoff_multiplier: f64, /// Maximum interval between retries in milliseconds pub(crate) max_interval_ms: u64, + /// When true, only retry on transient errors (ServerError, NetworkError, + /// RateLimitExceeded). RequestFailed (4xx client errors) will not be retried. + pub(crate) transient_only: bool, } impl Default for RetryConfig { @@ -29,6 +32,7 @@ impl Default for RetryConfig { initial_interval_ms: DEFAULT_INITIAL_RETRY_INTERVAL_MS, backoff_multiplier: DEFAULT_BACKOFF_MULTIPLIER, max_interval_ms: DEFAULT_MAX_RETRY_INTERVAL_MS, + transient_only: false, } } } @@ -45,9 +49,15 @@ impl RetryConfig { initial_interval_ms, backoff_multiplier, max_interval_ms, + transient_only: false, } } + pub fn transient_only(mut self) -> Self { + self.transient_only = true; + self + } + pub fn max_retries(&self) -> usize { self.max_retries } @@ -71,14 +81,14 @@ impl RetryConfig { } } -pub fn should_retry(error: &ProviderError) -> bool { - matches!( - error, +pub fn should_retry(error: &ProviderError, config: &RetryConfig) -> bool { + match error { ProviderError::RateLimitExceeded { .. } - | ProviderError::ServerError(_) - | ProviderError::NetworkError(_) - | ProviderError::RequestFailed(_) - ) + | ProviderError::ServerError(_) + | ProviderError::NetworkError(_) => true, + ProviderError::RequestFailed(_) => !config.transient_only, + _ => false, + } } pub async fn retry_operation( @@ -96,7 +106,7 @@ where match operation().await { Ok(result) => return Ok(result), Err(error) => { - if should_retry(&error) && attempts < config.max_retries { + if should_retry(&error, config) && attempts < config.max_retries { attempts += 1; tracing::warn!( "Request failed, retrying ({}/{}): {:?}", @@ -195,7 +205,7 @@ impl ProviderRetry for P { } } - if should_retry(&error) && attempts < config.max_retries { + if should_retry(&error, &config) && attempts < config.max_retries { attempts += 1; tracing::warn!( "Request failed, retrying ({}/{}): {:?}", @@ -232,3 +242,61 @@ impl ProviderRetry for P { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn default_config_retries_request_failed() { + let config = RetryConfig::default(); + let error = ProviderError::RequestFailed("Bad request (400): model not found".into()); + assert!(should_retry(&error, &config)); + } + + #[test] + fn transient_only_skips_request_failed() { + let config = RetryConfig::default().transient_only(); + let error = ProviderError::RequestFailed("Bad request (400): model not found".into()); + assert!(!should_retry(&error, &config)); + } + + #[test] + fn transient_only_still_retries_server_error() { + let config = RetryConfig::default().transient_only(); + assert!(should_retry( + &ProviderError::ServerError("500 internal".into()), + &config + )); + } + + #[test] + fn transient_only_still_retries_network_error() { + let config = RetryConfig::default().transient_only(); + assert!(should_retry( + &ProviderError::NetworkError("connection refused".into()), + &config + )); + } + + #[test] + fn transient_only_still_retries_rate_limit() { + let config = RetryConfig::default().transient_only(); + assert!(should_retry( + &ProviderError::RateLimitExceeded { + details: "too many requests".into(), + retry_delay: None, + }, + &config + )); + } + + #[test] + fn never_retries_auth_errors() { + let config = RetryConfig::default(); + assert!(!should_retry( + &ProviderError::Authentication("invalid key".into()), + &config + )); + } +} diff --git a/ui/acp/.npmrc b/ui/acp/.npmrc new file mode 100644 index 0000000000..214c29d139 --- /dev/null +++ b/ui/acp/.npmrc @@ -0,0 +1 @@ +registry=https://registry.npmjs.org/