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

Fixes one of the two cases of #3165

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 358aa828-ed44-482a-8a83-91d435d25b38

Author: Skn0tt

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)
@@ -68,6 +68,24 @@ def exception_handlerdler(loop: asyncio.AbstractEventLoop, context: Dict) -> Non
     asyncio.get_running_loop().set_exception_handler(None)
 
 
+async def test_should_not_orphan_callback_on_non_serializable_params(
+    browser_name: str,
+    launch_arguments: Dict,
+) -> None:
+    # Regression test for https://github.com/microsoft/playwright-python/issues/3165.
+    # A failed transport.send must not leave a ProtocolCallback in connection._callbacks
+    # (cleanup would later set_exception on it → "Future exception was never retrieved").
+    async with async_playwright() as p:
+        browser = await p[browser_name].launch(**launch_arguments)
+        page = await browser.new_page()
+        connection = page._impl_obj._connection
+        before = set(connection._callbacks)
+        with pytest.raises(TypeError, match="JSON serializable"):
+            await page.locator("asdf").highlight(style=object())  # type: ignore
+        assert set(connection._callbacks) == before
+        await browser.close()
+
+
 async def test_async_playwright_stop_multiple_times() -> None:
     playwright = await async_playwright().start()
     await playwright.stop()

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))