Skip to content

fix(fetch): do not crash on late EPIPE after refused body - #42116

Merged
Simon Knott (Skn0tt) merged 6 commits into
microsoft:mainfrom
Skn0tt:skn0tt-fix-42074
Aug 5, 2026
Merged

fix(fetch): do not crash on late EPIPE after refused body#42116
Simon Knott (Skn0tt) merged 6 commits into
microsoft:mainfrom
Skn0tt:skn0tt-fix-42074

Conversation

@Skn0tt

@Skn0ttSimon Knott (Skn0tt) commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Fixes#42074. node:http has a footgun where socket.on('error') is bubbled up into request.on('error') while the request is ongoing. If the server decides not to read the request body in full, e.g. to prevent DoS, there is a race where it first responds 413, fulfilling the response and unlinking socket.on('error') from request.on('error'), and only after that the client receives an error while trying to finish writing its request body. This is an unhandled error event and crashes the host process.

The fix is to listen for the error event and swallow it if the race was detected. Axios fixed this race in axios/axios#10576, i'm looking into what got does now.

When a server answers without reading the request body and then resets
the socket, Node can emit a late write EPIPE/ECONNRESET on the client
socket. APIRequestContext does not listen for that, so the process dies.
Fixes: microsoft#42074
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cd1c251-bdcb-4f5f-b9a6-17ed643d095e
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Hi, I'm the Playwright bot and I took a first look at the CI failures here.

🔴 The failures are caused by this PR — by design

Eight of the nine failures are the new should not crash when server refuses body before reading it test this PR adds, failing on chromium, firefox and webkit. That's exactly what the PR sets out to prove: it's a repro-only draft with no fix yet, so red CI is the expected outcome. The lone unrelated failure (test-step.spec.ts:1728 on macOS) is a pre-existing flake.

Details

Caused by this PR

Pre-existing flake / infra

  • [playwright-test] › test-step.spec.ts:1728 › show api calls inside expects (macos-latest-node22) — pre-existing flake. In the aggregated results DB this test failed 1 of 3876 runs, and that one failure was on an unrelated PR (feat(webkit): roll to r2331 #41783, sha 22abdad), same macos-latest-node22 bot — a SHA this PR can't be responsible for. The diff only touches tests/library/global-fetch.spec.ts, nowhere near the test runner's step reporting.

Triaged by the Playwright bot - agent run

Listen for errors on the request socket and ignore network write
errors once response headers have arrived, matching undici policy.
Otherwise a server that answers (e.g. 413) without reading the body
can leave an unhandled socket EPIPE/ECONNRESET and kill the process.
Fixes: microsoft#42074
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cd1c251-bdcb-4f5f-b9a6-17ed643d095e
@Skn0ttSimon Knott (Skn0tt) changed the title test(fetch): reproduce APIRequestContext crash on refused bodyfix(fetch): do not crash on late EPIPE after refused bodyAug 4, 2026
@Skn0tt
Simon Knott (Skn0tt) marked this pull request as ready for review August 4, 2026 09:52
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cd1c251-bdcb-4f5f-b9a6-17ed643d095e
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Remove the per-request socket error handler when the request closes and
the socket is still alive, so keep-alive reuse does not accumulate
listeners. If the socket is already destroyed, leave the handler for a
possible late write EPIPE.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cd1c251-bdcb-4f5f-b9a6-17ed643d095e
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.


socket.on('error', handleRequestError);
request.once('close', () => {
if (!socket.destroyed)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment explaining why it has to be removed and only if !socket.destroyed.

@Skn0ttSimon Knott (Skn0tt)Aug 5, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 4d0640d - kept the destroy check and documented why.

Simon Knott (Skn0tt)and others added 2 commits August 5, 2026 12:53
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cd1c251-bdcb-4f5f-b9a6-17ed643d095e
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5cd1c251-bdcb-4f5f-b9a6-17ed643d095e
@Skn0tt
Simon Knott (Skn0tt) merged commit d5b1217 into microsoft:mainAug 5, 2026
45 of 48 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

7 flaky⚠️ [chromium-library] › library/screencast.spec.ts:28 › screencast.start delivers frames via onFrame callback `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:495 › screencast › should capture static page in persistent context Radoslav Kirilov (@smoke) `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:299 › should report intercepted service worker requests in HAR `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:699 › screencast › should capture full viewport on hidpi `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/chromium/chromium.spec.ts:299 › should report intercepted service worker requests in HAR `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-page] › page/page-goto.spec.ts:90 › should work with Cross-Origin-Opener-Policy `@firefox-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:388 › should reveal errors in the sourcetab `@windows-latest-node22`

51230 passed, 1190 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

3 failed
❌ [chrome] › mcp/annotate.spec.ts:57 › should capture multiple screenshots in one annotation @mcp-windows-latest-chrome
❌ [chromium] › mcp/cli-save-as.spec.ts:41 › screenshot --hires @mcp-macos-latest-chromium
❌ [firefox] › mcp/annotate.spec.ts:137 › should abort MCP annotation when last screenshot is removed @mcp-windows-latest-firefox

8047 passed, 1284 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Hi, I'm the Playwright bot and I took a first look at the CI failures here.

🟢 CI is clear — all three failures are pre-existing flakes

The latest report (MCP run) has three failures, and none of them touch this PR's change. The diff only edits fetch.ts error handling on the API request path, while these are all MCP screenshot/annotation tests. Each one flips verdict across ~1700 runs on SHAs this PR can't be responsible for.

Details

The diff changes late-EPIPE/ECONNRESET handling in packages/playwright-core/src/server/fetch.ts plus a global-fetch.spec.ts test. That's the network fetch path — the MCP screenshot and annotation flows don't exercise it. All three failures landed on different browser/OS bots, which is the divergence pattern you'd expect from unrelated flakes.

Pre-existing flake / infra

  • [chrome] › mcp/annotate.spec.ts:57 › should capture multiple screenshots in one annotation (mcp-windows-latest-chrome) — bimodal in the results DB: failed 31 of 1750 runs on chrome, and it also flakes on chromium (2/1692), firefox (7/1666), webkit (6/1699) and msedge (1/441). Fails across ~30 unrelated PRs (41463, 41669, … 42132), so this PR isn't the cause.
  • [chromium] › mcp/cli-save-as.spec.ts:41 › screenshot --hires (mcp-macos-latest-chromium) — failed 1 of 1692 chromium runs, on an unrelated PR — a one-off flake on a test that otherwise passes everywhere (0 failures across chrome/firefox/webkit/msedge).
  • [firefox] › mcp/annotate.spec.ts:137 › should abort MCP annotation when last screenshot is removed (mcp-windows-latest-firefox) — bimodal: failed 3 of 1666 firefox runs, plus 1/1750 chrome and 4/441 msedge, on SHAs unrelated to this PR.

Of 56 failing runs for these three tests across the whole DB, only 3 are on this PR (one per test) — the rest are spread across dozens of other PRs. That's a flake fingerprint, not a regression.

For completeness: the earlier "tests 1" reports on this PR came back with only flaky (retry-rescued) results, no hard failures. The new global-fetch.spec.ts test this PR adds is passing now that the fix is in.

Triaged by the Playwright bot

Triaged by the Playwright bot - agent run

Sign up for freeto 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]: APIRequestContext kills the process on an unhandled request-socket ECONNRESET when a server refuses a body before reading it

2 participants

@Skn0tt@yury-s