fix: avoid success toast on failed provider test#7934
fix: avoid success toast on failed provider test#7934RC-CHN merged 1 commit intoAstrBotDevs:masterfrom
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
testSingleProvider, consider handling theresult.status !== 'available'case without throwing (e.g., by updating status and callingshowMessagedirectly) so that exceptions are reserved for genuinely unexpected failures rather than normal unavailability. - When using
result.errorfor the error toast and error throwing, ensure it is normalized to a user-facing string (e.g., handle cases where the backend might return a non-string error payload) to avoid showing[object Object]or similar in the UI.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `testSingleProvider`, consider handling the `result.status !== 'available'` case without throwing (e.g., by updating status and calling `showMessage` directly) so that exceptions are reserved for genuinely unexpected failures rather than normal unavailability.
- When using `result.error` for the error toast and error throwing, ensure it is normalized to a user-facing string (e.g., handle cases where the backend might return a non-string error payload) to avoid showing `[object Object]` or similar in the UI.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request refactors the testSingleProvider function in ProviderPage.vue to improve error handling, response validation, and user feedback via error toasts. The review feedback identifies opportunities to reduce code duplication by moving shared logic to a composable, improve URL construction using axios parameters, and ensure consistent use of translated error messages. A potential issue regarding redundant state updates and metadata loss in the error handling path was also highlighted for correction.
| @@ -676,18 +676,29 @@ async function testSingleProvider(provider) { | |||
|
|
|||
| const startTime = performance.now() | |||
| const res = await axios.get(`/api/config/provider/check_one?id=${provider.id}`) | |||
There was a problem hiding this comment.
The logic in testSingleProvider is largely duplicated with testProvider in useProviderSources.ts. Per the general rules, this core logic (API call, latency calculation, and error handling) should be refactored into a shared helper function in the composable to improve maintainability and ensure consistency across the application.
References
- Refactor similar functionality into shared helper functions to avoid code duplication.
| const res = await axios.get(`/api/config/provider/check_one?id=${provider.id}`) | ||
| if (res.data && res.data.status === 'ok') { | ||
| const index = providerStatuses.value.findIndex(s => s.id === provider.id) | ||
| if (index !== -1) { | ||
| providerStatuses.value.splice(index, 1, res.data.data) | ||
| } | ||
| const latency = Math.max(0, Math.round(performance.now() - startTime)) | ||
| showMessage(tm('models.testSuccessWithLatency', { id: provider.id, latency })) | ||
| } else { | ||
| if (!res.data || res.data.status !== 'ok') { | ||
| throw new Error(res.data?.message || `Failed to check status for ${provider.id}`) | ||
| } |
There was a problem hiding this comment.
Consider using the params option in axios.get for better URL encoding. Also, use a translated error message instead of a hardcoded English string for consistency with the rest of the application.
const res = await axios.get('/api/config/provider/check_one', { params: { id: provider.id } })\n if (!res.data || res.data.status !== 'ok') {\n throw new Error(res.data?.message || tm('models.testError'))\n }
| if (!result) { | ||
| throw new Error(`Failed to check status for ${provider.id}`) | ||
| } |
| if (index !== -1) { | ||
| providerStatuses.value.splice(index, 1, failedStatus) | ||
| } | ||
| showMessage(errorMessage, 'error') |
There was a problem hiding this comment.
While adding an error toast is a good fix, note that providerStatuses might be updated twice if the error was thrown at line 695 (once at line 690 and again at line 710). The second update in the catch block also loses metadata like model and type present in the backend result. Consider refactoring to avoid this redundant update and preserve the full result data.
Fixes #7933. The Provider page test flow previously reported success based on the HTTP wrapper status, which could show a success toast even when the provider test result was unavailable. This change ensures the success toast only appears when the provider is actually available.
Modifications / 改动点
Update the non-chat Provider page test flow to validate provider availability before showing a success toast.
On failure, show a standard error toast to avoid misleading success feedback.
Core file modified: dashboard/src/views/ProviderPage.vue
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
This is the original bug screenshot:

Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Ensure provider test success feedback only appears when the provider is actually available and surface clear error feedback on failures.
Bug Fixes:
Enhancements: