feat(provider): 允许的供应商模型支持从上游 API 动态获取#491
Conversation
新增或编辑供应商时，允许的模型列表优先从上游 API 获取真实可用的模型， 失败时回退到本地价格表，并通过图标指示数据来源（云端/本地）。
Summary of ChangesHello @NieiR, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求的核心目标是提升供应商模型管理的灵活性和准确性。通过引入从上游API动态获取模型列表的能力，系统能够实时反映供应商提供的最新模型，并提供清晰的模型来源指示。同时，健壮的回退机制和修复的代理兼容性问题，确保了在各种情况下用户都能获得可靠的模型选择体验。 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
Bot
added
enhancement
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次 PR 的主要目标是实现供应商模型列表的动态获取功能，当新增或编辑供应商时，允许从上游 API 获取可用模型，并在失败时回退到本地列表。整体实现思路清晰，代码结构良好，特别是在 src/actions/providers.ts 中对不同供应商的 API 进行了模块化处理，并增加了代理和错误处理逻辑，提高了代码的健壮性。在前端 model-multi-select.tsx 组件中，通过 useCallback 和 useEffect 实现了模型加载逻辑，并增加了数据来源指示和手动刷新功能，提升了用户体验。
我提出了两点建议：
- 在后端的
providers.ts文件中，建议移除GET请求中不必要的Content-Type头，以保持 HTTP 请求的准确性。 - 在前端的
model-multi-select.tsx组件中，当前的useEffect实现可能存在竞态条件和在组件卸载后更新状态的风险，建议进行重构以提高组件的稳健性。
除此之外，代码质量很高，注释清晰，功能实现完整。合并后将显著改善供应商配置的灵活性和准确性。
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds upstream model list fetching functionality for provider configuration. The implementation is functional but has critical error handling issues that need to be addressed before merging.
PR Size: M
- Lines changed: 579 (536 additions + 43 deletions)
- Files changed: 9
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Error Handling | 0 | 2 | 0 | 0 |
| User Feedback | 0 | 2 | 1 | 0 |
| Tests | 0 | 1 | 0 | 0 |
| Logic/Security | 0 | 1 | 0 | 0 |
Critical & High Priority Issues
1. [HIGH] Silent Gemini Auth Failure (src/actions/providers.ts:2918-2925)
Issue: When GeminiAuth.getAccessToken() fails, the code catches the error, logs a warning, and continues with the potentially invalid original API key. The user is never informed of the auth processing failure.
Location: Lines 2918-2925 in fetchGeminiModels()
Current Code:
try {
processedApiKey = await GeminiAuth.getAccessToken(data.apiKey);
isJsonCreds = GeminiAuth.isJson(data.apiKey);
} catch (e) {
logger.warn("fetchGeminiModels: auth process failed", { error: e });
}Fix:
try {
processedApiKey = await GeminiAuth.getAccessToken(data.apiKey);
isJsonCreds = GeminiAuth.isJson(data.apiKey);
} catch (e) {
logger.error("fetchGeminiModels: auth processing failed", {
error: e,
keyFormat: GeminiAuth.isJson(data.apiKey) ? 'json' : 'string'
});
return {
ok: false,
error: `认证处理失败: ${e instanceof Error ? e.message : '未知错误'}`
};
}Severity: High - Users will receive confusing 401/403 errors instead of clear auth processing errors
2. [HIGH] Insufficient HTTP Error Context (src/actions/providers.ts:2882-2892)
Issue: The handleHttpError() function returns minimal error information to the user (only HTTP status code), while the actual error message from the API is logged but not shown.
Location: Lines 2882-2892
Current Code:
function handleHttpError(
response: Response,
errorText: string,
logPrefix: string
): FetchUpstreamModelsResult {
logger.warn(`${logPrefix}: API returned error`, {
status: response.status,
errorPreview: errorText.substring(0, 200),
});
return { ok: false, error: `API 返回错误: HTTP ${response.status}` };
}Fix:
function handleHttpError(
response: Response,
errorText: string,
logPrefix: string
): FetchUpstreamModelsResult {
logger.warn(`${logPrefix}: API returned error`, {
url: response.url,
status: response.status,
statusText: response.statusText,
errorPreview: errorText.substring(0, 200),
});
const errorSummary = errorText.trim().split('\n')[0].substring(0, 100);
const userMessage = errorSummary
? `API 返回错误 (HTTP ${response.status}): ${errorSummary}`
: `API 返回错误: HTTP ${response.status} ${response.statusText}`;
return { ok: false, error: userMessage };
}Severity: High - Makes troubleshooting API failures very difficult for admins
3. [HIGH] Generic Exception Handling Without Error Type Discrimination (src/actions/providers.ts:2893-2903)
Issue: The handleFetchException() function treats all errors identically, losing important diagnostic context like timeouts, DNS failures, and connection issues.
Location: Lines 2893-2903
Current Code:
function handleFetchException(
error: unknown,
logPrefix: string
): FetchUpstreamModelsResult {
const err = error as Error & { code?: string };
logger.warn(`${logPrefix}: request failed`, {
error: err.message,
code: err.code,
});
return { ok: false, error: `请求失败: ${err.message}` };
}Fix:
function handleFetchException(
error: unknown,
logPrefix: string
): FetchUpstreamModelsResult {
const err = error as Error & { code?: string };
logger.warn(`${logPrefix}: request failed`, {
errorName: err.name,
errorMessage: err.message,
errorCode: err.code,
isTimeout: err.name === 'AbortError' || err.name === 'TimeoutError',
});
let userMessage = '请求失败';
if (err.name === 'AbortError' || err.name === 'TimeoutError') {
userMessage = '请求超时，请检查网络连接或增加超时时间';
} else if (err.code === 'ENOTFOUND') {
userMessage = 'DNS 解析失败，请检查 URL 是否正确';
} else if (err.code === 'ECONNREFUSED') {
userMessage = '连接被拒绝，请检查服务是否运行';
} else if (err.message) {
userMessage = `请求失败: ${err.message}`;
} else {
userMessage = '请求失败: 未知错误';
}
return { ok: false, error: userMessage };
}Severity: High - Timeout and network errors are common but not clearly identified
4. [HIGH] Silent Fallback Without Logging (model-multi-select.tsx:76-117)
Issue: When fetchUpstreamModels() fails, the component silently falls back to local price list without console logging or user notification.
Location: Lines 76-117 in loadModels() callback
Fix: Add logging when fallback occurs:
if (upstreamResult.ok && upstreamResult.data) {
setAvailableModels(upstreamResult.data.models);
setModelSource("upstream");
setLoading(false);
return;
}
// Log the failure reason for debugging
console.warn('[ModelSelect] Failed to fetch upstream models, falling back to local:',
upstreamResult.error);Severity: High - Makes production debugging very difficult
5. [MEDIUM] Vague Proxy URL Validation Error (src/actions/providers.ts:2837-2838)
Issue: When proxy URL validation fails, the error message is generic and doesn't explain what formats are valid.
Location: Lines 2837-2838
Fix:
if (data.proxyUrl && !isValidProxyUrl(data.proxyUrl)) {
logger.warn("fetchUpstreamModels: invalid proxy URL", {
proxyUrl: data.proxyUrl,
});
return {
ok: false,
error: "代理地址格式无效，请使用 http://host:port 或 socks5://host:port 格式"
};
}Severity: Medium - Affects UX but not critical
6. [HIGH] Missing Tests for Critical New Feature
Issue: No tests found for the new fetchUpstreamModels() function, which makes network calls and has complex error handling logic.
Required Test Coverage:
- ✅ Successful upstream fetch (OpenAI, Gemini, Anthropic)
- ✅ Network timeout handling
- ✅ HTTP error responses (401, 403, 500)
- ✅ Fallback to local price list
- ✅ Invalid URL/proxy URL validation
- ✅ Gemini auth failure scenarios
- ✅ Race condition handling in React component
Severity: High - Untested network code with complex error handling is high-risk
7. [HIGH] Potential Race Condition in loadModels (model-multi-select.tsx)
Issue: The loadModels() function doesn't track which invocation is the latest. If a user rapidly changes provider URL or clicks refresh multiple times, stale responses could overwrite fresh data.
Location: Lines 76-117
Fix: Add cancellation token or sequence tracking:
const loadModels = useCallback(async () => {
const requestId = ++requestIdRef.current; // Add ref to track latest request
setLoading(true);
setModelSource("loading");
// ... existing logic ...
// Before setState, check if this is still the latest request
if (requestId !== requestIdRef.current) {
return; // Stale request, ignore
}
setAvailableModels(upstreamResult.data.models);
setModelSource("upstream");
setLoading(false);
}, [providerUrl, apiKey, providerId, providerType, proxyUrl, proxyFallbackToDirect]);Severity: High - Race conditions can cause incorrect data to be displayed
Review Coverage
- Logic and correctness - 1 race condition identified
- Security (OWASP Top 10) - Clean (auth validation present)
- Error handling - 5 high-priority issues identified
- Type safety - Clean (proper TypeScript usage)
- Documentation accuracy - Clean
- Test coverage - Missing critical tests
- Code clarity - Good overall
Recommendations
Before merging:
- ✅ Fix all High severity error handling issues (#1, #2, #3, #4, #7)
- ✅ Add unit tests for
fetchUpstreamModels()covering error scenarios - ✅ Add test for race condition in
loadModels() ⚠️ Consider fixing Medium severity issue (#5) for better UX
Post-merge improvements:
- Add integration tests with mock upstream APIs
- Consider adding retry logic for transient network failures
- Add telemetry/metrics for upstream fetch success rate
Automated review by Claude AI
- 移除 wechat.ts 中无用的 case "info" 分支 - 修复 providers.ts 代码格式化问题
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
2 participants
Summary
新增或编辑供应商时，允许的模型列表支持从上游 API 动态获取真实可用的模型，失败时回退到本地价格表。
Changes
Screenshots