Skip to content

fix(expect): report target close reason instead of internal protocol error - #42861

Merged
Pavel Feldman (pavelfeldman) merged 1 commit into
microsoft:mainfrom
pavelfeldman:fix-expect-close-reason
Sep 23, 2026
Merged

Pavel Feldman (pavelfeldman) merged 1 commit into
microsoft:mainfrom
pavelfeldman:fix-expect-close-reason

Conversation

@pavelfeldman

Copy link
Copy Markdown
Member

Summary

  • When the target closes while an expect poll has a CDP call in flight, the call rejects with the raw Internal server error, session closed. protocol error. Frame.expect wraps it into ExpectError, so the dispatcher never gets to substitute the close reason.
  • Extract the dispatcher's close-reason rewriting into a shared helper and apply it in Frame.expect too, so the call log shows e.g. Test ended.
  • When the target closed before any intermediate result was collected, report the close message instead of a fabricated empty received value.

…error

When the target closes while an expect poll has a CDP call in flight, the
call rejects with the raw "Internal server error, session closed." protocol
error. Frame.expect wraps it into ExpectError, so the dispatcher never gets
a chance to substitute the close reason.

Extract the dispatcher's close-reason rewriting into a shared helper and
apply it in Frame.expect as well. Also use the close message as the error
detail when the target closed before any intermediate result was collected,
instead of reporting a fabricated empty received value.
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

5 flaky ⚠️ [chromium-library] › library/browsertype-connect.spec.ts:959 › run-server › socks proxy › should proxy requests from fetch api `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/video.spec.ts:762 › screencast › should work with video+trace `@realtime-time-library-chromium-linux`
⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-node22`
⚠️ [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`

52088 passed, 1252 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [firefox] › mcp/annotate.spec.ts:446 › should switch screencast to -s session on show --annotate @mcp-windows-latest-firefox
❌ [webkit] › mcp/http.spec.ts:449 › http transport shared context refuses browser_close @mcp-macos-latest-webkit

8690 passed, 1474 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

🟡 One known flake, one I can't clear

Hi, I'm the Playwright bot and I triaged the two MCP failures. The Firefox annotate one is a long-standing flake. The WebKit http.spec.ts:449 timeout I can't explain away — it hit both CI runs of this PR and has never failed anywhere else in the results DB.

Details

The diff is confined to error rewriting on target close (errors.ts, dispatcher.ts, Frame.expect) plus one new page test. Neither MCP test asserts on close-reason messages, so nothing in the diff obviously reaches them — but "obviously unrelated" isn't the same as proven, and the WebKit one has a suspicious run-for-run correlation.

Pre-existing flake / infra

  • [firefox] › mcp/annotate.spec.ts:446 › should switch screencast to -s session on show --annotate (@mcp-windows-latest-firefox) — pre-existing Firefox-only flake. Across the aggregated CI results this test failed 45 of 724 runs (6%) on Firefox, on SHAs and PRs unrelated to this one, while WebKit is 0/739 and Chromium 1/736. Screencast timing on Firefox, not this diff.

Uncertain

  • [webkit] › mcp/http.spec.ts:449 › http transport shared context refuses browser_close (@mcp-macos-latest-webkit) — Test timeout of 30000ms exceeded. This test is otherwise rock solid: 0 failures in 366 runs, and on this exact bot p50 is 2.4s with a historical max of 16.5s, so a 30s timeout is a real outlier. It failed on both runs of this PR (fff4b2c and 8bb00d3), same bot, and I found no occurrence on any other SHA or PR — so by the flake bar I can't call it noise, even though I see no path from the diff to a shared-context MCP HTTP test.

    To settle it: re-run just the macOS/WebKit MCP bot on this SHA and on main at 3bbcbca. If it reproduces on main too it's environmental; if it only reproduces here, the blob report's step tree will show which callTool hung.

Triaged by the Playwright bot - agent run

@pavelfeldman
Pavel Feldman (pavelfeldman) merged commit 091e4b9 into microsoft:main Sep 23, 2026
43 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.

3 participants