From 1237a01ed20c82ca2c039ac93d9f305528b0a769 Mon Sep 17 00:00:00 2001 From: Simon Knott Date: Thu, 6 Aug 2026 14:05:02 +0200 Subject: [PATCH] fix(connection): reject pending callbacks on transport death Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3351313b-bb35-475c-ae04-f79e1d2e4086 --- playwright/_impl/_browser_type.py | 9 +------ playwright/_impl/_connection.py | 43 ++++++++++++++----------------- 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/playwright/_impl/_browser_type.py b/playwright/_impl/_browser_type.py index dcc8db413..0e664a27d 100644 --- a/playwright/_impl/_browser_type.py +++ b/playwright/_impl/_browser_type.py @@ -257,15 +257,8 @@ def handle_transport_close(reason: Optional[str]) -> None: for page in context.pages: page._on_close() context._on_close() - connection.cleanup(reason) - # Give a chance to any API call promises to reject upon page/context closure. - # This happens naturally when we receive page.onClose and browser.onClose from the server - # in separate tasks. However, upon pipe closure we used to dispatch them all synchronously - # here and promises did not have a chance to reject. - # The order of rejects vs closure is a part of the API contract and our test runner - # relies on it to attribute rejections to the right test. - if browser: connection._loop.call_soon(browser._on_close) + connection.cleanup(reason) transport.once("close", handle_transport_close) diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 57a9dcf6d..29f781f49 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -144,13 +144,7 @@ async def _inner_send( self._object, method, augmented_params, timeout ) try: - done, _ = await asyncio.wait( - { - self._connection._transport.on_error_future, - callback.future, - }, - return_when=asyncio.FIRST_COMPLETED, - ) + result = await callback.future except asyncio.CancelledError as exc: await self._connection._abort( self._object, @@ -158,9 +152,6 @@ async def _inner_send( str(exc) or "Task was cancelled", ) raise - if not callback.future.done(): - callback.future.cancel() - result = next(iter(done)).result() # Protocol now has named return values, assume result is one level deeper unless # there is explicit ambiguity. if not result: @@ -351,9 +342,20 @@ async def init() -> None: if not self.playwright_future.done(): self.playwright_future.set_exception(exc) - await self._transport.connect() - self._init_task = self._loop.create_task(init()) - await self._transport.run() + try: + await self._transport.connect() + self._init_task = self._loop.create_task(init()) + await self._transport.run() + finally: + cause = None + if ( + self._transport.on_error_future.done() + and not self._transport.on_error_future.cancelled() + ): + transport_exc = self._transport.on_error_future.exception() + if transport_exc is not None: + cause = str(transport_exc) + self.cleanup(cause) def stop_sync(self) -> None: self._transport.request_stop() @@ -367,6 +369,8 @@ async def stop_async(self) -> None: self.cleanup() def cleanup(self, cause: str = None) -> None: + if self._closed_error: + return self._closed_error = TargetClosedError(cause) if cause else TargetClosedError() if self._init_task and not self._init_task.done(): self._init_task.cancel() @@ -463,19 +467,12 @@ async def _abort( except (Error, OSError): pass try: - done, _ = await asyncio.wait( - { - self._transport.on_error_future, - callback.future, - }, - return_when=asyncio.FIRST_COMPLETED, - ) + await callback.future + except (Exception, asyncio.CancelledError): + pass finally: if not callback.future.done(): callback.future.cancel() - for future in done: - if not future.cancelled(): - future.exception() def dispatch(self, msg: ParsedMessagePayload) -> None: if self._closed_error: