fix: order remote browser close events

playwright/_impl/_browser_type.py

@@ -259,15 +259,15 @@ def handle_transport_close(reason: Optional[str]) -> None:
                     for page in context.pages:
                         page._on_close()
                     context._on_close()
-                browser._on_close()
             connection.cleanup(reason)
-            # TODO: Backport https://github.com/microsoft/playwright/commit/d8d5289e8692c9b1265d23ee66988d1ac5122f33
             # Give a chance to any API call promises to reject upon page/context closure.
             # This happens naturally when we receive page.onClose and browser.onClose from the server
             # in separate tasks. However, upon pipe closure we used to dispatch them all synchronously
             # here and promises did not have a chance to reject.
             # The order of rejects vs closure is a part of the API contract and our test runner
             # relies on it to attribute rejections to the right test.
+            if browser:
+                connection._loop.call_soon(browser._on_close)
 
         transport.once("close", handle_transport_close)
 

playwright/_impl/_waiter.py

@@ -22,7 +22,7 @@
 from pyee import EventEmitter
 
 from playwright._impl._connection import ChannelOwner
-from playwright._impl._errors import Error, TimeoutError
+from playwright._impl._errors import Error, TimeoutError, is_target_closed_error
 
 
 class Waiter:
@@ -50,20 +50,29 @@ def _wait_for_event_info_before(self, wait_id: str, event: str) -> None:
         )
 
     def _wait_for_event_info_after(self, wait_id: str, error: Exception = None) -> None:
-        self._channel._connection.wrap_api_call_sync(
-            lambda: self._channel.send_no_reply(
-                "waitForEventInfo",
-                None,
-                {
-                    "info": {
-                        "waitId": wait_id,
-                        "phase": "after",
-                        **({"error": str(error)} if error else {}),
+        try:
+            self._channel._connection.wrap_api_call_sync(
+                lambda: self._channel.send_no_reply(
+                    "waitForEventInfo",
+                    None,
+                    {
+                        "info": {
+                            "waitId": wait_id,
+                            "phase": "after",
+                            **({"error": str(error)} if error else {}),
+                        },
                     },
-                },
-            ),
-            True,
-        )
+                ),
+                True,
+            )
+        except Error as e:
+            # This is best-effort telemetry. During remote browser shutdown, the
+            # waiter result has already been resolved, but the pipe may be closing.
+            if (
+                not is_target_closed_error(e)
+                and "Playwright connection closed" not in e.message
+            ):
+                raise
 
     def reject_on_event(
         self,

tests/async/test_browsertype_connect.py

@@ -180,6 +180,57 @@ async def test_browser_type_connect_should_reject_navigation_when_browser_closes
     assert "has been closed" in exc_info.value.message
 
 
+async def test_browser_type_connect_should_reject_wait_for_event_before_browser_close_finishes(
+    browser_type: BrowserType, launch_server: Callable[[], RemoteServer]
+) -> None:
+    remote_server = launch_server()
+    browser = await browser_type.connect(remote_server.ws_endpoint)
+    page = await browser.new_page()
+
+    rejected = False
+
+    async def wait_for_download() -> None:
+        nonlocal rejected
+        try:
+            await page.wait_for_event("download")
+        except Error:
+            rejected = True
+
+    wait_task = asyncio.create_task(wait_for_download())
+    await asyncio.sleep(0)
+
+    await browser.close()
+    assert rejected is True
+    await wait_task
+
+
+async def test_browser_type_connect_should_reject_wait_for_event_before_disconnected(
+    browser_type: BrowserType, launch_server: Callable[[], RemoteServer]
+) -> None:
+    remote_server = launch_server()
+    browser = await browser_type.connect(remote_server.ws_endpoint)
+    page = await browser.new_page()
+    log = []
+
+    async def wait_for_download() -> None:
+        try:
+            await page.wait_for_event("download")
+        except Error:
+            log.append("rejected")
+
+    wait_task = asyncio.create_task(wait_for_download())
+    await asyncio.sleep(0)
+    browser.on("disconnected", lambda _: log.append("disconnected"))
+
+    remote_server.kill()
+    await wait_task
+    for _ in range(10):
+        if len(log) == 2:
+            break
+        await asyncio.sleep(0.1)
+    assert log == ["rejected", "disconnected"]
+
+
 async def test_should_not_allow_getting_the_path(
     browser_type: BrowserType, launch_server: Callable[[], RemoteServer], server: Server
 ) -> None: