fix(connection): register protocol callback only after successful send

playwright/_impl/_connection.py

@@ -441,8 +441,8 @@ def _send_message_to_server(
         if self._tracing_count > 0 and frames and object._guid != "localUtils":
             self.local_utils.add_stack_to_tracing_no_reply(id, frames)
 
-        self._callbacks[id] = callback
         self._transport.send(message)
+        self._callbacks[id] = callback
 
         return callback
 

tests/async/test_asyncio.py

@@ -32,11 +32,11 @@ async def test_should_cancel_underlying_protocol_calls(
 ) -> None:
     handler_exception = None
 
-    def exception_handlerdler(loop: asyncio.AbstractEventLoop, context: Dict) -> None:
+    def exception_handler(loop: asyncio.AbstractEventLoop, context: Dict) -> None:
         nonlocal handler_exception
         handler_exception = context["exception"]
 
-    asyncio.get_running_loop().set_exception_handler(exception_handlerdler)
+    asyncio.get_running_loop().set_exception_handler(exception_handler)
 
     async with async_playwright() as p:
         browser = await p[browser_name].launch(**launch_arguments)

tests/sync/test_sync.py

@@ -14,7 +14,11 @@
 
 import multiprocessing
 import os
+import subprocess
+import sys
+import textwrap
 from datetime import timedelta
+from pathlib import Path
 from typing import Any, Callable, Dict
 
 import pytest
@@ -361,6 +365,40 @@ def test_should_return_proper_api_name_on_error(page: Page) -> None:
         assert str(error).startswith("Page.evaluate:")
 
 
+def test_should_not_orphan_callback_on_non_serializable_params(
+    browser_name: str,
+    launch_arguments: Dict[str, Any],
+    tmp_path: Path,
+) -> None:
+    # Regression test for https://github.com/microsoft/playwright-python/issues/3165.
+    # Run in a subprocess so "Future exception was never retrieved" on exit is visible on stderr.
+    script = tmp_path / "orphan_callback.py"
+    script.write_text(
+        textwrap.dedent(
+            f"""
+            from playwright.sync_api import sync_playwright
+
+            with sync_playwright() as p:
+                browser = p[{browser_name!r}].launch(**{launch_arguments!r})
+                page = browser.new_page()
+                try:
+                    page.locator("asdf").highlight(style=object())
+                except TypeError:
+                    pass
+                browser.close()
+            """
+        )
+    )
+    result = subprocess.run(
+        [sys.executable, str(script)],
+        capture_output=True,
+        text=True,
+        timeout=60,
+    )
+    assert result.returncode == 0, result.stderr
+    assert "Future exception was never retrieved" not in result.stderr
+
+
 def test_click_should_accept_timedelta_for_timeout(page: Page) -> None:
     with pytest.raises(TimeoutError, match="Timeout 1ms exceeded"):
         page.click("does-not-exist", timeout=timedelta(milliseconds=1))