From 71c80af3352f18cd663aead585cb1118122f3b63 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Wed, 6 Jul 2022 23:23:46 -0700 Subject: [PATCH 01/10] fix: cleanup pending route handlers on close Fixes #1402. --- playwright/_impl/_browser_context.py | 14 ++++++-- playwright/_impl/_helper.py | 18 ++++++++++ playwright/_impl/_page.py | 16 ++++++--- .../test_browsercontext_request_intercept.py | 17 +++++++++ tests/async/test_request_intercept.py | 36 ++++++++++++++++++- .../test_browsercontext_request_intercept.py | 15 ++++++++ tests/sync/test_request_intercept.py | 31 +++++++++++++++- 7 files changed, 137 insertions(+), 10 deletions(-) diff --git a/playwright/_impl/_browser_context.py b/playwright/_impl/_browser_context.py index 844955322..94f9d1dee 100644 --- a/playwright/_impl/_browser_context.py +++ b/playwright/_impl/_browser_context.py @@ -48,6 +48,7 @@ from playwright._impl._frame import Frame from playwright._impl._har_router import HarRouter from playwright._impl._helper import ( + BackgroundTaskTracker, HarRecordingMetadata, RouteFromHarNotFoundPolicy, RouteHandler, @@ -103,6 +104,7 @@ def __init__( self._request: APIRequestContext = from_channel( initializer["APIRequestContext"] ) + self._background_task_tracker: BackgroundTaskTracker = BackgroundTaskTracker() self._channel.on( "bindingCall", lambda params: self._on_binding(from_channel(params["binding"])), @@ -113,7 +115,7 @@ def __init__( ) self._channel.on( "route", - lambda params: asyncio.create_task( + lambda params: self._background_task_tracker.create_task( self._on_route( from_channel(params.get("route")), from_channel(params.get("request")), @@ -163,8 +165,14 @@ def __init__( ), ) self._closed_future: asyncio.Future = asyncio.Future() + + def _on_close(_: Any) -> None: + self._background_task_tracker.close() + self._closed_future.set_result(True) + self.once( - self.Events.Close, lambda context: self._closed_future.set_result(True) + self.Events.Close, + _on_close, ) def __repr__(self) -> str: @@ -187,7 +195,7 @@ async def _on_route(self, route: Route, request: Request) -> None: handled = await route_handler.handle(route, request) finally: if len(self._routes) == 0: - asyncio.create_task(self._disable_interception()) + await self._disable_interception() if handled: return await route._internal_continue(is_internal=True) diff --git a/playwright/_impl/_helper.py b/playwright/_impl/_helper.py index fb2295298..f59ac8886 100644 --- a/playwright/_impl/_helper.py +++ b/playwright/_impl/_helper.py @@ -362,3 +362,21 @@ def is_file_payload(value: Optional[Any]) -> bool: and "mimeType" in value and "buffer" in value ) + + +class BackgroundTaskTracker: + def __init__(self) -> None: + self._pending_tasks: List[asyncio.Task] = [] + + def create_task(self, coro: Coroutine) -> asyncio.Task: + task = asyncio.create_task(coro) + self._pending_tasks.append(task) + return task + + def close(self) -> None: + try: + for task in self._pending_tasks: + if not task.done(): + task.cancel() + except Exception: + pass diff --git a/playwright/_impl/_page.py b/playwright/_impl/_page.py index ddb41aa12..318683348 100644 --- a/playwright/_impl/_page.py +++ b/playwright/_impl/_page.py @@ -55,6 +55,7 @@ from playwright._impl._frame import Frame from playwright._impl._har_router import HarRouter from playwright._impl._helper import ( + BackgroundTaskTracker, ColorScheme, DocumentLoadState, ForcedColors, @@ -151,6 +152,7 @@ def __init__( self._browser_context._timeout_settings ) self._video: Optional[Video] = None + self._background_task_tracker = BackgroundTaskTracker() self._opener = cast("Page", from_nullable_channel(initializer.get("opener"))) self._channel.on( @@ -192,7 +194,7 @@ def __init__( ) self._channel.on( "route", - lambda params: asyncio.create_task( + lambda params: self._background_task_tracker.create_task( self._on_route( from_channel(params["route"]), from_channel(params["request"]) ) @@ -209,11 +211,15 @@ def __init__( "worker", lambda params: self._on_worker(from_channel(params["worker"])) ) self._closed_or_crashed_future: asyncio.Future = asyncio.Future() + + def _on_close(_: Any) -> None: + self._background_task_tracker.close() + if not self._closed_or_crashed_future.done(): + self._closed_or_crashed_future.set_result(True) + self.on( Page.Events.Close, - lambda _: self._closed_or_crashed_future.set_result(True) - if not self._closed_or_crashed_future.done() - else None, + _on_close, ) self.on( Page.Events.Crash, @@ -246,7 +252,7 @@ async def _on_route(self, route: Route, request: Request) -> None: handled = await route_handler.handle(route, request) finally: if len(self._routes) == 0: - asyncio.create_task(self._disable_interception()) + await self._disable_interception() if handled: return await self._browser_context._on_route(route, request) diff --git a/tests/async/test_browsercontext_request_intercept.py b/tests/async/test_browsercontext_request_intercept.py index 763073df0..1d3e33945 100644 --- a/tests/async/test_browsercontext_request_intercept.py +++ b/tests/async/test_browsercontext_request_intercept.py @@ -174,3 +174,20 @@ async def test_should_give_access_to_the_intercepted_response_body( route.fulfill(response=response), eval_task, ) + + +async def test_should_cleanup_route_handlers_after_context_close( + context: BrowserContext, page: Page +) -> None: + async def handle(r: Route): + pass + + await context.route("**", handle) + try: + await page.goto("https://example.com", timeout=700) + except Exception: + pass + await context.close() + assert len(asyncio.all_tasks()) == 2 + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) diff --git a/tests/async/test_request_intercept.py b/tests/async/test_request_intercept.py index 39ccf3d3f..e90e964cc 100644 --- a/tests/async/test_request_intercept.py +++ b/tests/async/test_request_intercept.py @@ -17,7 +17,7 @@ from twisted.web import http -from playwright.async_api import Page, Route +from playwright.async_api import BrowserContext, Page, Route from tests.server import Server @@ -168,3 +168,37 @@ async def test_should_give_access_to_the_intercepted_response_body( route.fulfill(response=response), eval_task, ) + + +async def test_should_cleanup_route_handlers_after_page_close( + context: BrowserContext, page: Page +) -> None: + async def handle(r: Route): + pass + + await page.route("**", handle) + try: + await page.goto("https://example.com", timeout=700) + except Exception: + pass + await page.close() + assert len(asyncio.all_tasks()) == 2 + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) + + +async def test_should_cleanup_route_handlers_after_context_close( + context: BrowserContext, page: Page +) -> None: + async def handle(r: Route): + pass + + await page.route("**", handle) + try: + await page.goto("https://example.com", timeout=700) + except Exception: + pass + await context.close() + assert len(asyncio.all_tasks()) == 2 + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) diff --git a/tests/sync/test_browsercontext_request_intercept.py b/tests/sync/test_browsercontext_request_intercept.py index b136038ec..39510778f 100644 --- a/tests/sync/test_browsercontext_request_intercept.py +++ b/tests/sync/test_browsercontext_request_intercept.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import asyncio from pathlib import Path from twisted.web import http @@ -121,3 +122,17 @@ def handle_route(route: Route) -> None: assert request.uri.decode() == "/title.html" original = (assetdir / "title.html").read_text() assert response.text() == original + + +def test_should_cleanup_route_handlers_after_context_close( + context: BrowserContext, page: Page +) -> None: + context.route("**", lambda r: None) + try: + page.goto("https://example.com", timeout=700) + except Exception: + pass + context.close() + assert len(asyncio.all_tasks()) == 1 + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) diff --git a/tests/sync/test_request_intercept.py b/tests/sync/test_request_intercept.py index dc714e832..f82399606 100644 --- a/tests/sync/test_request_intercept.py +++ b/tests/sync/test_request_intercept.py @@ -12,11 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. +import asyncio from pathlib import Path from twisted.web import http -from playwright.sync_api import Page, Route +from playwright.sync_api import BrowserContext, Page, Route from tests.server import Server @@ -115,3 +116,31 @@ def handle_route(route: Route) -> None: assert request.uri.decode() == "/title.html" original = (assetdir / "title.html").read_text() assert response.text() == original + + +def test_should_cleanup_route_handlers_after_page_close( + context: BrowserContext, page: Page +) -> None: + page.route("**", lambda r: None) + try: + page.goto("https://example.com", timeout=700) + except Exception: + pass + page.close() + assert len(asyncio.all_tasks()) == 1 + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) + + +def test_should_cleanup_route_handlers_after_context_close( + context: BrowserContext, page: Page +) -> None: + page.route("**", lambda r: None) + try: + page.goto("https://example.com", timeout=700) + except Exception: + pass + context.close() + assert len(asyncio.all_tasks()) == 1 + for task in asyncio.all_tasks(): + assert "_on_route" not in str(task) From 8ec880976b8002b8ea17d99d658db85e76f42e16 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Wed, 6 Jul 2022 23:52:50 -0700 Subject: [PATCH 02/10] wait for task cancellations to actually occur --- playwright/_impl/_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/playwright/_impl/_helper.py b/playwright/_impl/_helper.py index f59ac8886..f71bb9213 100644 --- a/playwright/_impl/_helper.py +++ b/playwright/_impl/_helper.py @@ -378,5 +378,6 @@ def close(self) -> None: for task in self._pending_tasks: if not task.done(): task.cancel() + asyncio.gather(*self._pending_tasks, return_exceptions=True) except Exception: pass From 51de46bc8ff5949aaaf81fdcb9fc7aa2c8157b63 Mon Sep 17 00:00:00 2001 From: Ross Wollman Date: Thu, 7 Jul 2022 07:53:42 +0000 Subject: [PATCH 03/10] cleanup har tests --- tests/async/test_har.py | 7 +++++++ tests/sync/test_har.py | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/tests/async/test_har.py b/tests/async/test_har.py index cd1c871a6..fb820232d 100644 --- a/tests/async/test_har.py +++ b/tests/async/test_har.py @@ -503,6 +503,7 @@ async def test_should_round_trip_har_zip( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_round_trip_har_with_post_data( @@ -536,6 +537,7 @@ async def test_should_round_trip_har_with_post_data( assert await page_2.evaluate(fetch_function, "3") == "3" with pytest.raises(Exception): await page_2.evaluate(fetch_function, "4") + await context_2.close() async def test_should_disambiguate_by_header( @@ -578,6 +580,7 @@ async def test_should_disambiguate_by_header( assert await page_2.evaluate(fetch_function, "baz2") == "baz2" assert await page_2.evaluate(fetch_function, "baz3") == "baz3" assert await page_2.evaluate(fetch_function, "baz4") == "baz1" + await context_2.close() async def test_should_produce_extracted_zip( @@ -605,6 +608,7 @@ async def test_should_produce_extracted_zip( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_har_zip_for_context( @@ -627,6 +631,7 @@ async def test_should_update_har_zip_for_context( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_har_zip_for_page( @@ -649,6 +654,7 @@ async def test_should_update_har_zip_for_page( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() async def test_should_update_extracted_har_zip_for_page( @@ -675,3 +681,4 @@ async def test_should_update_extracted_har_zip_for_page( await expect(page_2.locator("body")).to_have_css( "background-color", "rgb(255, 192, 203)" ) + await context_2.close() diff --git a/tests/sync/test_har.py b/tests/sync/test_har.py index 81452c9de..f313ae257 100644 --- a/tests/sync/test_har.py +++ b/tests/sync/test_har.py @@ -471,6 +471,7 @@ def test_should_round_trip_har_with_post_data( assert page_2.evaluate(fetch_function, "3") == "3" with pytest.raises(Exception): page_2.evaluate(fetch_function, "4") + context_2.close() def test_should_disambiguate_by_header( @@ -512,6 +513,7 @@ def test_should_disambiguate_by_header( assert page_2.evaluate(fetch_function, "baz2") == "baz2" assert page_2.evaluate(fetch_function, "baz3") == "baz3" assert page_2.evaluate(fetch_function, "baz4") == "baz1" + context_2.close() def test_should_produce_extracted_zip( @@ -537,6 +539,7 @@ def test_should_produce_extracted_zip( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_har_zip_for_context( @@ -557,6 +560,7 @@ def test_should_update_har_zip_for_context( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_har_zip_for_page( @@ -577,6 +581,7 @@ def test_should_update_har_zip_for_page( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() def test_should_update_extracted_har_zip_for_page( @@ -601,3 +606,4 @@ def test_should_update_extracted_har_zip_for_page( page_2.goto(server.PREFIX + "/one-style.html") assert "hello, world!" in page_2.content() expect(page_2.locator("body")).to_have_css("background-color", "rgb(255, 192, 203)") + context_2.close() From c1b9d24bc445b5bf58a42de988bd4929ade68d31 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 01:16:10 -0700 Subject: [PATCH 04/10] don't try to set done on cancelled chain --- playwright/_impl/_network.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/playwright/_impl/_network.py b/playwright/_impl/_network.py index 5fc408042..ca142e5d7 100644 --- a/playwright/_impl/_network.py +++ b/playwright/_impl/_network.py @@ -223,7 +223,8 @@ def _report_handled(self, done: bool) -> None: chain = self._handling_future assert chain self._handling_future = None - chain.set_result(done) + if not chain.done(): + chain.set_result(done) def _check_not_handled(self) -> None: if not self._handling_future: From 614fd52b6e1ae331d9ae0f0846953612ff489e01 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 01:38:19 -0700 Subject: [PATCH 05/10] fix test_should_not_throw_when_continuing_after_page_is_closed and test_should_not_throw_when_continuing_while_page_is_closing --- playwright/_impl/_network.py | 4 +--- playwright/_impl/_page.py | 12 ++++-------- tests/async/test_request_intercept.py | 17 ----------------- tests/sync/test_request_intercept.py | 14 -------------- 4 files changed, 5 insertions(+), 42 deletions(-) diff --git a/playwright/_impl/_network.py b/playwright/_impl/_network.py index ca142e5d7..d2b5dd5b4 100644 --- a/playwright/_impl/_network.py +++ b/playwright/_impl/_network.py @@ -223,8 +223,7 @@ def _report_handled(self, done: bool) -> None: chain = self._handling_future assert chain self._handling_future = None - if not chain.done(): - chain.set_result(done) + chain.set_result(done) def _check_not_handled(self) -> None: if not self._handling_future: @@ -321,7 +320,6 @@ async def continue_( self._check_not_handled() self.request._apply_fallback_overrides(overrides) await self._internal_continue() - self._report_handled(True) def _internal_continue( self, is_internal: bool = False diff --git a/playwright/_impl/_page.py b/playwright/_impl/_page.py index 318683348..6ce5e0f7a 100644 --- a/playwright/_impl/_page.py +++ b/playwright/_impl/_page.py @@ -194,7 +194,7 @@ def __init__( ) self._channel.on( "route", - lambda params: self._background_task_tracker.create_task( + lambda params: self._browser_context._background_task_tracker.create_task( self._on_route( from_channel(params["route"]), from_channel(params["request"]) ) @@ -211,15 +211,11 @@ def __init__( "worker", lambda params: self._on_worker(from_channel(params["worker"])) ) self._closed_or_crashed_future: asyncio.Future = asyncio.Future() - - def _on_close(_: Any) -> None: - self._background_task_tracker.close() - if not self._closed_or_crashed_future.done(): - self._closed_or_crashed_future.set_result(True) - self.on( Page.Events.Close, - _on_close, + lambda _: self._closed_or_crashed_future.set_result(True) + if not self._closed_or_crashed_future.done() + else None, ) self.on( Page.Events.Crash, diff --git a/tests/async/test_request_intercept.py b/tests/async/test_request_intercept.py index e90e964cc..9d9bce8d1 100644 --- a/tests/async/test_request_intercept.py +++ b/tests/async/test_request_intercept.py @@ -170,23 +170,6 @@ async def test_should_give_access_to_the_intercepted_response_body( ) -async def test_should_cleanup_route_handlers_after_page_close( - context: BrowserContext, page: Page -) -> None: - async def handle(r: Route): - pass - - await page.route("**", handle) - try: - await page.goto("https://example.com", timeout=700) - except Exception: - pass - await page.close() - assert len(asyncio.all_tasks()) == 2 - for task in asyncio.all_tasks(): - assert "_on_route" not in str(task) - - async def test_should_cleanup_route_handlers_after_context_close( context: BrowserContext, page: Page ) -> None: diff --git a/tests/sync/test_request_intercept.py b/tests/sync/test_request_intercept.py index f82399606..522fd4ee0 100644 --- a/tests/sync/test_request_intercept.py +++ b/tests/sync/test_request_intercept.py @@ -118,20 +118,6 @@ def handle_route(route: Route) -> None: assert response.text() == original -def test_should_cleanup_route_handlers_after_page_close( - context: BrowserContext, page: Page -) -> None: - page.route("**", lambda r: None) - try: - page.goto("https://example.com", timeout=700) - except Exception: - pass - page.close() - assert len(asyncio.all_tasks()) == 1 - for task in asyncio.all_tasks(): - assert "_on_route" not in str(task) - - def test_should_cleanup_route_handlers_after_context_close( context: BrowserContext, page: Page ) -> None: From a429f9b2f89ccd441c801cfac1f8a7136b619508 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 01:46:11 -0700 Subject: [PATCH 06/10] remove done tasks; ignore background errors --- playwright/_impl/_browser_context.py | 5 ++++- playwright/_impl/_helper.py | 1 + playwright/_impl/_page.py | 5 ++++- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/playwright/_impl/_browser_context.py b/playwright/_impl/_browser_context.py index 94f9d1dee..cd226164e 100644 --- a/playwright/_impl/_browser_context.py +++ b/playwright/_impl/_browser_context.py @@ -195,7 +195,10 @@ async def _on_route(self, route: Route, request: Request) -> None: handled = await route_handler.handle(route, request) finally: if len(self._routes) == 0: - await self._disable_interception() + try: + await self._disable_interception() + except Exception: + pass if handled: return await route._internal_continue(is_internal=True) diff --git a/playwright/_impl/_helper.py b/playwright/_impl/_helper.py index f71bb9213..e3677495d 100644 --- a/playwright/_impl/_helper.py +++ b/playwright/_impl/_helper.py @@ -370,6 +370,7 @@ def __init__(self) -> None: def create_task(self, coro: Coroutine) -> asyncio.Task: task = asyncio.create_task(coro) + task.add_done_callback(lambda task: self._pending_tasks.remove(task)) self._pending_tasks.append(task) return task diff --git a/playwright/_impl/_page.py b/playwright/_impl/_page.py index 6ce5e0f7a..5128acbcb 100644 --- a/playwright/_impl/_page.py +++ b/playwright/_impl/_page.py @@ -248,7 +248,10 @@ async def _on_route(self, route: Route, request: Request) -> None: handled = await route_handler.handle(route, request) finally: if len(self._routes) == 0: - await self._disable_interception() + try: + await self._disable_interception() + except Exception: + pass if handled: return await self._browser_context._on_route(route, request) From 86e52bb4075979360b5a51addbe515e7f039333e Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 02:08:10 -0700 Subject: [PATCH 07/10] remove fragile assertions --- tests/async/test_browsercontext_request_intercept.py | 2 +- tests/async/test_request_intercept.py | 2 +- tests/sync/test_browsercontext_request_intercept.py | 2 +- tests/sync/test_request_intercept.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/async/test_browsercontext_request_intercept.py b/tests/async/test_browsercontext_request_intercept.py index 1d3e33945..e4ea30bef 100644 --- a/tests/async/test_browsercontext_request_intercept.py +++ b/tests/async/test_browsercontext_request_intercept.py @@ -188,6 +188,6 @@ async def handle(r: Route): except Exception: pass await context.close() - assert len(asyncio.all_tasks()) == 2 + for task in asyncio.all_tasks(): assert "_on_route" not in str(task) diff --git a/tests/async/test_request_intercept.py b/tests/async/test_request_intercept.py index 9d9bce8d1..30d8941c1 100644 --- a/tests/async/test_request_intercept.py +++ b/tests/async/test_request_intercept.py @@ -182,6 +182,6 @@ async def handle(r: Route): except Exception: pass await context.close() - assert len(asyncio.all_tasks()) == 2 + for task in asyncio.all_tasks(): assert "_on_route" not in str(task) diff --git a/tests/sync/test_browsercontext_request_intercept.py b/tests/sync/test_browsercontext_request_intercept.py index 39510778f..acc021659 100644 --- a/tests/sync/test_browsercontext_request_intercept.py +++ b/tests/sync/test_browsercontext_request_intercept.py @@ -133,6 +133,6 @@ def test_should_cleanup_route_handlers_after_context_close( except Exception: pass context.close() - assert len(asyncio.all_tasks()) == 1 + for task in asyncio.all_tasks(): assert "_on_route" not in str(task) diff --git a/tests/sync/test_request_intercept.py b/tests/sync/test_request_intercept.py index 522fd4ee0..ab90fc079 100644 --- a/tests/sync/test_request_intercept.py +++ b/tests/sync/test_request_intercept.py @@ -127,6 +127,6 @@ def test_should_cleanup_route_handlers_after_context_close( except Exception: pass context.close() - assert len(asyncio.all_tasks()) == 1 + for task in asyncio.all_tasks(): assert "_on_route" not in str(task) From 49b6b8e05922860e57f38e89ca97ab3dcffe13a2 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 08:46:41 -0700 Subject: [PATCH 08/10] fix missing report handled chain --- playwright/_impl/_network.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/playwright/_impl/_network.py b/playwright/_impl/_network.py index d2b5dd5b4..ca142e5d7 100644 --- a/playwright/_impl/_network.py +++ b/playwright/_impl/_network.py @@ -223,7 +223,8 @@ def _report_handled(self, done: bool) -> None: chain = self._handling_future assert chain self._handling_future = None - chain.set_result(done) + if not chain.done(): + chain.set_result(done) def _check_not_handled(self) -> None: if not self._handling_future: @@ -320,6 +321,7 @@ async def continue_( self._check_not_handled() self.request._apply_fallback_overrides(overrides) await self._internal_continue() + self._report_handled(True) def _internal_continue( self, is_internal: bool = False From 82776f291728b3b75dcc645803903349493c3777 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 08:50:53 -0700 Subject: [PATCH 09/10] remove unused gather --- playwright/_impl/_helper.py | 1 - 1 file changed, 1 deletion(-) diff --git a/playwright/_impl/_helper.py b/playwright/_impl/_helper.py index e3677495d..36aa1601f 100644 --- a/playwright/_impl/_helper.py +++ b/playwright/_impl/_helper.py @@ -379,6 +379,5 @@ def close(self) -> None: for task in self._pending_tasks: if not task.done(): task.cancel() - asyncio.gather(*self._pending_tasks, return_exceptions=True) except Exception: pass From a92eec07c329ca47ed42cfc16d77ca9dfc7b1419 Mon Sep 17 00:00:00 2001 From: "Ross A. Wollman" Date: Thu, 7 Jul 2022 09:43:21 -0700 Subject: [PATCH 10/10] we don't expect errors, so don't suppress them --- playwright/_impl/_helper.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/playwright/_impl/_helper.py b/playwright/_impl/_helper.py index 36aa1601f..70cbee8b7 100644 --- a/playwright/_impl/_helper.py +++ b/playwright/_impl/_helper.py @@ -375,9 +375,6 @@ def create_task(self, coro: Coroutine) -> asyncio.Task: return task def close(self) -> None: - try: - for task in self._pending_tasks: - if not task.done(): - task.cancel() - except Exception: - pass + for task in self._pending_tasks: + if not task.done(): + task.cancel()