cmd/residential-urltest: in-region reachability + throughput repro tool#540
Open
myleshorton wants to merge 1 commit into
Open
cmd/residential-urltest: in-region reachability + throughput repro tool#540myleshorton wants to merge 1 commit into
myleshorton wants to merge 1 commit into
Conversation
Standalone debugging harness that url-tests (and now throughput-tests) a v9 client's sing-box outbounds from a chosen censored country's *residential* IP, to reproduce "connected but no traffic" reports. Reuses the exact v9 stack (lantern-box outbound registry + sing-box MutableURLTest) so custom protocols (samizdat, shadowsocks, vless+REALITY, anytls, reflex) are constructed the same way the client builds them; each proxy's server dial is routed through a residential HTTP-CONNECT detour (oxylabs / packetstream / brightdata). Key: `--throughput` downloads several MB through each reachable outbound and reports achieved KB/s. A url-test only proves handshake + first byte, which a *throttling* censor (RU TSPU) sails past — sustained throughput is what exposes it. Build: `go build -tags with_utls -o /tmp/rut ./cmd/residential-urltest` (the tag is required so vless+REALITY outbounds don't abort sing-box init). Used by the getlantern/skills `region-protocol-test` / `residential-proxy` skills, which previously referenced this tool while it was untracked here. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
📝 WalkthroughWalkthroughAdds a standalone CLI that parses sing-box config, skips unsupported outbounds, routes eligible proxies through residential HTTP-CONNECT detours, runs URL tests, prints reachability and latency, and optionally measures sustained throughput for reachable outbounds. ChangesResidential URL test CLI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new standalone CLI harness under cmd/ to reproduce “connected but no traffic” reports by URL-testing (and optionally throughput-testing) the same sing-box outbounds configuration used by the v9 client, from a residential HTTP-CONNECT detour.
Changes:
- Introduces
cmd/residential-urltestto parse an outbounds JSON, attach a residential HTTP proxy detour per outbound, and run sing-box URL tests. - Adds an optional sustained download throughput phase to detect data-plane throttling that a handshake/first-byte probe can miss.
- Implements provider-specific gateway/auth construction for oxylabs / brightdata / packetstream using env-provided credentials.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+15
to
+16
| // export OXY_USER=... OXY_PASS=... # oxylabs residential creds (vault: secret/lantern_cloud/pinger) | ||
| // go run ./cmd/residential-urltest --config /tmp/ticket-176866/attachments/debug-box-options.json --country ru |
Comment on lines
+120
to
+122
| if !*direct { | ||
| ob["detour"] = fmt.Sprintf("residential-%d", len(tags)%*poolN) | ||
| } |
Comment on lines
+140
to
+145
| seed := rand.Int63() | ||
| for i := 0; i < *poolN; i++ { | ||
| keep = append(keep, map[string]any{ | ||
| "type": "http", "tag": fmt.Sprintf("residential-%d", i), | ||
| "server": gwIPs[0], "server_port": gwPort, | ||
| "username": mkLogin(fmt.Sprintf("%d%d", seed, i)), "password": secret, |
Comment on lines
+294
to
+305
| cc := strings.ToLower(country) | ||
| host, port := "", 0 | ||
| if gwOverride != "" { | ||
| h, ps, err := net.SplitHostPort(gwOverride) | ||
| if err != nil { | ||
| return "", 0, nil, "", fmt.Errorf("bad --gw: %w", err) | ||
| } | ||
| host = h | ||
| port, _ = strconv.Atoi(ps) | ||
| } | ||
| switch provider { | ||
| case "brightdata", "brd": |
Comment on lines
+259
to
+265
| resp, err := client.Get(url) | ||
| if err != nil { | ||
| return 0, 0, time.Since(start), err | ||
| } | ||
| defer resp.Body.Close() | ||
| got, err = io.Copy(io.Discard, io.LimitReader(resp.Body, int64(maxBytes))) | ||
| dur = time.Since(start) |
Comment on lines
+162
to
+166
| // Mirror radiance RunOfflineURLTests setup. | ||
| ctx = service.ContextWith[filemanager.Manager](ctx, nil) | ||
| hist := urltest.NewHistoryStorage() | ||
| ctx = service.ContextWithPtr(ctx, hist) | ||
| service.MustRegister[adapter.URLTestHistoryStorage](ctx, hist) |
Comment on lines
+312
to
+314
| if p := os.Getenv("BRD_PORT"); p != "" { | ||
| port, _ = strconv.Atoi(p) | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
cmd/residential-urltest/main.go (1)
1-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlign Go doc comments with the project convention.
The package comment and these unexported filter comments are useful, but they do not start with the documented identifier names.
Proposed doc-comment cleanup
-// residential-urltest is a standalone reproduction tool: it url-tests a v9 +// Package main contains residential-urltest, a standalone reproduction tool: it url-tests a v9 // Lantern client's sing-box outbounds from a chosen country's *residential* IP,-// infra/non-proxy outbound types to skip. +// skipTypes lists infra/non-proxy outbound types to skip. var skipTypes = map[string]bool{-// UDP-based protocols that can't be carried over an HTTP-CONNECT (TCP) detour. +// udpTypes lists UDP-based protocols that can't be carried over an HTTP-CONNECT (TCP) detour. var udpTypes = map[string]bool{"hysteria": true, "hysteria2": true, "tuic": true}As per coding guidelines, Go doc comments must start with the identifier’s name and package-level comments should use
// Package foo ....Also applies to: 56-63
🤖 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 `@cmd/residential-urltest/main.go` around lines 1 - 24, The package-level comment in main.go and the nearby unexported filter comments need to follow Go doc conventions. Update the top comment to start with “Package residential-urltest …” and make each affected comment begin with the identifier name it documents, using the symbols in this file rather than generic prose. Keep the wording otherwise the same, but ensure the comment style matches the project’s documented naming convention.Source: Coding guidelines
🤖 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 `@cmd/residential-urltest/main.go`:
- Around line 167-171: The throughput path is using the single sing-box context
deadline from the residential URL test harness, which can expire mid-loop and
falsely mark later outbounds as STALLED. Update the setup in main and the
sequential measurement flow so the instance context stays alive until cleanup,
then apply separate per-operation timeouts inside URLTest and each throughput
request instead of relying on the shared 150s deadline. Use the URLTest and
throughput request handling code to locate the timeout logic and keep the outer
context from governing all downloads.
- Around line 297-303: The gateway port parsing in the `main`/config setup
ignores `strconv.Atoi` failures for both `gwOverride` and `BRD_PORT`, so invalid
values fall through as port 0. Update the parsing logic around
`net.SplitHostPort`, `strconv.Atoi`, and the `BRD_PORT` handling to check
conversion errors and return a clear configuration error instead of silently
continuing. Use the existing gateway parsing path and `host`/`port` assignment
as the place to validate the parsed port.
- Around line 304-343: The provider selection in the residential proxy resolver
currently falls back to Oxylabs for any unrecognized value, which can mask typos
in --provider. Update the switch in the provider resolution function to
normalize the provider string first, add an explicit oxylabs case alongside
brightdata/brd and packetstream/ps, and change the default branch to return an
error for unknown providers instead of returning Oxylabs settings. Keep the
existing environment-variable validation and host/password setup in the
provider-specific branches.
- Around line 259-264: Treat non-2xx responses as failures in the throughput
path. In the request flow around client.Get and the subsequent resp.Body
handling, add a status-code check on resp.StatusCode before measuring with
io.Copy so proxy block pages, 403/429s, and other error responses return an
error instead of being counted as success. Keep the change localized to the
fetch/measure logic in the throughput function that uses client.Get, io.Copy,
and io.LimitReader.
- Around line 120-121: The detour assignment in main uses `len(tags)%*poolN`, so
`--pool` must be validated before this modulo is reached. In
cmd/residential-urltest/main.go, after flag.Parse() and before the loop that
builds ob (when !*direct), reject any non-positive pool value with a clear error
so the program exits before the modulo and before creating unreachable
residential tags.
---
Nitpick comments:
In `@cmd/residential-urltest/main.go`:
- Around line 1-24: The package-level comment in main.go and the nearby
unexported filter comments need to follow Go doc conventions. Update the top
comment to start with “Package residential-urltest …” and make each affected
comment begin with the identifier name it documents, using the symbols in this
file rather than generic prose. Keep the wording otherwise the same, but ensure
the comment style matches the project’s documented naming convention.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a3dbfe07-906b-4d07-b233-c67b9567f299
📒 Files selected for processing (1)
cmd/residential-urltest/main.go
Comment on lines
+120
to
+121
| if !*direct { | ||
| ob["detour"] = fmt.Sprintf("residential-%d", len(tags)%*poolN) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Reject non-positive --pool before using it in modulo.
With --pool=0, Line 121 panics with integer divide by zero; with a negative pool, detours can be assigned to residential tags that are never created. Validate after flag.Parse() when --direct is false.
Proposed validation
flag.Parse()
+ if !*direct && *poolN <= 0 {
+ fatal("--pool must be greater than 0 when using a residential detour")
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if !*direct { | |
| ob["detour"] = fmt.Sprintf("residential-%d", len(tags)%*poolN) | |
| if !*direct && *poolN <= 0 { | |
| fatal("--pool must be greater than 0 when using a residential detour") | |
| } |
🤖 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 `@cmd/residential-urltest/main.go` around lines 120 - 121, The detour
assignment in main uses `len(tags)%*poolN`, so `--pool` must be validated before
this modulo is reached. In cmd/residential-urltest/main.go, after flag.Parse()
and before the loop that builds ob (when !*direct), reject any non-positive pool
value with a clear error so the program exits before the modulo and before
creating unreachable residential tags.
Comment on lines
+167
to
+171
| ctxTimeout := *timeoutS | ||
| if *throughput && ctxTimeout < 150 { // downloads need a longer window than a url-test probe | ||
| ctxTimeout = 150 | ||
| } | ||
| ctx, cancel := context.WithTimeout(ctx, time.Duration(ctxTimeout)*time.Second) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t use one 150s instance deadline for all throughput downloads.
When --throughput is enabled, the sing-box context can expire while iterating reachable outbounds sequentially; after that, remaining measurements can report STALLED because the harness timed out, not because the route is throttled. Keep the instance context alive until cleanup, and apply separate per-operation timeouts to URLTest and each throughput request.
Also applies to: 228-237
🤖 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 `@cmd/residential-urltest/main.go` around lines 167 - 171, The throughput path
is using the single sing-box context deadline from the residential URL test
harness, which can expire mid-loop and falsely mark later outbounds as STALLED.
Update the setup in main and the sequential measurement flow so the instance
context stays alive until cleanup, then apply separate per-operation timeouts
inside URLTest and each throughput request instead of relying on the shared 150s
deadline. Use the URLTest and throughput request handling code to locate the
timeout logic and keep the outer context from governing all downloads.
Comment on lines
+259
to
+264
| resp, err := client.Get(url) | ||
| if err != nil { | ||
| return 0, 0, time.Since(start), err | ||
| } | ||
| defer resp.Body.Close() | ||
| got, err = io.Copy(io.Discard, io.LimitReader(resp.Body, int64(maxBytes))) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Treat non-2xx throughput responses as failures.
io.Copy currently measures any response body, so a proxy block page, 403/429, or other error response can be reported as successful throughput.
Proposed status check
resp, err := client.Get(url)
if err != nil {
return 0, 0, time.Since(start), err
}
defer resp.Body.Close()
+ if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
+ return 0, 0, time.Since(start), fmt.Errorf("throughput target returned %s", resp.Status)
+ }
got, err = io.Copy(io.Discard, io.LimitReader(resp.Body, int64(maxBytes)))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| resp, err := client.Get(url) | |
| if err != nil { | |
| return 0, 0, time.Since(start), err | |
| } | |
| defer resp.Body.Close() | |
| got, err = io.Copy(io.Discard, io.LimitReader(resp.Body, int64(maxBytes))) | |
| resp, err := client.Get(url) | |
| if err != nil { | |
| return 0, 0, time.Since(start), err | |
| } | |
| defer resp.Body.Close() | |
| if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { | |
| return 0, 0, time.Since(start), fmt.Errorf("throughput target returned %s", resp.Status) | |
| } | |
| got, err = io.Copy(io.Discard, io.LimitReader(resp.Body, int64(maxBytes))) |
🤖 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 `@cmd/residential-urltest/main.go` around lines 259 - 264, Treat non-2xx
responses as failures in the throughput path. In the request flow around
client.Get and the subsequent resp.Body handling, add a status-code check on
resp.StatusCode before measuring with io.Copy so proxy block pages, 403/429s,
and other error responses return an error instead of being counted as success.
Keep the change localized to the fetch/measure logic in the throughput function
that uses client.Get, io.Copy, and io.LimitReader.
Comment on lines
+297
to
+303
| h, ps, err := net.SplitHostPort(gwOverride) | ||
| if err != nil { | ||
| return "", 0, nil, "", fmt.Errorf("bad --gw: %w", err) | ||
| } | ||
| host = h | ||
| port, _ = strconv.Atoi(ps) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate parsed gateway ports instead of silently using 0.
strconv.Atoi errors are ignored for both --gw and BRD_PORT; invalid input becomes port 0, causing misleading connection failures instead of a clear configuration error.
Proposed port validation helper
+func parsePort(name, value string) (int, error) {
+ port, err := strconv.Atoi(value)
+ if err != nil || port <= 0 || port > 65535 {
+ return 0, fmt.Errorf("%s must be a valid TCP port, got %q", name, value)
+ }
+ return port, nil
+}
+
func providerGateway(provider, gwOverride, country string) (string, int, func(string) string, string, error) {- port, _ = strconv.Atoi(ps)
+ port, err = parsePort("--gw port", ps)
+ if err != nil {
+ return "", 0, nil, "", err
+ }- port, _ = strconv.Atoi(p)
+ port, err = parsePort("BRD_PORT", p)
+ if err != nil {
+ return "", 0, nil, "", err
+ }Also applies to: 312-314
🤖 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 `@cmd/residential-urltest/main.go` around lines 297 - 303, The gateway port
parsing in the `main`/config setup ignores `strconv.Atoi` failures for both
`gwOverride` and `BRD_PORT`, so invalid values fall through as port 0. Update
the parsing logic around `net.SplitHostPort`, `strconv.Atoi`, and the `BRD_PORT`
handling to check conversion errors and return a clear configuration error
instead of silently continuing. Use the existing gateway parsing path and
`host`/`port` assignment as the place to validate the parsed port.
Comment on lines
+304
to
+343
| switch provider { | ||
| case "brightdata", "brd": | ||
| cust, zone, pw := os.Getenv("BRD_CUSTOMER_ID"), os.Getenv("BRD_ZONE"), os.Getenv("BRD_PASSWORD") | ||
| if cust == "" || zone == "" || pw == "" { | ||
| return "", 0, nil, "", fmt.Errorf("brightdata needs BRD_CUSTOMER_ID/BRD_ZONE/BRD_PASSWORD env") | ||
| } | ||
| if host == "" { | ||
| host, port = "brd.superproxy.io", 33335 | ||
| if p := os.Getenv("BRD_PORT"); p != "" { | ||
| port, _ = strconv.Atoi(p) | ||
| } | ||
| } | ||
| mk := func(sess string) string { | ||
| return fmt.Sprintf("brd-customer-%s-zone-%s-country-%s-session-%s", cust, zone, cc, sess) | ||
| } | ||
| return host, port, mk, pw, nil | ||
| case "packetstream", "ps": | ||
| user, key := os.Getenv("PS_USER"), os.Getenv("PS_AUTH_KEY") | ||
| if user == "" || key == "" { | ||
| return "", 0, nil, "", fmt.Errorf("packetstream needs PS_USER/PS_AUTH_KEY env") | ||
| } | ||
| if host == "" { | ||
| host, port = "proxy.packetstream.io", 31112 | ||
| } | ||
| pw := fmt.Sprintf("%s_country-%s", key, countryName(cc)) | ||
| mk := func(sess string) string { return user } // PacketStream rotates IP per connection; no session field | ||
| return host, port, mk, pw, nil | ||
| default: // oxylabs | ||
| user, pass := os.Getenv("OXY_USER"), os.Getenv("OXY_PASS") | ||
| if user == "" || pass == "" { | ||
| return "", 0, nil, "", fmt.Errorf("oxylabs needs OXY_USER/OXY_PASS env") | ||
| } | ||
| if host == "" { | ||
| host, port = "pr.oxylabs.io", 7777 | ||
| } | ||
| mk := func(sess string) string { | ||
| return fmt.Sprintf("customer-%s-cc-%s-sessid-%s-sesstime-10", user, cc, sess) | ||
| } | ||
| return host, port, mk, pass, nil | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject unknown providers instead of falling back to Oxylabs.
A typo in --provider currently routes through Oxylabs, which can make the test results describe the wrong residential network. Normalize the provider name, add an explicit Oxylabs case, and return an error in default.
🤖 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 `@cmd/residential-urltest/main.go` around lines 304 - 343, The provider
selection in the residential proxy resolver currently falls back to Oxylabs for
any unrecognized value, which can mask typos in --provider. Update the switch in
the provider resolution function to normalize the provider string first, add an
explicit oxylabs case alongside brightdata/brd and packetstream/ps, and change
the default branch to return an error for unknown providers instead of returning
Oxylabs settings. Keep the existing environment-variable validation and
host/password setup in the provider-specific branches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a standalone debugging harness (untracked until now) that reproduces "connected but no traffic" reports by exercising a v9 client's sing-box outbounds from a censored country's residential IP.
What it does
MutableURLTest— so custom protocols (samizdat, shadowsocks, vless+REALITY, anytls, reflex) are built the same way the client builds them.--directgives a no-detour control;--debugsurfaces per-outbound sing-box failures.--throughput(the important addition): downloads several MB through each reachable outbound and reports achieved KB/s. A url-test only proves handshake + first byte, which a throttling censor (RU TSPU) sails past — sustained throughput is what catches it. (e.g. RU prod SS tracks: ~2.5 MB/s direct, ~0.1 KB/s from RU residential — handshake OK, data plane throttled.)Build / use
Notes
mainundercmd/— doesn't touch the client build; compiles cleanly with and withoutwith_utls(verified), so it won't affect CI's default build.region-protocol-testandresidential-proxyskills, which referenced this tool while it lived only on local disk.🤖 Generated with Claude Code
https://claude.ai/code/session_01H9beSsYGzUaBhRK5ULmtGr
Summary by CodeRabbit