Skip to content

Commit 86d3004

Browse files
Skn0ttCopilot
andauthored
fix(asyncio): abort protocol calls on task cancellation (#3144)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 9d7aded commit 86d3004

2 files changed

Lines changed: 64 additions & 13 deletions

File tree

playwright/_impl/_connection.py

Lines changed: 48 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -142,13 +142,21 @@ async def _inner_send(
142142
callback = self._connection._send_message_to_server(
143143
self._object, method, augmented_params, timeout
144144
)
145-
done, _ = await asyncio.wait(
146-
{
147-
self._connection._transport.on_error_future,
148-
callback.future,
149-
},
150-
return_when=asyncio.FIRST_COMPLETED,
151-
)
145+
try:
146+
done, _ = await asyncio.wait(
147+
{
148+
self._connection._transport.on_error_future,
149+
callback.future,
150+
},
151+
return_when=asyncio.FIRST_COMPLETED,
152+
)
153+
except asyncio.CancelledError as exc:
154+
await self._connection._abort(
155+
self._object,
156+
callback,
157+
str(exc) or "Task was cancelled",
158+
)
159+
raise
152160
if not callback.future.done():
153161
callback.future.cancel()
154162
result = next(iter(done)).result()
@@ -240,7 +248,10 @@ def remove_listener(self, event: str, f: Any) -> None:
240248

241249

242250
class ProtocolCallback:
243-
def __init__(self, loop: asyncio.AbstractEventLoop, no_reply: bool = False) -> None:
251+
def __init__(
252+
self, loop: asyncio.AbstractEventLoop, id: int, no_reply: bool = False
253+
) -> None:
254+
self.id = id
244255
self.stack_trace: traceback.StackSummary
245256
self.no_reply = no_reply
246257
self.future = loop.create_future()
@@ -389,7 +400,7 @@ def _send_message_to_server(
389400
)
390401
self._last_id += 1
391402
id = self._last_id
392-
callback = ProtocolCallback(self._loop, no_reply=no_reply)
403+
callback = ProtocolCallback(self._loop, id, no_reply=no_reply)
393404
task = asyncio.current_task(self._loop)
394405
callback.stack_trace = cast(
395406
traceback.StackSummary,
@@ -433,6 +444,34 @@ def _send_message_to_server(
433444

434445
return callback
435446

447+
async def _abort(
448+
self, object: ChannelOwner, callback: ProtocolCallback, reason: str
449+
) -> None:
450+
try:
451+
self._transport.send(
452+
{
453+
"guid": object._guid,
454+
"method": "__abort__",
455+
"params": {"id": callback.id, "reason": reason},
456+
}
457+
)
458+
except (Error, OSError):
459+
pass
460+
try:
461+
done, _ = await asyncio.wait(
462+
{
463+
self._transport.on_error_future,
464+
callback.future,
465+
},
466+
return_when=asyncio.FIRST_COMPLETED,
467+
)
468+
finally:
469+
if not callback.future.done():
470+
callback.future.cancel()
471+
for future in done:
472+
if not future.cancelled():
473+
future.exception()
474+
436475
def dispatch(self, msg: ParsedMessagePayload) -> None:
437476
if self._closed_error:
438477
return

tests/async/test_asyncio.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727

2828

2929
async def test_should_cancel_underlying_protocol_calls(
30-
browser_name: str, launch_arguments: Dict
30+
browser_name: str,
31+
launch_arguments: Dict,
3132
) -> None:
3233
handler_exception = None
3334

@@ -40,12 +41,23 @@ def exception_handlerdler(loop: asyncio.AbstractEventLoop, context: Dict) -> Non
4041
async with async_playwright() as p:
4142
browser = await p[browser_name].launch(**launch_arguments)
4243
page = await browser.new_page()
43-
task = asyncio.create_task(page.wait_for_selector("will-never-find"))
44-
# make sure that the wait_for_selector message was sent to the server (driver)
45-
await asyncio.sleep(0.1)
44+
await page.set_content(
45+
"""
46+
<button disabled onclick="window.clicked = true">click me</button>
47+
<script>window.clicked = false</script>
48+
"""
49+
)
50+
task = asyncio.create_task(page.locator("button").click(timeout=0))
51+
await page.wait_for_timeout(100)
52+
assert not task.done()
53+
4654
task.cancel()
4755
with pytest.raises(asyncio.CancelledError):
4856
await task
57+
58+
await page.locator("button").evaluate("button => button.disabled = false")
59+
await page.wait_for_timeout(700)
60+
assert not await page.evaluate("window.clicked")
4961
await browser.close()
5062

5163
# The actual 'Future exception was never retrieved' is logged inside the Future destructor (__del__).

0 commit comments

Comments
 (0)