From 2018e49039f0ecf73aa82b9a7780db1cd54bb75b Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 15:20:56 +0800 Subject: [PATCH 1/5] Speed up connection by not asking for context when inspecting stack --- playwright/_impl/_connection.py | 7 +++++-- tests/async/test_asyncio.py | 13 +++++++++++++ tests/sync/test_sync.py | 13 +++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 027daf69d..3dd5df633 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -365,7 +365,6 @@ def _send_message_to_server( if ( self._tracing_count > 0 and frames - and frames and object._guid != "localUtils" ): self.local_utils.add_stack_to_tracing_no_reply(id, frames) @@ -519,7 +518,11 @@ async def wrap_api_call( if self._api_zone.get(): return await cb() task = asyncio.current_task(self._loop) - st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack()) + st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", None) + + if st == None: + st = inspect.stack(0) + parsed_st = _extract_stack_trace_information_from_stack(st, is_internal) self._api_zone.set(parsed_st) try: diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 33edc71ce..e7ed72d9e 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -87,3 +87,16 @@ async def raise_exception() -> None: assert "Something went wrong" in str(exc_info.value.exceptions[0]) assert isinstance(exc_info.value.exceptions[0], ValueError) assert await page.evaluate("() => 11 * 11") == 121 + +async def test_should_return_proper_api_name_on_error(page: Page) -> None: + try: + await page.evaluate("does_not_exist") + + assert False, "Accessing undefined JavaScript variable should have thrown exception" + except Exception as error: + print(':-:' + error.message + ':-:') + assert error.message == """Page.evaluate: ReferenceError: does_not_exist is not defined + at eval (eval at evaluate (:234:30), :1:1) + at eval () + at UtilityScript.evaluate (:234:30) + at UtilityScript. (:1:44)""" \ No newline at end of file diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index 64eace1e9..c673efad0 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -346,3 +346,16 @@ def test_call_sync_method_after_playwright_close_with_own_loop( p.start() p.join() assert p.exitcode == 0 + +def test_should_return_proper_api_name_on_error(page: Page) -> None: + try: + page.evaluate("does_not_exist") + + assert False, "Accessing undefined JavaScript variable should have thrown exception" + except Exception as error: + print(':-:' + error.message + ':-:') + assert error.message == """Page.evaluate: ReferenceError: does_not_exist is not defined + at eval (eval at evaluate (:234:30), :1:1) + at eval () + at UtilityScript.evaluate (:234:30) + at UtilityScript. (:1:44)""" \ No newline at end of file From 5a92f4f5713c985daa0f9cd3ea46a9431208bee4 Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 15:30:56 +0800 Subject: [PATCH 2/5] Formatting changes from the pre-commit hook --- playwright/_impl/_connection.py | 13 ++++--------- tests/async/test_asyncio.py | 14 ++++++++++---- tests/sync/test_sync.py | 14 ++++++++++---- 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 3dd5df633..647414295 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -362,11 +362,7 @@ def _send_message_to_server( "params": self._replace_channels_with_guids(params), "metadata": metadata, } - if ( - self._tracing_count > 0 - and frames - and object._guid != "localUtils" - ): + if self._tracing_count > 0 and frames and object._guid != "localUtils": self.local_utils.add_stack_to_tracing_no_reply(id, frames) self._transport.send(message) @@ -518,10 +514,9 @@ async def wrap_api_call( if self._api_zone.get(): return await cb() task = asyncio.current_task(self._loop) - st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", None) - - if st == None: - st = inspect.stack(0) + st: List[inspect.FrameInfo] = getattr( + task, "__pw_stack__", None + ) or inspect.stack(0) parsed_st = _extract_stack_trace_information_from_stack(st, is_internal) self._api_zone.set(parsed_st) diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index e7ed72d9e..1e4857ad5 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -88,15 +88,21 @@ async def raise_exception() -> None: assert isinstance(exc_info.value.exceptions[0], ValueError) assert await page.evaluate("() => 11 * 11") == 121 + async def test_should_return_proper_api_name_on_error(page: Page) -> None: try: await page.evaluate("does_not_exist") - assert False, "Accessing undefined JavaScript variable should have thrown exception" + assert ( + False + ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - print(':-:' + error.message + ':-:') - assert error.message == """Page.evaluate: ReferenceError: does_not_exist is not defined + print(":-:" + error.message + ":-:") + assert ( + error.message + == """Page.evaluate: ReferenceError: does_not_exist is not defined at eval (eval at evaluate (:234:30), :1:1) at eval () at UtilityScript.evaluate (:234:30) - at UtilityScript. (:1:44)""" \ No newline at end of file + at UtilityScript. (:1:44)""" + ) diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index c673efad0..d0f630f67 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -347,15 +347,21 @@ def test_call_sync_method_after_playwright_close_with_own_loop( p.join() assert p.exitcode == 0 + def test_should_return_proper_api_name_on_error(page: Page) -> None: try: page.evaluate("does_not_exist") - assert False, "Accessing undefined JavaScript variable should have thrown exception" + assert ( + False + ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - print(':-:' + error.message + ':-:') - assert error.message == """Page.evaluate: ReferenceError: does_not_exist is not defined + print(":-:" + error.message + ":-:") + assert ( + error.message + == """Page.evaluate: ReferenceError: does_not_exist is not defined at eval (eval at evaluate (:234:30), :1:1) at eval () at UtilityScript.evaluate (:234:30) - at UtilityScript. (:1:44)""" \ No newline at end of file + at UtilityScript. (:1:44)""" + ) From e973932c0099f5e65d462d6619edfc17d1e87cd5 Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 16:06:56 +0800 Subject: [PATCH 3/5] Also use inspect.stack(0) for the sync version of _connect --- playwright/_impl/_connection.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/playwright/_impl/_connection.py b/playwright/_impl/_connection.py index 647414295..2d1dad933 100644 --- a/playwright/_impl/_connection.py +++ b/playwright/_impl/_connection.py @@ -533,7 +533,9 @@ def wrap_api_call_sync( if self._api_zone.get(): return cb() task = asyncio.current_task(self._loop) - st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack()) + st: List[inspect.FrameInfo] = getattr( + task, "__pw_stack__", None + ) or inspect.stack(0) parsed_st = _extract_stack_trace_information_from_stack(st, is_internal) self._api_zone.set(parsed_st) try: From ebddce42e287f5142948cef37e02fcb093140a57 Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 17:24:16 +0800 Subject: [PATCH 4/5] Fix pre-commit warnings --- tests/async/test_asyncio.py | 3 +-- tests/sync/test_sync.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 1e4857ad5..5d878157d 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -97,9 +97,8 @@ async def test_should_return_proper_api_name_on_error(page: Page) -> None: False ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - print(":-:" + error.message + ":-:") assert ( - error.message + str(error) == """Page.evaluate: ReferenceError: does_not_exist is not defined at eval (eval at evaluate (:234:30), :1:1) at eval () diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index d0f630f67..de9514634 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -356,9 +356,8 @@ def test_should_return_proper_api_name_on_error(page: Page) -> None: False ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - print(":-:" + error.message + ":-:") assert ( - error.message + str(error) == """Page.evaluate: ReferenceError: does_not_exist is not defined at eval (eval at evaluate (:234:30), :1:1) at eval () From 5ac20ef84f3f6577ce19de5ed734da8dc701a84f Mon Sep 17 00:00:00 2001 From: Eli Black Date: Tue, 29 Apr 2025 23:30:30 +0800 Subject: [PATCH 5/5] Simplify test cases, so that they pass on all browsers --- tests/async/test_asyncio.py | 10 ++-------- tests/sync/test_sync.py | 10 ++-------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/tests/async/test_asyncio.py b/tests/async/test_asyncio.py index 5d878157d..971c65473 100644 --- a/tests/async/test_asyncio.py +++ b/tests/async/test_asyncio.py @@ -97,11 +97,5 @@ async def test_should_return_proper_api_name_on_error(page: Page) -> None: False ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - assert ( - str(error) - == """Page.evaluate: ReferenceError: does_not_exist is not defined - at eval (eval at evaluate (:234:30), :1:1) - at eval () - at UtilityScript.evaluate (:234:30) - at UtilityScript. (:1:44)""" - ) + # Each browser returns slightly different error messages, but they should all start with "Page.evaluate:", because that was the Playwright method where the error originated + assert str(error).startswith("Page.evaluate:") diff --git a/tests/sync/test_sync.py b/tests/sync/test_sync.py index de9514634..92d40c19a 100644 --- a/tests/sync/test_sync.py +++ b/tests/sync/test_sync.py @@ -356,11 +356,5 @@ def test_should_return_proper_api_name_on_error(page: Page) -> None: False ), "Accessing undefined JavaScript variable should have thrown exception" except Exception as error: - assert ( - str(error) - == """Page.evaluate: ReferenceError: does_not_exist is not defined - at eval (eval at evaluate (:234:30), :1:1) - at eval () - at UtilityScript.evaluate (:234:30) - at UtilityScript. (:1:44)""" - ) + # Each browser returns slightly different error messages, but they should all start with "Page.evaluate:", because that was the Playwright method where the error originated + assert str(error).startswith("Page.evaluate:")