diff --git a/crates/forge_app/src/app.rs b/crates/forge_app/src/app.rs index d53b3c5b7e..ffe84e4bdb 100644 --- a/crates/forge_app/src/app.rs +++ b/crates/forge_app/src/app.rs @@ -299,14 +299,15 @@ impl> ForgeAp /// Gets available models from all configured providers concurrently. /// - /// Returns a list of `ProviderModels` for each configured provider that - /// successfully returned models. If every configured provider fails (e.g. - /// due to an invalid API key), the first error encountered is returned so - /// the caller receives the real underlying cause rather than an empty list. + /// Returns one [`ProviderModels`] per configured provider, in attempt + /// order. Each entry carries either the fetched models or the + /// per-provider error, letting callers display partial results and + /// surface failures alongside successes. pub async fn get_all_provider_models(&self) -> Result> { let all_providers = self.services.get_all_providers().await?; - // Build one future per configured provider, preserving the error on failure. + // Build one future per configured provider, capturing the per-provider + // result inline so failures don't drop their provider id on the floor. let futures: Vec<_> = all_providers .into_iter() .filter_map(|any_provider| any_provider.into_configured()) @@ -314,24 +315,23 @@ impl> ForgeAp let provider_id = provider.id.clone(); let services = self.services.clone(); async move { - let result: Result = async { + let models = async { let refreshed = services .provider_auth_service() .refresh_provider_credential(provider) .await?; - let models = services.models(refreshed).await?; - Ok(ProviderModels { provider_id, models }) + services.models(refreshed).await } .await; - result + if let Err(err) = &models { + tracing::warn!(provider = %provider_id, %err, "failed to fetch models"); + } + ProviderModels { provider_id, models } } }) .collect(); // Execute all provider fetches concurrently. - futures::future::join_all(futures) - .await - .into_iter() - .collect::>>() + Ok(futures::future::join_all(futures).await) } } diff --git a/crates/forge_domain/src/provider.rs b/crates/forge_domain/src/provider.rs index 6899e7a751..ca89085209 100644 --- a/crates/forge_domain/src/provider.rs +++ b/crates/forge_domain/src/provider.rs @@ -359,13 +359,14 @@ impl AnyProvider { } } -/// Represents a provider with its available models -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +/// Represents a provider with its available models, or the per-provider +/// error encountered while fetching them. +#[derive(Debug)] pub struct ProviderModels { /// The provider identifier pub provider_id: ProviderId, - /// Available models from this provider - pub models: Vec, + /// Available models from this provider, or the per-provider fetch error. + pub models: anyhow::Result>, } #[cfg(test)] diff --git a/crates/forge_main/src/ui.rs b/crates/forge_main/src/ui.rs index 00a75d7e67..8e5546ff9a 100644 --- a/crates/forge_main/src/ui.rs +++ b/crates/forge_main/src/ui.rs @@ -1388,14 +1388,16 @@ impl A + Send + Sync> UI // Sort models and then providers all_provider_models .iter_mut() - .for_each(|pm| pm.models.sort_by(|a, b| a.id.as_str().cmp(b.id.as_str()))); + .filter_map(|pm| pm.models.as_mut().ok()) + .for_each(|models| models.sort_by(|a, b| a.id.as_str().cmp(b.id.as_str()))); all_provider_models.sort_by(|a, b| a.provider_id.as_ref().cmp(b.provider_id.as_ref())); let mut info = Info::new(); for pm in &all_provider_models { let provider_id: &str = &pm.provider_id; let provider_display = pm.provider_id.to_string(); - for model in &pm.models { + let Ok(models) = &pm.models else { continue }; + for model in models { let id = model.id.to_string(); info = info .add_title(&id) @@ -1448,6 +1450,12 @@ impl A + Send + Sync> UI self.writeln(info)?; } + for pm in &all_provider_models { + if let Err(err) = &pm.models { + self.writeln_title(TitleFormat::error(format!("{err:?}")))?; + } + } + Ok(()) } @@ -2828,6 +2836,8 @@ impl A + Send + Sync> UI all_provider_models.retain(|pm| &pm.provider_id == filter_id); } + all_provider_models.retain(|pm| pm.models.is_ok()); + if all_provider_models.is_empty() { return Ok(None); } @@ -2835,7 +2845,8 @@ impl A + Send + Sync> UI // Sort models and providers (same as on_show_models) all_provider_models .iter_mut() - .for_each(|pm| pm.models.sort_by(|a, b| a.id.as_str().cmp(b.id.as_str()))); + .filter_map(|pm| pm.models.as_mut().ok()) + .for_each(|models| models.sort_by(|a, b| a.id.as_str().cmp(b.id.as_str()))); all_provider_models.sort_by(|a, b| a.provider_id.as_ref().cmp(b.provider_id.as_ref())); // Build the same Info structure as on_show_models, then convert to @@ -2843,7 +2854,8 @@ impl A + Send + Sync> UI let mut info = Info::new(); for pm in &all_provider_models { let provider_display = pm.provider_id.to_string(); - for model in &pm.models { + let Ok(models) = &pm.models else { continue }; + for model in models { let id = model.id.to_string(); info = info .add_title(&id) @@ -2904,7 +2916,8 @@ impl A + Send + Sync> UI // the Info entries (sorted by provider, then model within provider). let mut model_entries: Vec<(ModelId, ProviderId)> = Vec::new(); for pm in &all_provider_models { - for model in &pm.models { + let Ok(models) = &pm.models else { continue }; + for model in models { model_entries.push((model.id.clone(), pm.provider_id.clone())); } } @@ -3642,8 +3655,8 @@ impl A + Send + Sync> UI let model_available = provider_models .iter() .find(|pm| pm.provider_id == provider.id) - .map(|pm| pm.models.iter().any(|m| m.id == current_model)) - .unwrap_or(false); + .and_then(|pm| pm.models.as_ref().ok()) + .is_some_and(|models| models.iter().any(|m| m.id == current_model)); if model_available { (false, Some(current_model)) } else { @@ -4548,7 +4561,7 @@ impl A + Send + Sync> UI .with_context(|| { format!("Provider '{provider_id}' not found or returned no models") })? - .models + .models? } }; let model_id = ModelId::new(model_str);