Skip to content

Commit 154f67c

Browse files
Skn0ttCopilot
andauthored
fix(sync): wait for initialize before leaving __enter__ (#3168)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 16cb16dc-1986-42c7-90ce-9549b32cfccb
1 parent 4af2fc6 commit 154f67c

3 files changed

Lines changed: 54 additions & 25 deletions

File tree

‎playwright/_impl/_connection.py‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,6 @@ def __init__(
305305
self._dispatcher_fiber = dispatcher_fiber
306306
self._transport = transport
307307
self._transport.on_message = lambda msg: self.dispatch(msg)
308-
self._waiting_for_object: Dict[str, Callable[[ChannelOwner], None]] = {}
309308
self._last_id = 0
310309
self._objects: Dict[str, ChannelOwner] = {}
311310
self._callbacks: Dict[int, ProtocolCallback] = {}
@@ -341,7 +340,16 @@ async def run(self) -> None:
341340
self._root_object = RootChannelOwner(self)
342341

343342
async def init() -> None:
344-
self.playwright_future.set_result(await self._root_object.initialize())
343+
try:
344+
result = await self._root_object.initialize()
345+
if not self.playwright_future.done():
346+
self.playwright_future.set_result(result)
347+
except Exception as exc:
348+
# No re-raise: callers observe playwright_future; a task
349+
# exception would log "never retrieved". Skip set_* if async
350+
# __aenter__ already cancelled the future after a transport error.
351+
if not self.playwright_future.done():
352+
self.playwright_future.set_exception(exc)
345353

346354
await self._transport.connect()
347355
self._init_task = self._loop.create_task(init())
@@ -374,11 +382,6 @@ def cleanup(self, cause: str = None) -> None:
374382
self._callbacks.clear()
375383
self.emit("close")
376384

377-
def call_on_object_with_known_name(
378-
self, guid: str, callback: Callable[[ChannelOwner], None]
379-
) -> None:
380-
self._waiting_for_object[guid] = callback
381-
382385
def set_is_tracing(self, is_tracing: bool) -> None:
383386
if is_tracing:
384387
self._tracing_count += 1
@@ -574,10 +577,7 @@ def _create_remote_object(
574577
self, parent: ChannelOwner, type: str, guid: str, initializer: Dict
575578
) -> ChannelOwner:
576579
initializer = self._replace_guids_with_channels(initializer)
577-
result = self._object_factory(parent, type, guid, initializer)
578-
if guid in self._waiting_for_object:
579-
self._waiting_for_object.pop(guid)(result)
580-
return result
580+
return self._object_factory(parent, type, guid, initializer)
581581

582582
def _replace_channels_with_guids(
583583
self,

‎playwright/sync_api/_context_manager.py‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,14 @@
1313
# limitations under the License.
1414

1515
import asyncio
16-
from typing import TYPE_CHECKING, Any, Optional, cast
16+
from typing import TYPE_CHECKING, Any, Optional
1717

1818
from greenlet import greenlet
1919

20-
from playwright._impl._connection import ChannelOwner, Connection
20+
from playwright._impl._connection import Connection
2121
from playwright._impl._errors import Error
2222
from playwright._impl._greenlets import MainGreenlet
2323
from playwright._impl._object_factory import create_remote_object
24-
from playwright._impl._playwright import Playwright
2524
from playwright._impl._transport import PipeTransport
2625
from playwright.sync_api._generated import Playwright as SyncPlaywright
2726

@@ -66,19 +65,19 @@ def greenlet_main() -> None:
6665

6766
g_self = greenlet.getcurrent()
6867

69-
def callback_wrapper(channel_owner: ChannelOwner) -> None:
70-
playwright_impl = cast(Playwright, channel_owner)
71-
self._playwright = SyncPlaywright(playwright_impl)
72-
g_self.switch()
73-
74-
# Switch control to the dispatcher, it'll fire an event and pass control to
75-
# the calling greenlet.
76-
self._connection.call_on_object_with_known_name("Playwright", callback_wrapper)
68+
# Wait until initialize completes (not just Playwright __create__), matching async.
69+
self._connection.playwright_future.add_done_callback(lambda _: g_self.switch())
7770
dispatcher_fiber.switch()
7871

79-
playwright = self._playwright
80-
playwright.stop = self.__exit__ # type: ignore
81-
return playwright
72+
try:
73+
self._playwright = SyncPlaywright(
74+
self._connection.playwright_future.result()
75+
)
76+
except BaseException:
77+
self.__exit__()
78+
raise
79+
self._playwright.stop = self.__exit__ # type: ignore
80+
return self._playwright
8281

8382
def start(self) -> SyncPlaywright:
8483
return self.__enter__()

‎tests/sync/test_context_manager.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
import subprocess
16+
import sys
17+
import textwrap
18+
from pathlib import Path
1519
from typing import Dict
1620

1721
import pytest
@@ -34,3 +38,29 @@ def test_context_managers_not_hang(context: BrowserContext) -> None:
3438
with pytest.raises(Exception, match="Oops!"):
3539
with context.new_page():
3640
raise Exception("Oops!")
41+
42+
43+
def test_empty_sync_playwright_does_not_warn(tmp_path: Path) -> None:
44+
# Regression test for https://github.com/microsoft/playwright-python/issues/3165.
45+
# __enter__ must wait for initialize to finish; otherwise teardown races the
46+
# in-flight init callback and prints asyncio warnings on an empty with-block.
47+
script = tmp_path / "empty_sync_playwright.py"
48+
script.write_text(
49+
textwrap.dedent(
50+
"""
51+
from playwright.sync_api import sync_playwright
52+
53+
with sync_playwright() as pw:
54+
pass
55+
"""
56+
)
57+
)
58+
result = subprocess.run(
59+
[sys.executable, str(script)],
60+
capture_output=True,
61+
text=True,
62+
timeout=30,
63+
)
64+
assert result.returncode == 0, result.stderr
65+
assert "Future exception was never retrieved" not in result.stderr
66+
assert "Task was destroyed but it is pending" not in result.stderr

0 commit comments

Comments
 (0)