Skip to content

fix: retry channel tests with stream when configured - #3303

Open
zhuhaow wants to merge 3 commits into
QuantumNous:mainfrom
zhuhaow:fix/issue-3298-channel-test-stream-retry
Open

fix: retry channel tests with stream when configured#3303
zhuhaow wants to merge 3 commits into
QuantumNous:mainfrom
zhuhaow:fix/issue-3298-channel-test-stream-retry

Conversation

@zhuhaow

@zhuhaow zhuhaow commented Mar 18, 2026

Copy link
Copy Markdown

Closes #3298

Summary

  • add channel-test monitoring settings for retrying with stream after a non-stream test fails
  • retry default channel health checks with stream=true when the configured status codes and error keywords match
  • keep manual model test behavior unchanged when users explicitly choose endpoint type or stream mode
  • add monitoring UI fields and complete i18n entries for all supported frontend locales

Details

Summary

  • add channellicit manual_test=true flag so user-selected test behavior is preserved

Summary by CodeRabbit

  • New Features

    • Channel test stream retry: failed channel tests can automatically retry using streaming when configured criteria match.
    • Settings UI: enable/disable switch, status-code rules input with parsing/validation and keywords textarea; modal and manual/batch test flows honor the optional retry.
  • Tests

    • Unit tests for keyword parsing and retry decision logic.
  • Documentation

    • Added translations for new settings in multiple locales.

@coderabbitai

coderabbitai Bot commented Mar 18, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9ba1a2b1-fe24-40a1-8d73-b3351e9cd3ba

📥 Commits

Reviewing files that changed from the base of the PR and between 396c963 and 767bb7a.

📒 Files selected for processing (7)
  • controller/topup.go
  • web/src/i18n/locales/en.json
  • web/src/i18n/locales/fr.json
  • web/src/i18n/locales/ja.json
  • web/src/i18n/locales/ru.json
  • web/src/i18n/locales/vi.json
  • web/src/i18n/locales/zh-TW.json
💤 Files with no reviewable changes (1)
  • controller/topup.go
✅ Files skipped from review due to trivial changes (6)
  • web/src/i18n/locales/zh-TW.json
  • web/src/i18n/locales/fr.json
  • web/src/i18n/locales/en.json
  • web/src/i18n/locales/ja.json
  • web/src/i18n/locales/vi.json
  • web/src/i18n/locales/ru.json

Walkthrough

Adds an optional stream-retry mechanism for channel tests: when a non-stream test fails with configured HTTP status codes or error keywords, the backend may retry the same test with streaming enabled. Settings, UI controls, i18n entries, and unit tests were added to configure and validate this behavior.

Changes

Cohort / File(s) Summary
Backend: channel test logic
controller/channel-test.go
Adds shouldRetryChannelTestWithStream and testChannelWithOptionalStreamRetry; tracks statusCode and duration in testResult; integrates optional stream retry into TestChannel/testAllChannels flows and adjusts HTTP result handling.
Backend: monitor settings & parsing
setting/operation_setting/monitor_setting.go, setting/operation_setting/monitor_setting_test.go
Extends MonitorSetting with stream-retry fields, adds ParseMonitorKeywords and ShouldRetryChannelTestWithStream, and unit tests validating parsing and retry decision logic.
Backend: option validation
controller/option.go
Adds validation branch for monitor_setting.channel_test_stream_retry_status_codes parsing and returns structured error on invalid input.
Frontend: API call changes
web/src/hooks/channels/useChannelsData.jsx, web/src/components/table/channels/modals/ModelTestModal.jsx
Extends testChannel signature with optional manualTest param and updates call sites to pass the new argument (adds manual_test=true to single tests and batch flows).
Frontend: settings UI
web/src/components/settings/OperationSetting.jsx, web/src/pages/Setting/Operation/SettingsMonitoring.jsx
Adds UI controls (Switch, status-code rules input, keywords textarea), validation/parsing for status code rules, and persists normalized stream-retry settings.
Frontend: i18n
web/src/i18n/locales/{en,fr,ja,ru,vi,zh-CN,zh-TW}.json
Adds localization strings for the stream-retry feature: labels, descriptions, and validation messages across supported locales.
Misc: trivial
controller/topup.go
Whitespace cleanup (removed trailing blank line).

Sequence Diagram

sequenceDiagram
    participant User as User/Admin
    participant Frontend as Frontend
    participant API as Backend API
    participant Monitor as Monitor Setting
    participant Upstream as Upstream Service

    User->>Frontend: Click "Test Channel"
    Frontend->>API: POST /test-channel?manual_test=true (isStream=false)
    API->>Upstream: Send non-stream request
    Upstream-->>API: 400 + error text
    API->>Monitor: shouldRetryChannelTestWithStream(400, errText)
    Monitor-->>API: true
    API->>Upstream: Retry request with isStream=true
    Upstream-->>API: 200 OK
    API-->>Frontend: Return aggregated result (statusCode, duration)
    Frontend-->>User: Display test result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • seefs001
  • creamlike1024
  • Calcium-Ion

Poem

🐰 I sniffed the logs, a 400 cried,
"Stream must be true," the upstream sighed.
I hopped back once, then tried with flow,
Measured time and code to show.
Now the channel purrs — hop, retry, go! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: retry channel tests with stream when configured' accurately describes the main change: adding configurable stream retry logic for channel tests that fail non-stream attempts.
Linked Issues check ✅ Passed The PR comprehensively addresses issue #3298 by implementing configurable stream retry for channel tests with three new settings (enabled, status codes, keywords), allowing failed non-stream tests to retry with stream=true when conditions match.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing the stream retry feature: backend logic in Go (retry decision, test wrapper, monitor settings), frontend UI/state management, and i18n translations. One unrelated trailing newline removal in topup.go is negligible.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@controller/channel-test.go`:
- Around line 52-64: The wrapper testChannelWithOptionalStreamRetry currently
hides a retry and returns only the first attempt's timing, causing callers
(e.g., UpdateResponseTime and ChannelDisableThreshold) to include the failed
probe in latency; change testChannelWithOptionalStreamRetry to return the actual
successful-attempt result or a retried flag (e.g., return (testResult, bool
retried) or include a duration in testResult), adjust callers that call
testChannelWithOptionalStreamRetry to use the successful attempt's duration for
UpdateResponseTime/ChannelDisableThreshold, and ensure the retry path (when
shouldRetryChannelTestWithStream(result) is true and you call testChannel(...,
true)) propagates the second call's timing and sets retried=true so the
intentional first failure is not used for health-check latency.
- Around line 45-49: The retry logic is checking result.newAPIError.StatusCode
(shouldRetryChannelTestWithStream) but upstream errors are being wrapped as
http.StatusInternalServerError in testChannel, so preserve the original upstream
status before wrapping: ensure testChannel stores the upstream HTTP status (e.g.
as a field on testResult or by setting newAPIError.StatusCode to the upstream
status) so shouldRetryChannelTestWithStream sees the real upstream code; update
either testChannel's error construction or testResult to carry upstreamStatus
and make shouldRetryChannelTestWithStream consult that upstreamStatus (or the
preserved StatusCode) when calling
operation_setting.ShouldRetryChannelTestWithStream.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c098eba8-1706-4708-951e-c03a7ca1aeed

📥 Commits

Reviewing files that changed from the base of the PR and between 8ed2ea6 and add81b8.

📒 Files selected for processing (15)
  • controller/channel-test.go
  • controller/option.go
  • setting/operation_setting/monitor_setting.go
  • setting/operation_setting/monitor_setting_test.go
  • web/src/components/settings/OperationSetting.jsx
  • web/src/components/table/channels/modals/ModelTestModal.jsx
  • web/src/hooks/channels/useChannelsData.jsx
  • web/src/i18n/locales/en.json
  • web/src/i18n/locales/fr.json
  • web/src/i18n/locales/ja.json
  • web/src/i18n/locales/ru.json
  • web/src/i18n/locales/vi.json
  • web/src/i18n/locales/zh-CN.json
  • web/src/i18n/locales/zh-TW.json
  • web/src/pages/Setting/Operation/SettingsMonitoring.jsx

Comment thread controller/channel-test.go Outdated
Comment thread controller/channel-test.go
@ghost

This comment was marked as spam.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

渠道自动测试硬编码为非流式,导致强制流式的渠道(如部分 Codex/Azure 接口)测试失败并无法自动启用/重试

1 participant