Skip to content

feat: Add outbound egress metadata (IP + country)#5886

Merged
MHSanaei merged 3 commits into
MHSanaei:mainfrom
isultanov99:outbound-egress-trace
Jul 12, 2026
Merged

feat: Add outbound egress metadata (IP + country)#5886
MHSanaei merged 3 commits into
MHSanaei:mainfrom
isultanov99:outbound-egress-trace

Conversation

@isultanov99

Copy link
Copy Markdown
Contributor

Show egress IP and country information for outbound HTTP tests. The probe reuses the temporary SOCKS route from the existing HTTP test and fetches Cloudflare trace metadata after the reachability check succeeds.

The outbound list now adds separate Egress and Country columns, hides egress IPs until the user reveals them, and marks Cloudflare WARP results with an orange cloud pill. Mobile cards keep the same data compact by placing the country and IPv4/IPv6 values on separate lines.

Validation: npm run typecheck; npm run lint; npm run build; go test ./internal/web/service/outbound

Show egress IP and country information for outbound HTTP tests. The probe reuses the temporary SOCKS route from the existing HTTP test and fetches Cloudflare trace metadata after the reachability check succeeds.

The outbound list now adds separate Egress and Country columns, hides egress IPs until the user reveals them, and marks Cloudflare WARP results with an orange cloud pill. Mobile cards keep the same data compact by placing the country and IPv4/IPv6 values on separate lines.

Validation: npm run typecheck; npm run lint; npm run build; go test ./internal/web/service/outbound
@isultanov99

Copy link
Copy Markdown
Contributor Author
Без имени

@isultanov99 isultanov99 changed the title Add outbound egress metadata feat: Add outbound egress metadata (IP + country) Jul 9, 2026
@github-actions github-actions Bot added New Feature go Pull requests that update Go code javascript Pull requests that update javascript code labels Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Reviewed the diff and the base-repo source touched by this PR (internal/web/service/outbound/, the outbound-tab frontend, and translations). The egress/country display itself is a reasonable, well-scoped addition (best-effort trace fetch that never affects reachability, graceful nil-handling, a real Go test for the parser), but there is one concrete test-hermeticity regression, an undisclosed removal of the "Real Delay" test mode, and a state-scoping bug in the mobile card view that undermines the "hidden until revealed" privacy intent. Details below, most severe first.

1. High / High confidence / Testing-Reliability
Location: internal/web/service/outbound/probe_http_test.go:402 (TestTestOutboundsHTTPBatchThroughStubSocks) and :519 (TestTestOutboundsTCPModeForcesUDPToHTTPProbe)
Problem: this PR doesn't touch probe_http_test.go, but both of these pre-existing tests run a stub SOCKS proxy that really dials whatever destination it's asked for (serveStubSocks, line 115: net.Dial("tcp", ...)), with realDelay=false (mode "http"/"tcp"+UDP-transport). Since their local reachability probe succeeds, the new if !realDelay { result.Egress = probeEgressTrace(proxyURL, timeout) } (probe_http.go:548-550) now fires inside them too, triggering a real net.LookupIP("cloudflare.com") (probe_http.go:592) and a real HTTPS request to Cloudflare's production IP over the internet, using the same client := &http.Client{..., Timeout: timeout} (probe_http.go:574) as production (httpProbeTimeout = 10s), for the IPv4 and then the IPv6 target.
Why it matters: these were previously fully hermetic, offline-capable unit tests. They now depend on live internet access and Cloudflare's availability. In a network-restricted CI runner, a firewalled contributor sandbox, or during a Cloudflare hiccup/rate-limit, each affected test can hang for up to ~20 seconds (two sequential attempts at the full timeout) or fail, for reasons unrelated to the code under test.
Recommendation: inject the egress-trace dependency behind a seam tests can stub (similar to the existing newBatchProcess var), or otherwise make it possible to disable/mock during go test.

2. High / High confidence / Correctness (undisclosed regression)
Location: frontend/src/pages/xray/outbounds/OutboundsTab.tsx:114 (default testMode changed 'tcp''http') and :493-495 (the Radio.Button value="real" that lived at base-file line 496 is deleted, leaving only TCP/HTTP)
Problem: the PR silently removes the only "Real Delay" control in the app and changes the default test mode, but OutboundTestMode (frontend/src/hooks/useXraySetting.ts:32) still includes 'real', the backend still fully implements it (probeModeLabel, realDelay := mode == "real" in probe_http.go), testModeLabel()/modeRealDelay (outbounds-tab-helpers.ts:56) are still defined, and both the API docs (frontend/src/pages/api-docs/endpoints.ts:1318,1329) and the testModeTooltip translation string (internal/web/translation/en-US.json:1761) still describe three modes including "Real delay: total time including connection setup." None of this is mentioned in the PR description, which only discusses egress metadata.
Why it matters: SubscriptionOutbounds.tsx only receives the shared testMode prop and has no selector of its own, so this is the only entry point — after this PR, no user can reach "Real Delay" mode from the UI anymore, while the rest of the codebase still half-supports it, including a now-stale tooltip. If this removal is intentional it needs to be disclosed and cleaned up consistently; if not, it's a silent feature regression bundled into an unrelated PR.
Recommendation: restore the button, or if it's being retired, remove it consistently (type union, backend handling or explicit deprecation, tooltip copy, endpoints.ts, i18n key) and call it out explicitly in the PR description.

3. Medium / High confidence / Correctness-UX
Location: frontend/src/pages/xray/outbounds/OutboundCardList.tsx:59 (const [showEgressIp, setShowEgressIp] = useState(false)) and :153 ({renderEgress(index)} inside rows.map(...))
Problem: showEgressIp is a single boolean owned by the whole OutboundCardList component, but renderEgress(index) — including its own eye-icon toggle — is invoked once per card. Every card's icon reads and writes the same shared state, so revealing (or hiding) the egress IP on any one card reveals/hides it on every visible card simultaneously.
Why it matters: the UI shows what looks like a per-card control (an eye icon next to each card's own IP), so a user tapping "reveal" on one server, expecting to expose just that one, will inadvertently expose every outbound's egress IP. This has no precedent elsewhere in the codebase: the equivalent existing pattern (frontend/src/pages/nodes/NodeList.tsx) only ever places this toggle once — in a table column header or in a single-record modal — never duplicated across a list while sharing one boolean.
Recommendation: key visibility per row (e.g. Record<number, boolean>), or if a single global toggle is genuinely intended, render it once above the card list instead of duplicating the affordance in every card.

4. Medium / High confidence / Performance
Location: internal/web/service/outbound/probe_http.go:578-583 (sequential if ipv4 != nil {...} / if ipv6 != nil {...} calls) and :574 (Timeout: timeout)
Problem: probeEgressTrace fetches the IPv4 and IPv6 Cloudflare trace endpoints sequentially, each through an http.Client whose Timeout is the full timeout argument (the same 10s httpProbeTimeout budget already used for the cold and warm requests), with no shorter, dedicated timeout and no concurrency between the two.
Why it matters: this runs inside the goroutine that already holds one of the 16 httpProbeConcurrency slots, and the batch's wg.Wait() blocks on every item. For a slow or filtered outbound, one item can now take up to ~40s (10s cold + 10s warm + 10s IPv4 trace + 10s IPv6 trace) instead of ~20s, and "Test All" waits for the slowest item before returning any results.
Recommendation: give the egress trace its own shorter timeout (a few seconds is enough for a diagnostic call) and/or fetch IPv4/IPv6 concurrently.

5. Low / High confidence / Accessibility
Location: frontend/src/pages/xray/outbounds/OutboundCardList.tsx:81,83 and frontend/src/pages/xray/outbounds/useOutboundColumns.tsx:150,152
Problem: the new EyeOutlined/EyeInvisibleOutlined toggles use role="button" tabIndex={0} with only onClick, no onKeyDown.
Why it matters: a keyboard user who tabs to these controls cannot activate them with Enter/Space. The identical ip-toggle-icon pattern elsewhere in this codebase (frontend/src/pages/nodes/NodeList.tsx:300,302,576,578, frontend/src/pages/index/IndexPage.tsx:409,418) always pairs onClick with onKeyDown={activateOnKey(...)} from @/utils/a11y; the new code diverges from that established convention.
Recommendation: add onKeyDown={activateOnKey(...)} to each of the four new toggle icons, matching the existing pattern.

6. Low / Medium confidence / Correctness
Location: frontend/src/pages/xray/outbounds/CountryPill.tsx:10
Problem: isWarp is computed as warp === 'on'. Cloudflare's /cdn-cgi/trace warp= field is documented to also report plus for WARP+ subscriptions, which this check would not match.
Why it matters: if that value appears in practice, WARP+ connections would silently miss the orange "WARP" pill this PR is meant to surface.
Recommendation: treat any non-empty, non-off value as WARP-like (e.g. warp !== '' && warp !== 'off'), or check for both 'on' and 'plus' explicitly.

Positive observations: parseCloudflareTrace/applyEgressTrace are nil-safe and best-effort, never affecting the underlying reachability result; the new Go test (egress_trace_test.go) follows the repo's stdlib-only, exact-value-assertion convention; and all 13 locale files received the new egress/egressHint keys.

Verdict: Request changes — @MHSanaei please take a look. Findings 1 and 2 (a live-network dependency newly introduced into existing unit tests, and an undisclosed removal of the "Real Delay" mode with inconsistent leftovers) should be resolved or at least explained before merge; findings 3-6 are smaller UX/performance/accessibility issues worth addressing alongside.

This review was generated automatically. A maintainer may follow up.

Restore the Real Delay selector and TCP default so the egress metadata change does not remove an existing test mode.

Keep HTTP probe tests hermetic by stubbing egress trace lookups, run IPv4 and IPv6 trace fetches concurrently with a shorter diagnostic timeout, scope mobile IP reveal state per row, support keyboard activation for reveal toggles, and treat WARP+ trace values as WARP-like.
@isultanov99

Copy link
Copy Markdown
Contributor Author

Good points, thanks. Fixed in the latest push:

  • Restored the Real Delay selector and the original TCP default, so this PR no longer removes an existing test mode.
  • Kept the existing HTTP probe unit tests hermetic by injecting the egress trace probe in tests instead of allowing real Cloudflare/DNS calls.
  • Added a shorter dedicated timeout for egress trace metadata and fetch IPv4/IPv6 trace results concurrently.
  • Scoped the mobile egress IP reveal state per outbound row instead of sharing one toggle across all cards.
  • Added keyboard activation for the new eye toggles.
  • Treat non-off WARP trace values, including plus, as WARP-like for the country pill.

Validation run locally:

  • go test ./internal/web/service/outbound
  • npm run typecheck
  • npm run lint
  • npm run build

@MHSanaei MHSanaei merged commit 30f6bc1 into MHSanaei:main Jul 12, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go Pull requests that update Go code javascript Pull requests that update javascript code New Feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants