Skip to content

fix(net): apply socketTimeout from socket creation in httpRequest - #42660

Merged
Pavel Feldman (pavelfeldman) merged 1 commit into
microsoft:mainfrom
ashrafiucse:fix-42578
Sep 10, 2026
Merged

Pavel Feldman (pavelfeldman) merged 1 commit into
microsoft:mainfrom
ashrafiucse:fix-42578

Conversation

@ashrafiucse

@ashrafiucse Ashraf Ali (ashrafiucse) commented Sep 10, 2026

Copy link
Copy Markdown
Contributor
  • request.setTimeout() only takes effect after the socket connects, so on a stalled TCP connect the default agent's built-in 5s timeout killed the request while the error still reported the configured value — e.g. debug logs showed timed out after 120000ms firing at +5s, and PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT had no effect (see [Bug]: Browser Install Timeout regardless of env timeout. 1.63.0 regression #42597)
  • Pass socketTimeout via request options so it governs the socket from creation; regression-tested with a stalled-connect case (fails at ~5s before the fix) and a stalled-response case

Fixes #42578
Fixes #42597

request.setTimeout() only takes effect after the socket connects, so on a
stalled TCP connect the default agent's 5s timeout killed the request
while the error still reported the configured timeout. This made
PLAYWRIGHT_DOWNLOAD_CONNECTION_TIMEOUT ineffective for browser downloads
on networks where connecting to the CDN takes longer than 5s.

Fixes: microsoft#42578
@pavelfeldman

Copy link
Copy Markdown
Member

Do you have an idea why this fixes the "1.63 regression"?

@ashrafiucse

Copy link
Copy Markdown
Contributor Author

Yes — it took a while to pin down, because nothing in the diff touches anything version-specific. The mechanism is buried in Node.

Node's default agents (http.globalAgent / https.globalAgent) are created with timeout: 5000. When a request goes through one, Agent.createSocket() copies that into the socket options (req.timeout || this.options.timeout), so the socket gets a 5-second idle timeout from the moment it's created — before it has even connected. request.setTimeout(N) can't override that in time, because Node only applies it once the socket emits 'connect'.

Before 1.63 this never mattered, because httpRequest() used its own happy-eyeballs agent, which carries no timeout — Playwright's request.setTimeout() was the only timer in play. #42240 switched to native autoSelectFamily and dropped the custom agent, so the no-proxy path silently fell back to globalAgent and its 5s timer.

On a network where the TCP connect to the CDN takes more than 5s — say the first address in the race is blackholed, which is what the #42597 logs show — the agent timer fires mid-connect, and Playwright's callback reports whatever value it was configured with. Hence that debug log: timed out after 120000ms firing at +5s, five attempts in a row. Raising the env var changed the number in the message but not when the request actually died.

I reproduced it hermetically: lookup returns a blackholed address first and a working one second, server accepts but never responds. Without the fix the request dies at ~5s while claiming 60s; with it, it survives to the configured timeout. That's what the new test pins down.

One-line version: setting options.timeout makes req.timeout truthy, so Agent.createSocket() forwards Playwright's value to the socket instead of the agent's 5000 default, and the intended timeout governs from creation.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

3 failed
❌ [msedge] › mcp/annotate.spec.ts:269 › should start dashboard and annotate when no dashboard is running @mcp-windows-latest-msedge
❌ [msedge] › mcp/annotate.spec.ts:291 › should enter annotate mode on fresh dashboard.tsx mount with -s --annotate @mcp-windows-latest-msedge
❌ [msedge] › mcp/http.spec.ts:145 › http transport browser sigint @mcp-windows-latest-msedge

8345 passed, 1376 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

7 flaky ⚠️ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport `@chromium-ubuntu-22.04-arm-node20`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:257 › third party 'Partitioned;' cookies `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:470 › top level 'Partitioned;' cookie and same origin iframe `@firefox-ubuntu-22.04-node20`
⚠️ [webkit-library] › library/browsercontext-clearcookies.spec.ts:92 › should remove cookies by domain `@webkit-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:294 › should not fail on internal page logs `@windows-latest-node22`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:827 › should update state on subsequent run `@windows-latest-node22`

51557 passed, 1247 skipped


Merge workflow run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Browser Install Timeout regardless of env timeout. 1.63.0 regression [Bug]: Unable to install playwright bundled browsers via terminal

2 participants