fix: Use channel proxy settings for task query scenarios - #2398
Conversation
WalkthroughThis PR adds proxy support to video task status fetching by introducing a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ 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: 0
🧹 Nitpick comments (2)
service/http_client.go (1)
82-85: Consider aligning empty-proxy behavior withGetHttpClientWithProxy.
NewProxyHttpClient("")returnshttp.DefaultClient, whileGetHttpClientWithProxy("")returns the properly initializedhttpClientfromGetHttpClient(). This inconsistency is unlikely to cause issues since callers should useGetHttpClientWithProxy, but could be confusing for future maintainers.func NewProxyHttpClient(proxyURL string) (*http.Client, error) { if proxyURL == "" { - return http.DefaultClient, nil + return GetHttpClient(), nil }relay/channel/task/vertex/adaptor.go (1)
123-127: Guard against nil ChannelSetting when deriving proxyThe
proxyextraction is fine, but ifinfo.ChannelSettingis ever a pointer type that can be nil,info.ChannelSetting.Proxy(Line [125]) would panic. If there’s any doubt, consider an additional nil-check aroundChannelSettingor centralizing proxy resolution to one helper.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
controller/task.go(1 hunks)controller/task_video.go(2 hunks)controller/video_proxy.go(3 hunks)controller/video_proxy_gemini.go(1 hunks)relay/channel/adapter.go(1 hunks)relay/channel/task/ali/adaptor.go(2 hunks)relay/channel/task/doubao/adaptor.go(2 hunks)relay/channel/task/gemini/adaptor.go(2 hunks)relay/channel/task/hailuo/adaptor.go(2 hunks)relay/channel/task/jimeng/adaptor.go(2 hunks)relay/channel/task/kling/adaptor.go(2 hunks)relay/channel/task/sora/adaptor.go(2 hunks)relay/channel/task/suno/adaptor.go(2 hunks)relay/channel/task/vertex/adaptor.go(4 hunks)relay/channel/task/vidu/adaptor.go(2 hunks)relay/relay_task.go(1 hunks)service/http_client.go(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
relay/channel/task/jimeng/adaptor.go (2)
relay/channel/adapter.go (1)
TaskAdaptor(34-53)service/http_client.go (1)
GetHttpClientWithProxy(62-67)
relay/channel/task/suno/adaptor.go (2)
relay/channel/adapter.go (1)
TaskAdaptor(34-53)service/http_client.go (1)
GetHttpClientWithProxy(62-67)
relay/channel/task/doubao/adaptor.go (2)
relay/channel/adapter.go (1)
TaskAdaptor(34-53)service/http_client.go (1)
GetHttpClientWithProxy(62-67)
relay/channel/task/gemini/adaptor.go (2)
relay/channel/adapter.go (1)
TaskAdaptor(34-53)service/http_client.go (1)
GetHttpClientWithProxy(62-67)
relay/channel/task/vidu/adaptor.go (1)
service/http_client.go (1)
GetHttpClientWithProxy(62-67)
relay/channel/task/kling/adaptor.go (2)
relay/channel/adapter.go (1)
TaskAdaptor(34-53)service/http_client.go (1)
GetHttpClientWithProxy(62-67)
relay/channel/task/ali/adaptor.go (2)
relay/channel/adapter.go (1)
TaskAdaptor(34-53)service/http_client.go (1)
GetHttpClientWithProxy(62-67)
service/http_client.go (1)
common/constants.go (2)
RelayMaxIdleConns(124-124)RelayMaxIdleConnsPerHost(125-125)
relay/channel/task/sora/adaptor.go (2)
relay/channel/adapter.go (1)
TaskAdaptor(34-53)service/http_client.go (1)
GetHttpClientWithProxy(62-67)
relay/channel/task/vertex/adaptor.go (3)
relay/channel/vertex/service_account.go (1)
AcquireAccessToken(143-149)relay/channel/adapter.go (1)
TaskAdaptor(34-53)service/http_client.go (1)
GetHttpClientWithProxy(62-67)
relay/channel/task/hailuo/adaptor.go (1)
service/http_client.go (1)
GetHttpClientWithProxy(62-67)
controller/video_proxy.go (2)
service/http_client.go (1)
GetHttpClientWithProxy(62-67)logger/logger.go (1)
LogError(65-67)
🔇 Additional comments (17)
relay/relay_task.go (1)
329-337: LGTM! Proxy correctly passed to FetchTask for task status queries.The changes properly extract the proxy setting from channel configuration and pass it to
adaptor.FetchTask, which aligns with the PR objective to fix task status queries not using the channel proxy. This ensures GET requests for task status will route through the configured proxy (e.g., socks5://127.0.0.1:1080).service/http_client.go (1)
61-67: LGTM! Clean abstraction for proxy-enabled HTTP clients.The new
GetHttpClientWithProxyfunction provides a convenient wrapper that returns the default client when no proxy is configured, or a proxy-enabled client otherwise. This aligns well with the PR's goal of threading proxy settings through task fetch calls.controller/task.go (1)
119-122: LGTM! Proxy support added for Suno task polling.The proxy is correctly extracted from channel settings and passed to
adaptor.FetchTask, ensuring that background task status polling for Suno tasks also routes through the configured proxy.relay/channel/adapter.go (1)
49-50: LGTM! Interface updated to support proxy parameter.The
FetchTasksignature change enables all task adaptors to receive proxy settings. This is a breaking change for any external implementations ofTaskAdaptor, but necessary to fulfill the PR objective of routing task queries through channel proxies.relay/channel/task/jimeng/adaptor.go (1)
213-258: LGTM! Proxy support correctly implemented for Jimeng adaptor.The
FetchTaskmethod properly:
- Accepts the new
proxyparameter per the updated interface- Creates a proxy-enabled HTTP client via
GetHttpClientWithProxy(proxy)- Handles client creation errors appropriately
- Uses the proxy-enabled client to execute the request
This implementation correctly enables task status queries to route through the channel's configured proxy.
controller/task_video.go (1)
70-80: LGTM! Proxy support correctly added for task status queries.The changes properly extract the proxy from channel settings and pass it to
FetchTask, ensuring that task status queries now respect the channel's configured proxy. This directly addresses the issue where GET requests for task status were timing out due to missing proxy configuration.relay/channel/task/hailuo/adaptor.go (1)
113-134: LGTM! Proxy-enabled task fetching correctly implemented.The implementation follows the correct pattern:
- Accepts proxy parameter in the updated signature
- Creates a proxy-aware HTTP client via
GetHttpClientWithProxy- Handles client creation errors appropriately
- Uses the proxy-enabled client for the request
This ensures Hailuo task status queries will route through the configured channel proxy.
relay/channel/task/gemini/adaptor.go (1)
203-231: LGTM! Gemini adaptor correctly implements proxy support.The changes properly integrate proxy support into the Gemini task fetching flow. The implementation is consistent with other adaptors and ensures that Gemini video generation task queries will use the channel's configured proxy.
relay/channel/task/suno/adaptor.go (1)
135-161: LGTM! Proxy support correctly added for Suno task fetching.The implementation properly integrates proxy support following the same pattern as other adaptors. The proxy-enabled client will ensure Suno task status queries route through the configured channel proxy.
relay/channel/task/doubao/adaptor.go (1)
149-171: LGTM! Doubao adaptor correctly implements proxy support.The changes properly add proxy support to the Doubao task fetching implementation. The pattern is consistent with other adaptors and ensures task status queries will respect the channel's proxy configuration.
controller/video_proxy.go (1)
80-105: LGTM! Video proxy endpoint now correctly uses channel proxy settings.The implementation properly:
- Extracts proxy from channel settings
- Creates a proxy-aware HTTP client with comprehensive error handling
- Adds a 60-second timeout context for resilience
- Provides clear error messages for proxy-related failures
This ensures video content retrieval will route through the configured channel proxy, addressing the core issue.
relay/channel/task/sora/adaptor.go (1)
128-148: LGTM! Sora adaptor correctly implements proxy support.The changes properly integrate proxy support into the Sora task fetching flow. The implementation is consistent with other adaptors and ensures task status queries will use the channel's configured proxy settings.
controller/video_proxy_gemini.go (1)
38-42: LGTM! Gemini video URL resolution now uses channel proxy.The changes correctly extract the proxy from channel settings and pass it to the Gemini adaptor's
FetchTaskmethod, ensuring that Gemini video URL resolution respects the channel's proxy configuration.relay/channel/task/ali/adaptor.go (1)
395-416: Proxy-enabled FetchTask implementation looks correct
FetchTasknow correctly routes the GET throughservice.GetHttpClientWithProxy(proxy)and handles client-construction failures before executingclient.Do(req). This satisfies the requirement that task status queries respect the channel proxy while remaining a no-op whenproxy == "".relay/channel/task/vertex/adaptor.go (1)
222-273: Vertex FetchTask now fully proxy-aware (token + HTTP client)
FetchTaskcorrectly uses the suppliedproxyboth forvertexcore.AcquireAccessToken(*adc, proxy)and for the HTTP client viaservice.GetHttpClientWithProxy(proxy), with proper error propagation on client construction. This aligns with the intended behavior that operation polling goes through the channel’s proxy.relay/channel/task/vidu/adaptor.go (1)
191-212: Vidu FetchTask proxy plumbing is consistent and correct
FetchTasknow obtains its client viaservice.GetHttpClientWithProxy(proxy)and surfaces construction errors with context, while preserving existing URL and header logic. This ensures Vidu task status polling respects channel proxy settings.relay/channel/task/kling/adaptor.go (1)
201-236: Kling FetchTask correctly migrated to proxy-aware clientThe updated
FetchTaskkeeps the existing URL/JWT behavior and swaps execution toservice.GetHttpClientWithProxy(proxy)with proper error handling. This brings Kling task polling in line with other adaptors for honoring channel proxy configuration.
fix: Use channel proxy settings for task query scenarios
fix #2392
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.