Repository navigation
test(http-agent): cover HTTPS upstream proxies with real server - #742
Merged
Merged
Conversation
Closes #620 Replace the smoke test that pointed at a non-existent HTTPS upstream with tests against a local proxy-chain server running with serverType 'https': - forward mode pools connections to the HTTPS upstream with a keepAlive agent - keepAlive: false opens a connection per request, proving the agent from prepareRequestFunction is used instead of https.globalAgent - chain mode tunnels HTTPS targets through the HTTPS upstream; each CONNECT gets its own upstream connection because Node.js detaches tunnel sockets from the agent Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UdrrEAC7cYnarj1ZUc9hEf
A connection that has carried a CONNECT tunnel is never returned to the agent's pool, so HTTPS targets get a connection per tunnel even with keepAlive. Sticky IPs through connection reuse apply to plain HTTP only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UdrrEAC7cYnarj1ZUc9hEf
apify-service-account
added
the
tested
apify-service-account
added
the
t-unblocking
…m protocol The note said "HTTPS request", which reads as if HTTPS upstream proxies do not pool. They do for http:// targets. Only https:// targets, sent as CONNECT tunnels, get a connection per tunnel. Also drop the claim that sticky IPs apply to plain HTTP only: requests inside one tunnel share its connection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UdrrEAC7cYnarj1ZUc9hEf
VojtaM39
approved these changes
Nov 7, 2026
bliuchak
added
the
validated
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.
Closes #620
What
Replaces the HTTPS upstream smoke test in
test/e2e/http-agent.ts, which pointed at a non-existent host, with three tests against a real local HTTPS upstream (proxy-chain itself withserverType: 'https'):keepAlivehttpsAgentreach the upstream over one TLS connection.prepareRequestFunctionis the one used. WithkeepAlive: falsethe same requests open three connections, so a regression that falls back tohttps.globalAgentfails.CONNECT, and every tunnel gets its own upstream connection.Also adds a README note: pooling depends on the target URL, not on the upstream proxy's protocol. Requests for
http://URLs are pooled through both HTTP and HTTPS upstreams. Requests forhttps://URLs go throughCONNECTtunnels, and a connection that has carried a tunnel is never returned to the agent's pool, so each tunnel gets its own upstream connection. Requests inside one tunnel share its connection.Why
#619 added agent pooling but left HTTPS upstreams untested in both forward and chain mode (see the review in #619 (comment)). The old test passed on any failure, so it proved only that the code path does not crash.
How
connectionIds in itsprepareRequestFunction, andvi.spyOn(httpsAgent, 'createConnection')counts the connections the agent opens.ignoreUpstreamProxyCertificate: true. With verification on, chain mode through an HTTPS upstream currently fails for a separate reason, tracked in CONNECT through an HTTPS upstream proxy verifies the proxy certificate against the target hostname #743.Testing
test/Dockerfile, Node 20):http-agent.ts8/8 pass, and 5/5 repeated runs of the file pass. The only failures are the 3 SOCKS tests that also fail onmaster(they depend onexample.com, see Replaceexample.comwithapi.apify.com/v2/browser-infoin the SOCKS e2e tests #739 / test(socks): use api.apify.com/v2/browser-info instead of example.com #740).pnpm run type-checkandeslintpass.src/does not change meaningfully. The lines these tests exercise were already covered; what was missing were assertions that pooling over an HTTPS upstream actually works.http(s).requestCONNECTcalls, shaped like the one insrc/chain.ts, on Node 20, 22, 24 and 26, with HTTP and HTTPS upstreams and200and407responses. A tunnel connection was never reused afterwards. With an HTTP upstream, aCONNECTcan take an idle pooled connection, which is then dedicated to that tunnel. "Each tunnel gets its own upstream connection" covers both cases.Risks
Tests and docs only, no change in
src/. The chain-mode test partly asserts Node.js behaviour (tunnel sockets leave the agent's pool), which matches what the README now documents.🤖 Generated with Claude Code