diff --git a/playwright/_impl/_browser_type.py b/playwright/_impl/_browser_type.py index 8abac6061..24b520abb 100644 --- a/playwright/_impl/_browser_type.py +++ b/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) diff --git a/playwright/_impl/_waiter.py b/playwright/_impl/_waiter.py index b5bf53382..c61b46eb8 100644 --- a/playwright/_impl/_waiter.py +++ b/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, diff --git a/tests/async/test_browsertype_connect.py b/tests/async/test_browsertype_connect.py index ccb112ab9..ca50407d2 100644 --- a/tests/async/test_browsertype_connect.py +++ b/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: