为 API v4 接入 FastIP 预热与直连优化 - #177
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Walkthrough在 ipgeo 中新增 NextTrace API v4 FastIP 预热与按端点 HTTP 客户端工厂，并在 cmd 和 service 初始化路径按 NextTrace APIV4 token 配置触发预热；新增/调整大量单元测试覆盖代理、超时与回退场景。 ChangesNextTrace API v4 FastIP 预热与集成
Sequence DiagramsequenceDiagram
participant Init as initLeoWebsocket / withServiceRuntime
participant Token as NextTraceAPIV4TokenConfigured
participant Prepare as PrepareNextTraceAPIV4FastIP
participant ClientFactory as nextTraceAPIV4HTTPClientFactory
participant Transport as newNextTraceAPIV4HTTPClient
participant Dial as dialNextTraceAPIV4
participant Geo as util.LookupHostForGeo
Init->>Token: 检查 v4 token
Token-->>Init: true
Init->>Prepare: 调用(ctx, enableOutput)
Prepare->>ClientFactory: 请求按 endpoint 创建客户端
ClientFactory->>Transport: 返回带自定义 Dial 的 Transport
Transport->>Dial: DialContext 发起拨号
Dial->>Prepare: 查询 FastIP 缓存并尝试直连
Dial->>Geo: 回退到 Geo DNS 并逐个拨号
估计审阅工作量🎯 4 (复杂) | ⏱️ ~45 分钟 可能相关的 PR
诗歌
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@ipgeo/nexttrace_api_v4.go`:
- Around line 246-251: When a cached FastIP exists (util.GetFastIPCache) the
code currently returns the DialContext error directly; change this so you
attempt dialer.DialContext(ctx, network, net.JoinHostPort(fastIP, port)) but if
that call returns a non-nil error fall back to calling dialGeoResolved(ctx,
dialer, network, host, port) instead of returning the error immediately; update
the branch that checks strings.EqualFold(host, apiHost) && port == apiPort to
perform the fallback logic using the same fastIP, dialer.DialContext, and
dialGeoResolved symbols.
- Around line 205-208: The port defaulting logic currently sets an implicit port
to nextTraceAPIV4DefaultPort regardless of scheme; change it so when port :=
u.Port() is empty you pick "80" for u.Scheme == "http" and
nextTraceAPIV4DefaultPort (likely "443") for other schemes (i.e., check u.Scheme
and assign port = "80" when scheme == "http", else port =
nextTraceAPIV4DefaultPort) in the same block where port and u are used.
- Around line 177-190: PrepareNextTraceAPIV4FastIP currently passes the caller's
ctx through, which can block forever; modify prepareNextTraceAPIV4FastIP to
create a bounded child context (e.g., context.WithTimeout(ctx, shortDuration))
before calling nextTraceAPIV4FastIPFn, defer the cancel(), and pass that timed
ctx into nextTraceAPIV4FastIPFn so the FastIP prewarm is bounded; keep the
existing host/port checks and return behavior unchanged.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 84fd279f-cefb-4bd9-a442-7968bbc10555
📒 Files selected for processing (6)
cmd/cmd.gocmd/cmd_test.gointernal/service/service.gointernal/service/service_test.goipgeo/nexttrace_api_v4.goipgeo/nexttrace_api_v4_test.go
There was a problem hiding this comment.
Pull request overview
为 NextTrace API v4 的 ipGeo HTTP 路径补齐 FastIP 预热与“直连已探测 FastIP”的拨号逻辑，并在 v4 token 模式下让 CLI / service runtime 不再初始化 v3 WebSocket，而是改为预热 v4 FastIP；同时补充对应测试覆盖，以统一 v3/v4 的连接优化行为边界（默认 endpoint / 自定义 endpoint / proxy）。
Changes:
- 为 v4 默认 endpoint 构建 FastIP-aware 的 HTTP client，并新增 v4 FastIP 预热入口（含 proxy / custom endpoint 分支）。
- CLI 与 service runtime 在检测到 v4 token 时改为预热 v4 FastIP，跳过 v3 WebSocket 初始化路径。
- 增加 v4 FastIP、custom endpoint、proxy、CLI/runtime 分支相关的单测覆盖。
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| ipgeo/nexttrace_api_v4.go | 引入 v4 FastIP 预热与基于 FastIP cache 的 DialContext 逻辑，并调整 client factory 签名以支持 endpoint-aware client 创建。 |
| ipgeo/nexttrace_api_v4_test.go | 新增/更新测试覆盖：默认 endpoint 使用 FastIP、自定义 endpoint 跳过 FastIP、proxy 场景走代理等。 |
| internal/service/service.go | service runtime：v4 token 模式下预热 FastIP，避免初始化 v3 WebSocket。 |
| internal/service/service_test.go | 增加 v4 token 模式下应调用 FastIP 预热且不触发 LeoWS 初始化的测试，并加强测试环境隔离。 |
| cmd/cmd.go | CLI 初始化：v4 token 模式下调用 FastIP 预热后直接返回，跳过 v3 WebSocket 初始化。 |
| cmd/cmd_test.go | 增加 CLI 分支测试，断言 v4 token 模式下会调用 FastIP 预热。 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func nextTraceAPIV4APIEndpointHostPort(endpoint string) (string, string, bool) { | ||
| u, err := url.Parse(normalizeNextTraceAPIV4Endpoint(endpoint)) | ||
| if err != nil { | ||
| return "", "", false | ||
| } | ||
| scheme := strings.ToLower(u.Scheme) | ||
| if scheme != "https" && scheme != "http" { | ||
| return "", "", false | ||
| } | ||
| if !strings.EqualFold(u.Hostname(), nextTraceAPIV4APIHost) || u.EscapedPath() != nextTraceAPIV4GeoPath { | ||
| return "", "", false | ||
| } | ||
| port := u.Port() | ||
| if port == "" { | ||
| port = nextTraceAPIV4DefaultPort | ||
| } | ||
| return nextTraceAPIV4APIHost, port, true |
| fastIPCtx, cancelFastIP := context.WithTimeout(context.Background(), timeout) | ||
| _ = prepareNextTraceAPIV4FastIP(fastIPCtx, nextTraceAPIV4GeoEndpoint, true) | ||
| cancelFastIP() | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), timeout) | ||
| defer cancel() |
| timeout = normalizeNextTraceAPIV4Timeout(timeout) | ||
| client := cachedNextTraceAPIV4Client(nextTraceAPIV4GeoEndpoint, util.GetNextTraceAPIV4Token(), timeout) | ||
| fastIPCtx, cancelFastIP := context.WithTimeout(context.Background(), timeout) | ||
| _ = prepareNextTraceAPIV4FastIP(fastIPCtx, nextTraceAPIV4GeoEndpoint, true) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b81f0365f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fastIPCtx, cancelFastIP := context.WithTimeout(context.Background(), timeout) | ||
| _ = prepareNextTraceAPIV4FastIP(fastIPCtx, nextTraceAPIV4GeoEndpoint, true) | ||
| cancelFastIP() | ||
|
|
||
| ctx, cancel := context.WithTimeout(context.Background(), timeout) |
There was a problem hiding this comment.
This now spends one full timeout warming FastIP and then starts a second full-timeout lookup, so a single LeoIPNextTraceAPIV4HTTP call can block for roughly 2× the caller’s budget on cache misses or slow FastIP probes. Before this change the timeout bounded the whole call path, so callers that rely on timeout as an end-to-end SLA will see unexpected latency inflation.
Useful? React with 👍 / 👎.
| if !ok || util.GetProxy() != nil { | ||
| return nil |
There was a problem hiding this comment.
The prewarm skip condition only checks NEXTTRACE_PROXY via util.GetProxy(), but the HTTP transport is cloned from http.DefaultTransport, which still honors HTTP_PROXY/HTTPS_PROXY (ProxyFromEnvironment). In environments using standard proxy env vars, this path still runs FastIP probing even though the actual API request is proxied, adding avoidable delay and direct outbound probe traffic that bypasses the configured proxy policy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ipgeo/nexttrace_api_v4.go (1)
43-45: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win默认工厂别绕过
util.NewGeoHTTPClient(...)。这里把默认路径统一切到
newNextTraceAPIV4HTTPClient了，但该工厂最终是手工组装http.Client/Transport。这样会把 Geo HTTP 客户端已有的默认行为分叉出去。建议先从util.NewGeoHTTPClient(timeout)派生，再只覆写这次需要的Proxy/DialContext。As per coding guidelines "Geo HTTP requests must use
util.NewGeoHTTPClient(...)with Transport cloned from default Transport to preserve proxy/HTTP2/connection pool behavior."Also applies to: 74-75, 131-131
🤖 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 `@ipgeo/nexttrace_api_v4.go` around lines 43 - 45, The default factory nextTraceAPIV4HTTPClientFactory currently returns a bespoke http.Client assembled by newNextTraceAPIV4HTTPClient, which bypasses util.NewGeoHTTPClient(...) and loses shared proxy/HTTP2/connection-pool behavior; change newNextTraceAPIV4HTTPClient to call util.NewGeoHTTPClient(timeout) (or clone its Transport from http.DefaultTransport) and then only override the specific Transport fields you need (Proxy and DialContext) before returning the client, and update any other factory usages that create clients (the other factory assignments referenced) to follow the same pattern so all Geo HTTP clients preserve the default transport 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 `@ipgeo/nexttrace_api_v4_test.go`:
- Around line 410-414: In TestDialNextTraceAPIV4FallsBackWhenCachedFastIPFails
replace net.Listen("tcp", "127.0.0.1:0") with net.ListenConfig{}.Listen to
create the listener so context-aware listen is used, and change the defer to
capture and handle listener.Close()’s error (e.g., defer func() { if err :=
listener.Close(); err != nil { t.Fatalf("listener.Close() error = %v", err) }
}()) to satisfy errcheck/noctx; ensure you update the variable name `listener`
and any surrounding error handling in that test accordingly.
---
Outside diff comments:
In `@ipgeo/nexttrace_api_v4.go`:
- Around line 43-45: The default factory nextTraceAPIV4HTTPClientFactory
currently returns a bespoke http.Client assembled by
newNextTraceAPIV4HTTPClient, which bypasses util.NewGeoHTTPClient(...) and loses
shared proxy/HTTP2/connection-pool behavior; change newNextTraceAPIV4HTTPClient
to call util.NewGeoHTTPClient(timeout) (or clone its Transport from
http.DefaultTransport) and then only override the specific Transport fields you
need (Proxy and DialContext) before returning the client, and update any other
factory usages that create clients (the other factory assignments referenced) to
follow the same pattern so all Geo HTTP clients preserve the default transport
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5c895ba3-983a-4cd5-84e9-89eb8727d427
📒 Files selected for processing (2)
ipgeo/nexttrace_api_v4.goipgeo/nexttrace_api_v4_test.go
Summary
LeoMoeAPI的 NextTrace API v4 HTTP 路径补上 FastIP 预热与复用逻辑。api.nxtrace.org/v4/ipGeo请求在无代理场景下优先拨到已探测的 FastIP，同时保留原有 URL、Host、SNI 和 token 行为。Motivation
v3 WebSocket 已经复用 FastIP 优化连接，但 v4 HTTP GeoIP 路径没有走同样的建连逻辑，导致两条 API 路径的连接优化不一致。
Changes
ipgeo中新增 v4 FastIP 预热入口，并为默认 API endpoint 构建 FastIP-aware HTTP client。cmd和internal/service的 runtime 选择中，v4 token 模式下改为预热 FastIP，并补充对应单测。Testing
go test ./ipgeo ./cmd ./internal/servicego test ./...git diff --checkNotes for Reviewers
pull request create failed: GraphQL: Head sha can't be blank, Base sha can't be blank, No commits between main and codex/api-v4-fastip, Head ref must be a branch (createPullRequest)
无法创建拉取请求
Summary by CodeRabbit
发布说明
新功能
改进
测试