Skip to content

fix(app): tolerate partial provider failures in get_all_provider_models()#2936

Open
tivris wants to merge 7 commits intotailcallhq:mainfrom
tivris:fix/partial-provider-failure
Open

fix(app): tolerate partial provider failures in get_all_provider_models()#2936
tivris wants to merge 7 commits intotailcallhq:mainfrom
tivris:fix/partial-provider-failure

Conversation

@tivris
Copy link
Copy Markdown
Contributor

@tivris tivris commented Apr 10, 2026

Summary

get_all_provider_models() uses .collect::<Result<Vec<_>>>() which fails the entire operation if any single configured provider returns an error. This means a stale credential for one provider (e.g. Bedrock) blocks model fetching for all other providers, causing confusing errors after an unrelated provider login like:

● GoogleAIStudio configured successfully
● ERROR: Bearer token is required in API key field

The function's docstring already describes the correct behavior (return partial results, only error when all providers fail), but the implementation doesn't match.

This PR replaces the fail-fast collection with a loop that:

  • Collects successful provider results
  • Logs tracing::warn! for individual provider failures
  • Only returns an error when every configured provider fails

Follows the same pattern used in the MCP service connection handler (forge_services/src/mcp/service.rs).

Fixes #2935

Test plan

  • RUSTFLAGS='-Dwarnings' cargo check -p forge_app compiles clean
  • cargo test -p forge_app passes
  • Configure two providers, invalidate one's credentials, verify the other still works for model selection

…ls()

The function used .collect::<Result<Vec<_>>>() which fails on the
first error, discarding all successful results from other providers.
This meant a single stale provider (e.g. Bedrock with expired
credentials) would block model fetching for all providers, causing
confusing errors after an unrelated provider login.

Replace with a loop that collects successes and logs warnings for
individual failures. Only returns an error when every configured
provider fails, matching the existing docstring contract.

Fixes tailcallhq#2935
@github-actions github-actions bot added the type: fix Iterations on existing features or infrastructure. label Apr 10, 2026
return Err(err);
}

Ok(successes)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation was recently changed. The problem was when the API key was invalid and more than one provider was activated the user would not be able to view the models of the invalid provider.

One option that I propose is - we could return a Vec of Result of models and on the UI, first show the error and then print the model selector with the models.

:model
[**:**:**] Error: Something bad happened
MODEL  PROVIDER  CONTEXT WINDOW
...    ...       ...    
...    ...       ...    
...    ...       ...    

Change get_all_provider_models() to return Vec<Result<ProviderModels>>
so individual provider failures (e.g. stale credentials) are shown to
the user instead of being silently swallowed or failing the entire
operation.

- Errors are written to stderr (preserves --porcelain stdout contract)
- Full error chain shown via alternate Display ({:#})
- All-providers-failed still surfaces as a real error
- Addresses review feedback on tailcallhq#2936

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@tivris
Copy link
Copy Markdown
Contributor Author

tivris commented Apr 10, 2026

Updated the implementation per your suggestion. get_all_provider_models() now returns Vec<Result<ProviderModels>> so individual provider errors reach the UI. Changes:

  • Per-provider errors displayed to stderr (keeps --porcelain stdout clean)
  • Full error chain shown via alternate Display format
  • If every provider fails, callers still get a real error (not a silent empty list)
  • 4 call sites in ui.rs updated accordingly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix Iterations on existing features or infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: get_all_provider_models() fails entirely when a single provider has stale credentials

2 participants