Skip to content

chore: swallow TargetClosedErrors in beforeunload dialog.dismiss()#39788

Open
hbenl wants to merge 1 commit intomicrosoft:mainfrom
hbenl:dialog-dismiss-target-closed
Open

chore: swallow TargetClosedErrors in beforeunload dialog.dismiss()#39788
hbenl wants to merge 1 commit intomicrosoft:mainfrom
hbenl:dialog-dismiss-target-closed

Conversation

@hbenl
Copy link
Collaborator

@hbenl hbenl commented Mar 20, 2026

See #39701 (comment). This swallows TargetClosedErrors only for beforeunload dialogs because this test checks that dialog.dismiss() will throw for alert dialogs.

@hbenl hbenl force-pushed the dialog-dismiss-target-closed branch from 0554def to 5450e51 Compare March 20, 2026 13:06
@hbenl hbenl changed the title chore: swallow TargetClosedErrors in dialog.dismiss() chore: swallow TargetClosedErrors in beforeunload dialog.dismiss() Mar 20, 2026
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

Test results for "MCP"

7 failed
❌ [chrome] › mcp/test-debug.spec.ts:167 › test_debug (pause/snapshot/resume) @mcp-windows-latest
❌ [chromium] › mcp/test-run.spec.ts:87 › test_run filters @mcp-windows-latest
❌ [chromium] › mcp/test-setup.spec.ts:60 › setup should run global setup and teardown @mcp-windows-latest
❌ [chrome] › mcp/test-setup.spec.ts:60 › setup should run global setup and teardown @mcp-macos-latest
❌ [chromium] › mcp/cli-help.spec.ts:24 › prints command help @mcp-macos-latest
❌ [msedge] › mcp/test-debug.spec.ts:21 › test_debug (passed) @mcp-windows-latest
❌ [msedge] › mcp/test-debug.spec.ts:48 › test_debug (pause/resume) @mcp-windows-latest

5451 passed, 340 skipped


Merge workflow run.

@github-actions
Copy link
Contributor

Test results for "tests 1"

2 failed
❌ [playwright-test] › reporter.spec.ts:251 › created › should not have internal error when steps are finished after timeout @macos-latest-node20
❌ [playwright-test] › ui-mode-test-network-tab.spec.ts:397 › should not preserve selection across test runs @windows-latest-node20

8 flaky ⚠️ [chromium-library] › library/popup.spec.ts:261 › should not throw when click closes popup `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [chromium-library] › library/trace-viewer.spec.ts:1223 › should display language-specific locators `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:358 › screencast › should capture navigation `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/popup.spec.ts:261 › should not throw when click closes popup `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/trace-viewer.spec.ts:1223 › should display language-specific locators `@chromium-ubuntu-22.04-node22`
⚠️ [webkit-library] › library/screencast.spec.ts:75 › start reuses existing screencast when video recording is running `@webkit-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-test-output.spec.ts:118 › should collapse repeated console messages for test `@macos-latest-node20`
⚠️ [playwright-test] › ui-mode-test-source.spec.ts:129 › should show syntax errors in file `@windows-latest-node20`

38804 passed, 845 skipped


Merge workflow run.

try {
await this._channel.dismiss();
} catch (e) {
if (isTargetClosedError(e) && this.type() === 'beforeunload')
Copy link
Member

Choose a reason for hiding this comment

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

Why the beforeunload?

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.

2 participants