diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 12a1c60af..e88c996b0 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -305,7 +305,6 @@ def __init__( self._dispatcher_fiber = dispatcher_fiber self._transport = transport self._transport.on_message = lambda msg: self.dispatch(msg) - self._waiting_for_object: Dict[str, Callable[[ChannelOwner], None]] = {} self._last_id = 0 self._objects: Dict[str, ChannelOwner] = {} self._callbacks: Dict[int, ProtocolCallback] = {} @@ -341,7 +340,16 @@ async def run(self) -> None: self._root_object = RootChannelOwner(self) async def init() -> None: - self.playwright_future.set_result(await self._root_object.initialize()) + try: + result = await self._root_object.initialize() + if not self.playwright_future.done(): + self.playwright_future.set_result(result) + except Exception as exc: + # No re-raise: callers observe playwright_future; a task + # exception would log "never retrieved". Skip set_* if async + # __aenter__ already cancelled the future after a transport error. + if not self.playwright_future.done(): + self.playwright_future.set_exception(exc) await self._transport.connect() self._init_task = self._loop.create_task(init()) @@ -374,11 +382,6 @@ def cleanup(self, cause: str = None) -> None: self._callbacks.clear() self.emit("close") - def call_on_object_with_known_name( - self, guid: str, callback: Callable[[ChannelOwner], None] - ) -> None: - self._waiting_for_object[guid] = callback - def set_is_tracing(self, is_tracing: bool) -> None: if is_tracing: self._tracing_count += 1 @@ -574,10 +577,7 @@ def _create_remote_object( self, parent: ChannelOwner, type: str, guid: str, initializer: Dict ) -> ChannelOwner: initializer = self._replace_guids_with_channels(initializer) - result = self._object_factory(parent, type, guid, initializer) - if guid in self._waiting_for_object: - self._waiting_for_object.pop(guid)(result) - return result + return self._object_factory(parent, type, guid, initializer) def _replace_channels_with_guids( self, diff --git a/playwright/sync_api/_context_manager.py b/playwright/sync_api/_context_manager.py index feb648ca0..729830747 100644 --- a/playwright/sync_api/_context_manager.py +++ b/playwright/sync_api/_context_manager.py @@ -13,15 +13,14 @@ # limitations under the License. import asyncio -from typing import TYPE_CHECKING, Any, Optional, cast +from typing import TYPE_CHECKING, Any, Optional from greenlet import greenlet -from playwright._impl._connection import ChannelOwner, Connection +from playwright._impl._connection import Connection from playwright._impl._errors import Error from playwright._impl._greenlets import MainGreenlet from playwright._impl._object_factory import create_remote_object -from playwright._impl._playwright import Playwright from playwright._impl._transport import PipeTransport from playwright.sync_api._generated import Playwright as SyncPlaywright @@ -66,19 +65,19 @@ def greenlet_main() -> None: g_self = greenlet.getcurrent() - def callback_wrapper(channel_owner: ChannelOwner) -> None: - playwright_impl = cast(Playwright, channel_owner) - self._playwright = SyncPlaywright(playwright_impl) - g_self.switch() - - # Switch control to the dispatcher, it'll fire an event and pass control to - # the calling greenlet. - self._connection.call_on_object_with_known_name("Playwright", callback_wrapper) + # Wait until initialize completes (not just Playwright __create__), matching async. + self._connection.playwright_future.add_done_callback(lambda _: g_self.switch()) dispatcher_fiber.switch() - playwright = self._playwright - playwright.stop = self.__exit__ # type: ignore - return playwright + try: + self._playwright = SyncPlaywright( + self._connection.playwright_future.result() + ) + except BaseException: + self.__exit__() + raise + self._playwright.stop = self.__exit__ # type: ignore + return self._playwright def start(self) -> SyncPlaywright: return self.__enter__() diff --git a/tests/sync/test_context_manager.py b/tests/sync/test_context_manager.py index 6074691e9..f3dcd6081 100644 --- a/tests/sync/test_context_manager.py +++ b/tests/sync/test_context_manager.py @@ -12,6 +12,10 @@ # See the License for the specific language governing permissions and # limitations under the License. +import subprocess +import sys +import textwrap +from pathlib import Path from typing import Dict import pytest @@ -34,3 +38,29 @@ def test_context_managers_not_hang(context: BrowserContext) -> None: with pytest.raises(Exception, match="Oops!"): with context.new_page(): raise Exception("Oops!") + + +def test_empty_sync_playwright_does_not_warn(tmp_path: Path) -> None: + # Regression test for https://github.com/microsoft/playwright-python/issues/3165. + # __enter__ must wait for initialize to finish; otherwise teardown races the + # in-flight init callback and prints asyncio warnings on an empty with-block. + script = tmp_path / "empty_sync_playwright.py" + script.write_text( + textwrap.dedent( + """ + from playwright.sync_api import sync_playwright + + with sync_playwright() as pw: + pass + """ + ) + ) + result = subprocess.run( + [sys.executable, str(script)], + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, result.stderr + assert "Future exception was never retrieved" not in result.stderr + assert "Task was destroyed but it is pending" not in result.stderr