From c7e1bcea017430cce54f54849711228536051213 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 4 Aug 2026 17:06:15 +0200 Subject: [PATCH 1/2] fix(connection): register protocol callback only after successful send Fixes one of the two cases of https://github.com/microsoft/playwright-python/issues/3165 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 358aa828-ed44-482a-8a83-91d435d25b38 --- playwright/_impl/_connection.py | 2 +- tests/async/test_asyncio.py | 22 +++++++++++++++++-- tests/sync/test_sync.py | 38 +++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 12a1c60af..3a7356605 100644 --- a/playwright/_impl/_connection.py +++ b/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 diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 243a0eed8..1cb563f9b 100644 --- a/tests/async/test_asyncio.py +++ b/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() diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index fb9c7c16e..7e3d36977 100644 --- a/tests/sync/test_sync.py +++ b/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)) From a7dd1de1ed643ef7dd97247a310e1df1685b369c Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Tue, 4 Aug 2026 17:10:45 +0200 Subject: [PATCH 2/2] test: drop white-box async orphan-callback check The sync subprocess test already covers the user-visible warning. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 358aa828-ed44-482a-8a83-91d435d25b38 --- tests/async/test_asyncio.py | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 1cb563f9b..2c90cb494 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -68,24 +68,6 @@ def exception_handler(loop: asyncio.AbstractEventLoop, context: Dict) -> None: 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()