Skip to content

Commit 1237a01

Browse files
Skn0ttCopilot
andcommitted
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
1 parent 154f67c commit 1237a01

2 files changed

Lines changed: 21 additions & 31 deletions

File tree

playwright/_impl/_browser_type.py

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -257,15 +257,8 @@ def handle_transport_close(reason: Optional[str]) -> None:
257257
for page in context.pages:
258258
page._on_close()
259259
context._on_close()
260-
connection.cleanup(reason)
261-
# Give a chance to any API call promises to reject upon page/context closure.
262-
# This happens naturally when we receive page.onClose and browser.onClose from the server
263-
# in separate tasks. However, upon pipe closure we used to dispatch them all synchronously
264-
# here and promises did not have a chance to reject.
265-
# The order of rejects vs closure is a part of the API contract and our test runner
266-
# relies on it to attribute rejections to the right test.
267-
if browser:
268260
connection._loop.call_soon(browser._on_close)
261+
connection.cleanup(reason)
269262

270263
transport.once("close", handle_transport_close)
271264

playwright/_impl/_connection.py

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -144,23 +144,14 @@ async def _inner_send(
144144
self._object, method, augmented_params, timeout
145145
)
146146
try:
147-
done, _ = await asyncio.wait(
148-
{
149-
self._connection._transport.on_error_future,
150-
callback.future,
151-
},
152-
return_when=asyncio.FIRST_COMPLETED,
153-
)
147+
result = await callback.future
154148
except asyncio.CancelledError as exc:
155149
await self._connection._abort(
156150
self._object,
157151
callback,
158152
str(exc) or "Task was cancelled",
159153
)
160154
raise
161-
if not callback.future.done():
162-
callback.future.cancel()
163-
result = next(iter(done)).result()
164155
# Protocol now has named return values, assume result is one level deeper unless
165156
# there is explicit ambiguity.
166157
if not result:
@@ -351,9 +342,20 @@ async def init() -> None:
351342
if not self.playwright_future.done():
352343
self.playwright_future.set_exception(exc)
353344

354-
await self._transport.connect()
355-
self._init_task = self._loop.create_task(init())
356-
await self._transport.run()
345+
try:
346+
await self._transport.connect()
347+
self._init_task = self._loop.create_task(init())
348+
await self._transport.run()
349+
finally:
350+
cause = None
351+
if (
352+
self._transport.on_error_future.done()
353+
and not self._transport.on_error_future.cancelled()
354+
):
355+
transport_exc = self._transport.on_error_future.exception()
356+
if transport_exc is not None:
357+
cause = str(transport_exc)
358+
self.cleanup(cause)
357359

358360
def stop_sync(self) -> None:
359361
self._transport.request_stop()
@@ -367,6 +369,8 @@ async def stop_async(self) -> None:
367369
self.cleanup()
368370

369371
def cleanup(self, cause: str = None) -> None:
372+
if self._closed_error:
373+
return
370374
self._closed_error = TargetClosedError(cause) if cause else TargetClosedError()
371375
if self._init_task and not self._init_task.done():
372376
self._init_task.cancel()
@@ -463,19 +467,12 @@ async def _abort(
463467
except (Error, OSError):
464468
pass
465469
try:
466-
done, _ = await asyncio.wait(
467-
{
468-
self._transport.on_error_future,
469-
callback.future,
470-
},
471-
return_when=asyncio.FIRST_COMPLETED,
472-
)
470+
await callback.future
471+
except (Exception, asyncio.CancelledError):
472+
pass
473473
finally:
474474
if not callback.future.done():
475475
callback.future.cancel()
476-
for future in done:
477-
if not future.cancelled():
478-
future.exception()
479476

480477
def dispatch(self, msg: ParsedMessagePayload) -> None:
481478
if self._closed_error:

0 commit comments

Comments
 (0)