← Back to PRs

#17518: fix(browser): catch dialog accept/dismiss errors after page navigation

by aldoeliacim open 2026-02-15 21:02 View on GitHub →
stale size: XS
## Summary Fixes #17499 When a page navigates or a dialog auto-dismisses before the Playwright dialog handler fires, `dialog.accept()`/`dialog.dismiss()` throws a `Protocol error (Page.handleJavaScriptDialog): No dialog is showing`. The existing `.catch()` on the promise chain only handles `waitForEvent` timeouts. Errors thrown inside the `.then()` callback (from `accept()`/`dismiss()`) create a new rejected promise that propagates as an unhandled rejection. ## Fix Wrap `dialog.accept()`/`dialog.dismiss()` in a `try-catch` inside the `.then()` callback. The error is silently caught since the dialog being gone is a benign race condition. ## Testing - Lint passes (oxlint, 0 warnings/errors) - The change is a defensive catch around an existing code path — no behavior change when the dialog is present <!-- greptile_comment --> <h3>Greptile Summary</h3> Adds a `try-catch` around `dialog.accept()` / `dialog.dismiss()` in `armDialogViaPlaywright` to handle the race condition where a page navigates or a dialog auto-dismisses before the Playwright handler fires, preventing unhandled promise rejections. - Wraps `dialog.accept(promptText)` and `dialog.dismiss()` in a `try-catch` inside the `.then()` callback, silently catching errors when the dialog is already gone - Follows the existing defensive pattern already used in `armFileUploadViaPlaywright` (e.g., the `try-catch` around `page.keyboard.press("Escape")`) - The existing `.catch()` at the end of the promise chain only handles `waitForEvent` timeouts, not errors thrown inside `.then()` — this fix addresses that gap - No behavior change when the dialog is present; purely defensive error handling for a benign race condition <h3>Confidence Score: 5/5</h3> - This PR is safe to merge — it is a minimal, defensive try-catch that prevents unhandled rejections without changing any existing behavior. - The change is a 4-line try-catch wrapper around an existing code path. It follows established patterns in the same file, has no behavioral impact when dialogs are present, and correctly addresses a real race condition (referenced in issue #17499). No new logic, no new dependencies, no risk of regression. - No files require special attention. <sub>Last reviewed commit: 76c93a0</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs