Skip to content

Commit 664cc3f

Browse files
committed
fix(providers): only fallback to custom models for endpoint-not-implemented errors
Auth failures (401/403), rate limits (429), and server errors (5xx) now propagate correctly instead of being hidden by fallback. Only 404 and connection failures trigger fallback to preserve diagnostics. Fixes P1 feedback from PR aaif-goose#7530
1 parent e5d66f6 commit 664cc3f

3 files changed

Lines changed: 57 additions & 16 deletions

File tree

crates/goose/src/providers/anthropic.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,18 +223,23 @@ impl Provider for AnthropicProvider {
223223
}
224224

225225
async fn fetch_supported_models(&self) -> Result<Vec<String>, ProviderError> {
226-
// If custom models are defined, try API first but fallback to them on error
226+
// If custom models are defined, try API first but fallback to them only if endpoint doesn't exist
227227
if let Some(custom_models) = &self.custom_models {
228228
match self.fetch_models_from_api().await {
229229
Ok(models) => return Ok(models),
230230
Err(e) => {
231-
// Log the error but don't fail - fallback to custom models
232-
tracing::debug!(
233-
"Failed to fetch models from API for provider '{}' ({}), using predefined list",
234-
self.name,
235-
e
236-
);
237-
return Ok(custom_models.clone());
231+
// Only fall back for endpoint-not-implemented errors (404, connection failures)
232+
// Auth errors, rate limits, and server errors should propagate
233+
if e.is_endpoint_not_implemented() {
234+
tracing::debug!(
235+
"Models endpoint not implemented for provider '{}' ({}), using predefined list",
236+
self.name,
237+
e
238+
);
239+
return Ok(custom_models.clone());
240+
}
241+
// Otherwise, propagate the error to preserve diagnostics
242+
return Err(e);
238243
}
239244
}
240245
}

crates/goose/src/providers/errors.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,37 @@ impl ProviderError {
5252
ProviderError::CreditsExhausted { .. } => "credits_exhausted",
5353
}
5454
}
55+
56+
/// Returns true if this error indicates the models endpoint is not implemented
57+
/// and we should fall back to configured custom models.
58+
///
59+
/// Only certain errors should trigger fallback - specifically 404 (endpoint not found)
60+
/// or connection failures that suggest the endpoint doesn't exist.
61+
///
62+
/// Critical errors that should NOT fallback:
63+
/// - Authentication failures (401/403) - indicates misconfigured credentials
64+
/// - Rate limits (429) - indicates service is functioning but rate-limited
65+
/// - Server errors (5xx) - indicates service is having issues
66+
/// - Context length exceeded - indicates API is working
67+
pub fn is_endpoint_not_implemented(&self) -> bool {
68+
match self {
69+
// 404 or connection failures suggest endpoint doesn't exist - safe to fallback
70+
ProviderError::RequestFailed(msg) => {
71+
msg.contains("404")
72+
|| msg.contains("not found")
73+
|| msg.contains("connection failed")
74+
|| msg.contains("failed to connect")
75+
}
76+
// Auth, rate limit, server, context errors - do NOT fallback
77+
ProviderError::Authentication(_) => false,
78+
ProviderError::RateLimitExceeded { .. } => false,
79+
ProviderError::ServerError(_) => false,
80+
ProviderError::ContextLengthExceeded(_) => false,
81+
ProviderError::CreditsExhausted { .. } => false,
82+
// Other errors may indicate the endpoint exists - safer to not fallback
83+
_ => false,
84+
}
85+
}
5586
}
5687

5788
impl From<anyhow::Error> for ProviderError {

crates/goose/src/providers/openai.rs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -366,18 +366,23 @@ impl Provider for OpenAiProvider {
366366
}
367367

368368
async fn fetch_supported_models(&self) -> Result<Vec<String>, ProviderError> {
369-
// If custom models are defined, try API first but fallback to them on error
369+
// If custom models are defined, try API first but fallback to them only if endpoint doesn't exist
370370
if let Some(custom_models) = &self.custom_models {
371371
match self.fetch_models_from_api().await {
372372
Ok(models) => return Ok(models),
373373
Err(e) => {
374-
// Log the error but don't fail - fallback to custom models
375-
tracing::debug!(
376-
"Failed to fetch models from API for provider '{}' ({}), using predefined list",
377-
self.name,
378-
e
379-
);
380-
return Ok(custom_models.clone());
374+
// Only fall back for endpoint-not-implemented errors (404, connection failures)
375+
// Auth errors, rate limits, and server errors should propagate
376+
if e.is_endpoint_not_implemented() {
377+
tracing::debug!(
378+
"Models endpoint not implemented for provider '{}' ({}), using predefined list",
379+
self.name,
380+
e
381+
);
382+
return Ok(custom_models.clone());
383+
}
384+
// Otherwise, propagate the error to preserve diagnostics
385+
return Err(e);
381386
}
382387
}
383388
}

0 commit comments

Comments
 (0)