fix(channel): improve proxy client compatibility and cache lifecycle - #6157
Conversation
WalkthroughThe PR adds shared proxy URL parsing and canonical client caching, migrates proxied callers to the shared accessor, narrows cache invalidation to affected channels, validates cloned and updated settings, reports actual batch deletion counts, and documents proxy compatibility rules. ChangesProxy handling
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/default/src/features/channels/lib/channel-form.ts (1)
39-70: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueProxy validation logic looks correct and matches backend contract.
Cross-checked against
common/proxy_url.go'sparseProxyURL(strict mode): scheme whitelist, required host, "empty or root-only" path, and rejection of query/fragment are all mirrored correctly here.One minor note: port validity relies on
URL's own range enforcement rather than an explicit1-65535check (matching backend'sstrconv.Atoibounds). This is likely fine in practice sinceURLrejects out-of-range ports, but worth keeping in mind if browser URL parsing behavior ever diverges.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/channels/lib/channel-form.ts` around lines 39 - 70, Keep the isOptionalProxyURL validation aligned with common/proxy_url.go’s strict parseProxyURL contract: retain the supported-scheme whitelist, required hostname, empty-or-root-only path rule, query/fragment rejection, and URL-based port validation. No code change is required unless browser URL parsing diverges from the backend’s port-range behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@controller/channel_test_internal_test.go`:
- Around line 131-142: Import testify/assert and replace non-fatal value checks
with assert in the affected tests: response.Success, response.Data, and
auditData.Operation.Params["count"] in the shown test, plus require.NotSame in
TestDeleteChannelResetsProxyCacheWhenPreReadFails and
require.Contains/require.Equal in
TestCopyChannelRejectsInvalidLegacyProxySettings. Keep require for setup and
error checks, including require.NoError and database reads, and apply the same
value-check change in TestDeleteChannelBatchReportsAndAuditsActualDeletedCount.
---
Nitpick comments:
In `@web/default/src/features/channels/lib/channel-form.ts`:
- Around line 39-70: Keep the isOptionalProxyURL validation aligned with
common/proxy_url.go’s strict parseProxyURL contract: retain the supported-scheme
whitelist, required hostname, empty-or-root-only path rule, query/fragment
rejection, and URL-based port validation. No code change is required unless
browser URL parsing diverges from the backend’s port-range behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 581e6a9f-c948-4206-9ea0-f8224608bbc0
📒 Files selected for processing (26)
common/proxy_url.gocontroller/channel-billing.gocontroller/channel.gocontroller/channel_test_internal_test.gocontroller/channel_upstream_update.gocontroller/codex_usage.godocs/channel/other_setting.mdmodel/channel.gorelay/channel/api_request.gorelay/channel/aws/relay-aws.gorelay/channel/coze/relay-coze.gorelay/channel/vertex/service_account.gorelay/mjproxy_handler.goservice/codex_credential_refresh.goservice/codex_credential_refresh_task.goservice/http_client.goweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/channels/constants.tsweb/default/src/features/channels/lib/channel-form.tsweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh-TW.jsonweb/default/src/i18n/locales/zh.json
💤 Files with no reviewable changes (3)
- controller/channel_upstream_update.go
- service/codex_credential_refresh.go
- service/codex_credential_refresh_task.go
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
controller/channel_upstream_update.go (1)
320-320: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winUse the shared proxy client accessor to benefit from connection pooling.
Using
service.NewProxyHttpClientcircumvents the concurrent safe proxy cache introduced in this PR. This instantiates a newhttp.Client(and potentially a new underlyinghttp.Transport) for every model fetch request, which fails to reuse connections and may lead to socket exhaustion over time.Please switch to the cached accessor to utilize the new cache mechanism.
⚡ Proposed fix
- client, err := service.NewProxyHttpClient(channel.GetSetting().Proxy) + client, err := service.GetHttpClientWithProxy(channel.GetSetting().Proxy)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@controller/channel_upstream_update.go` at line 320, Replace the direct service.NewProxyHttpClient call in the channel upstream update flow with the shared cached proxy client accessor, passing channel.GetSetting().Proxy so model fetch requests reuse pooled connections and transports.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@controller/channel_upstream_update.go`:
- Line 320: Replace the direct service.NewProxyHttpClient call in the channel
upstream update flow with the shared cached proxy client accessor, passing
channel.GetSetting().Proxy so model fetch requests reuse pooled connections and
transports.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 995ef0ed-9954-4eb2-a77e-4b36789abf8e
📒 Files selected for processing (3)
controller/channel.gocontroller/channel_upstream_update.gomodel/channel.go
🚧 Files skipped from review as they are similar to previous changes (2)
- model/channel.go
- controller/channel.go
同步上游 10 个提交(至 1721144),重点整合: - QuantumNous#6329 鉴权重构:dashboard 会话全面改为无状态 token(access/refresh + 版本栅栏 + 会话管理),gin session 全部移除。fork 侧适配: - turnstile 一次性消费改为按 token 键控内存缓存(兼容发码+注册两步流) - TRUSTED_PROXY_CIDRS 作为 TRUSTED_PROXIES 的兼容别名保留 - UserBase/ToBaseUser 保留 ParentId(子号计费),补 AuthVersion/CacheSchema - OAuth 绑定改 flow_token 流,保留 GitHub 账号年龄门禁(消费 flow 后校验) - RecordUserIP 反欺诈埋点移入 setupLoginAtAuthVersion - 子号/代理鉴权门(SubPermission/RejectSubAccount/AgentAuth)原样保留 - profile 嫁接上游 LoginSessionsCard(会话管理 UI),绑定卡接入 popup+postMessage 新绑定机制 - 2FA/OAuth/微信登录后 redirect 目标经 handleLoginSuccess 传递恢复 - web/default → web/ 扁平化 + 删除 classic 主题:fork 全部前端定制 (agent/supplier/detector/sub-account 等 180+ 文件)迁移至新路径, 保留 fork 的 i18n 按需加载、每表分页记忆、主题调校与设计系统 - QuantumNous#6157 渠道代理客户端重构(别名缓存+失效清理),保留 fork 全局代理 与 RELAY_DISABLE_HTTP2;QuantumNous#6074 suno CAS 防重复退款;QuantumNous#6163 playground 自动分组;QuantumNous#6224 无限额度密钥显示已用量;QuantumNous#6032 realtime GA 去 beta 头 - 语言文件三方合并:fork ~6280 键 + 上游新增 55 键鉴权文案 - fork 刻意删除的组件与 workflows 维持删除(上次合并曾误恢复) 验证:go build/test 全绿,前端 tsgo 类型检查与 rsbuild 构建通过。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uantumNous#6157) * fix(channel): improve proxy client compatibility and cache lifecycle * test(controller): use non-fatal assertions for channel tests
…uantumNous#6157) * fix(channel): improve proxy client compatibility and cache lifecycle * test(controller): use non-fatal assertions for channel tests
…uantumNous#6157) * fix(channel): improve proxy client compatibility and cache lifecycle * test(controller): use non-fatal assertions for channel tests
Important
📝 变更描述 / Description
(简述:做了什么?为什么这样改能生效?请基于你对代码逻辑的理解来写,避免粘贴未经整理的内容)
优化渠道代理客户端的创建、复用与缓存失效逻辑,补充 HTTP、HTTPS、SOCKS5 和 SOCKS5H 的统一校验及规范化处理。
运行时兼容旧版带 path、query 或 fragment 的代理配置,避免升级后中断现有流量;保存时继续执行严格校验。SOCKS 拨号支持 context 取消和连接超时,并通过并发安全缓存减少重复 Transport 创建及不必要的全量重置。前端同步代理校验规则,避免保存时意外改写代理地址。
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告,以证明变更生效)
Summary by CodeRabbit
New Features
thinking_to_contentchannel setting for converting reasoning content into tagged output.Bug Fixes
Documentation