Skip to content

fix(fetch): preserve node error fields across serialization - #42562

Merged
Dmitry Gozman (dgozman) merged 2 commits into
microsoft:mainfrom
dgozman:fix-42532
Sep 8, 2026
Merged

Dmitry Gozman (dgozman) merged 2 commits into
microsoft:mainfrom
dgozman:fix-42532

Conversation

@dgozman

@dgozman Dmitry Gozman (dgozman) commented Sep 4, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Serialize the structured fields of a Node.js system error — code, errno, syscall, address, port, hostname — alongside the message, so APIRequestContext failures can be classified without parsing the error text.
  • Fields are whitelisted in one shared table in @protocol/serializers, and copied only when the runtime type matches the protocol type.
  • Include error.code into the error message for other languages.

Fixes #42532

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

else
e = Object.assign(new PlaywrightError(error.error.message), { name: error.error.name });
e.stack = error.error.stack || '';
parseSystemErrorFields(error.error, e);

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.

can we just append it to the message, so that it works across langs?

Serialize the structured fields of a Node.js system error (code, errno,
syscall, address, port, hostname) alongside the message, so that callers
can classify network failures without parsing the error text.

Fixes: microsoft#42532
Node does not always mention the code in the message, e.g. "socket hang
up" is an ECONNRESET. Append it during serialization, so that clients in
all languages can tell the errors apart.
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

6 flaky ⚠️ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:356 › screencast › should work for popups `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace `@chromium-ubuntu-22.04-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`
⚠️ [playwright-test] › ui-mode-trace.spec.ts:827 › should update state on subsequent run `@windows-latest-node22`

51374 passed, 1247 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Test results for "MCP"

1 failed
❌ [chromium] › mcp/annotate.spec.ts:291 › should enter annotate mode on fresh dashboard.tsx mount with -s --annotate @mcp-windows-latest-chromium

8326 passed, 1371 skipped


Merge workflow run.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

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

🟢 The one failure is a pre-existing flake — this PR is clear

mcp/annotate.spec.ts:291 doesn't touch fetch/error serialization, and it flips verdict across ~700 runs on unrelated SHAs and PRs. Nothing here points at this change.

Details

This PR only changes error serialization for APIRequestContext failures (serializeError/parseError plus the @protocol/serializers system-error whitelist). The one red is an MCP annotate-mode test that exercises none of that. The "tests 1" report had 0 failures (6 flaky).

Pre-existing flake / infra

  • [chromium] › mcp/annotate.spec.ts:291 › should enter annotate mode on fresh dashboard.tsx mount with -s --annotate (@mcp-windows-latest-chromium) — bimodal flake. On the chromium project it failed 2 of 717 runs and passed the other 715; across all projects it fails a handful of times each. Every failure in the DB is on an unrelated target — pushes to main (SHAs d506664, 1e9d2b1, 5812882) and PRs fix(trace-viewer): store snapshot style text in an attribute #42405 and docs(browsercontext): undeprecate setHTTPCredentials #42419 — none related to this branch (8b04f54). The annotate CLI flow doesn't run fetch or touch SerializedError, so this change can't reach it.

Triaged by the Playwright bot — agent run

Triaged by the Playwright bot - agent run

@dgozman
Dmitry Gozman (dgozman) merged commit fab00c0 into microsoft:main Sep 8, 2026
44 of 45 checks passed
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]: APIRequestContext errors lose structured fields (err.code is undefined) after cross-process serialization

2 participants