refactor(cli): replace ancient tunnel with dedicated proxy agents#1157
Open
sorccu wants to merge 3 commits into
Open
refactor(cli): replace ancient tunnel with dedicated proxy agents#1157sorccu wants to merge 3 commits into
tunnel with dedicated proxy agents#1157sorccu wants to merge 3 commits into
Conversation
sorccu
force-pushed
the
simo/sim-138-cli-replace-7-year-told-tunnel-package-with-proxy-agent
branch
from
65fa143 to
70d8793
Compare
sorccu
force-pushed
the
simo/sim-138-cli-replace-7-year-told-tunnel-package-with-proxy-agent
branch
from
70d8793 to
16fed95
Compare
sorccu
force-pushed
the
simo/sim-138-cli-replace-7-year-told-tunnel-package-with-proxy-agent
branch
from
16fed95 to
7c79611
Compare
sorccu
changed the title
tunnel + proxy-from-env with proxy-agenttunnel with proxy-agent
sorccu
force-pushed
the
simo/sim-138-cli-replace-7-year-told-tunnel-package-with-proxy-agent
branch
from
7c79611 to
834c22b
Compare
…M-138] Drop the ~7-year-old `tunnel` package in favour of the maintained `http-proxy-agent`, `https-proxy-agent`, and `socks-proxy-agent`, selecting the right agent per target URL via `getProxyForUrl` (which honors HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY). Proxy handling is centralized in services/proxy.ts: `createProxyAgent` returns the agent for a target URL (or undefined for a direct connection), and `assignProxy` installs it on an axios config and sets `proxy: false`. Setting `proxy: false` is mandatory — otherwise axios re-reads the proxy env vars itself and appends the proxy's port to portless target URLs (e.g. https://api.checklyhq.com -> :8080). All proxy paths now go through the helper, including two axios.get calls that previously ignored proxy settings entirely (the npm version check in baseCommand and the V2 asset download in rest/assets). The WebSocket client uses createProxyAgent directly via wsOptions. These focused agents add ~180 KB versus the ~5 MB `proxy-agent` meta-package (whose 3 MB QuickJS WASM only powers PAC evaluation, which cannot honor system/WPAD proxy configuration anyway). SOCKS proxies remain supported. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ks4mnGCV1MAK69MMzojmcx
…t [SIM-138] Add proxy.spec.ts covering createProxyAgent selection (http/https/socks/wss, NO_PROXY, and direct when unset), that assignProxy always sets `proxy: false` and shares one agent across both slots, and a behavioral test that a portless target host is forwarded through a local proxy without the proxy port appended. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ks4mnGCV1MAK69MMzojmcx
sorccu
force-pushed
the
simo/sim-138-cli-replace-7-year-told-tunnel-package-with-proxy-agent
branch
from
834c22b to
188707c
Compare
sorccu
changed the title
tunnel with proxy-agenttunnel with dedicated proxy agents
…ble [SIM-138] When a proxy is set but down, the connection (made to the proxy first) fails with ECONNREFUSED and the user previously saw the generic "check that the internet connection is working" message, which points at the wrong thing. handleErrorResponse now throws a ProxyConnectionError that names the proxy (credentials stripped) when a proxy applies to the request and the failure is a connection-level error (ECONNREFUSED/ETIMEDOUT/EHOSTUNREACH/ENETUNREACH/ ENOTFOUND/EAI_AGAIN), found by walking the error's cause chain and any AggregateError. Requests without a proxy keep the generic message. A getProxyUrl helper is extracted in services/proxy.ts to resolve the proxy for a target URL using the same env-var rules as agent selection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ks4mnGCV1MAK69MMzojmcx
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
1 participant
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.
Replaces the ~7-year-old
tunnelpackage with the maintainedhttp-proxy-agent,https-proxy-agent, andsocks-proxy-agent, selecting the right agent per target URL viaproxy-from-env'sgetProxyForUrl(which honorsHTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY).tunnel's behavior on current Node is uncertain and it complicated MITM/proxy debugging.Resolves SIM-138. This supersedes the original attempt on this branch (rebased onto the pnpm monorepo).
What changed
tunnel+@types/tunnel; addhttp-proxy-agent,https-proxy-agent,socks-proxy-agent.services/proxy.ts:createProxyAgent(targetUrl)— returns the agent for the target (HTTPS/HTTP/SOCKS) orundefinedfor a direct connection.ws:/wss:are mapped to their HTTP equivalents for proxy selection.assignProxy(targetUrl, config)— installs the agent and setsproxy: false.rest/api.ts,auth/index.ts,helpers/result-assets.ts,services/socket-client.ts(viawsOptions.agent), plus twoaxios.getcalls that previously ignored proxy settings entirely (commands/baseCommand.tsnpm version check,rest/assets.tsV2 asset download).services/proxy.spec.tscovering agent selection (http/https/socks/wss/NO_PROXY/direct) and a behavioral test that a portless target host is forwarded through a local proxy without the proxy port appended.Why the original was blocked — and the fix
The first version of this change dropped the old helper but didn't set
proxy: false, so axios re-read the proxy env vars itself and appended the proxy's port to portless targets (https://api.checklyhq.com→:8080). Centralizingproxy: falseinassignProxymakes the invariant impossible to forget at a call site.Why dedicated agents instead of
proxy-agentAn interim version used the
proxy-agentmeta-package. It pulls ~5 MB (≈3 MB of which is a QuickJS WASM blob) solely to evaluate PAC files — and that PAC support cannot honor system/WPAD proxy configuration anyway (it only triggers on a non-standardpac+...env var). The three dedicated agents cover every proxy setup the CLI actually uses, including SOCKS, for ~180 KB.Better proxy errors
When a proxy is configured but unreachable, the connection (made to the proxy first) failed with
ECONNREFUSEDand the CLI showed a generic "check that the internet connection is working" message. It now throws aProxyConnectionErrornaming the proxy (credentials stripped), e.g. "Could not connect to the proxy at http://localhost:1234. Check that the proxy is running and that the http_proxy/https_proxy/all_proxy environment variables point to the correct address." Requests made without a proxy keep the generic message.Affected Components
Notes for the Reviewer
proxy: falseinassignProxyis load-bearing — see the regression test for the exact behavior it guards.