fix(socks): downgrade socks-proxy-agent to fix two failing tests on main #32988
Conversation
| @@ -97,8 +98,8 @@ class SocksProxyConnection { | |||
| } | |||
|
|
|||
| async connect() { | |||
| if (this.socksProxy.proxyAgentFromOptions) | |||
| this.target = await this.socksProxy.proxyAgentFromOptions.connect(new EventEmitter() as any, { host: rewriteToLocalhostIfNeeded(this.host), port: this.port, secureEndpoint: false }); | |||
| if (this.socksProxy.proxyAgentFromOptions instanceof SocksProxyAgent) | |||
There was a problem hiding this comment.
Looks wrong, it can be also a HttpsProxyAgent.
There was a problem hiding this comment.
In a file called socksClientCertificatesInterceptor.ts?
There was a problem hiding this comment.
Background: Only the communication between the browser and Playwright is made via a Socks Proxy, hence its called like that. Internally when we make requests to the target (e.g. internal.corp.com it can be either direct or via an agent. An agent is a proxy agent which then can be https or socks.). (We don't intercept http traffic.)
| @@ -24,7 +24,7 @@ | |||
| "proxy-from-env": "1.1.0", | |||
| "retry": "0.12.0", | |||
| "signal-exit": "3.0.7", | |||
| "socks-proxy-agent": "8.0.4", | |||
| "socks-proxy-agent": "6.1.1", | |||
There was a problem hiding this comment.
https-proxy-agent and socks-proxy-agent depend both on agent-base. So ideally we keep them in sync.
There was a problem hiding this comment.
We need the up-to-date version for the fix from de4a4d1 though. If we want to keep them in sync, we need to revert it until I figure out how we can update socks-proxy-agent.
|
Alright, reverting in #32989. |
Test results for "tests others"2 failed 20524 passed, 491 skipped Merge workflow run. |
Test results for "tests 1"6 failed 2 flaky35814 passed, 619 skipped Merge workflow run. |
Test results for "tests 2"70 failed 43 flaky234821 passed, 9301 skipped Merge workflow run. |
"should pass with matching certificates when a socks proxy is used"was broken by de4a4d1. This PR fixes it by downgradingsocks-proxy-agentto the version it was at before - I added that upgrade to the PR to keep it up-to-date, but we only needed to really update https-proxy-agent.