mirror of
https://github.com/block/goose.git
synced 2026-07-17 12:56:20 +02:00
fix: increase Ollama retry config + add transient-only mode (#7677)
Signed-off-by: fre <anonwurcod@proton.me>
This commit is contained in:
@@ -181,12 +181,12 @@ impl BedrockProvider {
|
||||
.get_param::<u64>("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 {
|
||||
|
||||
@@ -217,12 +217,12 @@ impl DatabricksProvider {
|
||||
.and_then(|v: String| v.parse::<u64>().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 {
|
||||
|
||||
@@ -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
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<F, Fut, T>(
|
||||
@@ -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<P: Provider> 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<P: Provider> 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
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1 @@
|
||||
registry=https://registry.npmjs.org/
|
||||
Reference in New Issue
Block a user