fix: brush up exception messages to include apiName & call log

playwright/_impl/_connection.py

@@ -37,7 +37,7 @@
 from pyee.asyncio import AsyncIOEventEmitter
 
 import playwright
-from playwright._impl._errors import TargetClosedError
+from playwright._impl._errors import TargetClosedError, rewrite_error
 from playwright._impl._greenlets import EventGreenlet
 from playwright._impl._helper import Error, ParsedMessagePayload, parse_error
 from playwright._impl._transport import Transport
@@ -374,11 +374,12 @@ def dispatch(self, msg: ParsedMessagePayload) -> None:
                 return
             error = msg.get("error")
             if error and not msg.get("result"):
-                parsed_error = parse_error(error["error"])  # type: ignore
+                parsed_error = parse_error(
+                    error["error"], format_call_log(msg.get("log"))  # type: ignore
+                )
                 parsed_error._stack = "".join(
                     traceback.format_list(callback.stack_trace)[-10:]
                 )
-                parsed_error._message += format_call_log(msg.get("log"))  # type: ignore
                 callback.future.set_exception(parsed_error)
             else:
                 result = self._replace_guids_with_channels(msg.get("result"))
@@ -504,9 +505,12 @@ async def wrap_api_call(
             return await cb()
         task = asyncio.current_task(self._loop)
         st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack())
-        self._api_zone.set(_extract_stack_trace_information_from_stack(st, is_internal))
+        parsed_st = _extract_stack_trace_information_from_stack(st, is_internal)
+        self._api_zone.set(parsed_st)
         try:
             return await cb()
+        except Exception as error:
+            raise rewrite_error(error, f"{parsed_st['apiName']}: {error}") from None
         finally:
             self._api_zone.set(None)
 
@@ -517,9 +521,12 @@ def wrap_api_call_sync(
             return cb()
         task = asyncio.current_task(self._loop)
         st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack())
-        self._api_zone.set(_extract_stack_trace_information_from_stack(st, is_internal))
+        parsed_st = _extract_stack_trace_information_from_stack(st, is_internal)
+        self._api_zone.set(parsed_st)
         try:
             return cb()
+        except Exception as error:
+            raise rewrite_error(error, f"{parsed_st['apiName']}: {error}") from None
         finally:
             self._api_zone.set(None)
 
@@ -546,7 +553,7 @@ class ParsedStackTrace(TypedDict):
 
 def _extract_stack_trace_information_from_stack(
     st: List[inspect.FrameInfo], is_internal: bool
-) -> Optional[ParsedStackTrace]:
+) -> ParsedStackTrace:
     playwright_module_path = str(Path(playwright.__file__).parents[0])
     last_internal_api_name = ""
     api_name = ""

playwright/_impl/_errors.py

@@ -50,3 +50,11 @@ class TimeoutError(Error):
 class TargetClosedError(Error):
     def __init__(self, message: str = None) -> None:
         super().__init__(message or "Target page, context or browser has been closed")
+
+
+def rewrite_error(error: Exception, message: str) -> Exception:
+    rewritten_exc = type(error)(message)
+    if isinstance(rewritten_exc, Error) and isinstance(error, Error):
+        rewritten_exc._name = error.name
+        rewritten_exc._stack = error.stack
+    return rewritten_exc

playwright/_impl/_helper.py

@@ -210,26 +210,24 @@ def serialize_error(ex: Exception, tb: Optional[TracebackType]) -> ErrorPayload:
     )
 
 
-def parse_error(error: ErrorPayload) -> Error:
+def parse_error(error: ErrorPayload, log: Optional[str] = None) -> Error:
     base_error_class = Error
     if error.get("name") == "TimeoutError":
         base_error_class = TimeoutError
     if error.get("name") == "TargetClosedError":
         base_error_class = TargetClosedError
-    exc = base_error_class(cast(str, patch_error_message(error.get("message"))))
+    if not log:
+        log = ""
+    exc = base_error_class(patch_error_message(error["message"]) + log)
     exc._name = error["name"]
     exc._stack = error["stack"]
     return exc
 
 
-def patch_error_message(message: Optional[str]) -> Optional[str]:
-    if message is None:
-        return None
-
+def patch_error_message(message: str) -> str:
     match = re.match(r"(\w+)(: expected .*)", message)
     if match:
         message = to_snake_case(match.group(1)) + match.group(2)
-    assert message is not None
     message = message.replace(
         "Pass { acceptDownloads: true }", "Pass { accept_downloads: True }"
     )

tests/async/test_browsercontext.py

@@ -126,7 +126,7 @@ async def test_page_event_should_not_allow_device_scale_factor_with_null_viewpor
         await browser.new_context(no_viewport=True, device_scale_factor=1)
     assert (
         exc_info.value.message
-        == '"deviceScaleFactor" option is not supported with null "viewport"'
+        == 'Browser.new_context: "deviceScaleFactor" option is not supported with null "viewport"'
     )
 
 
@@ -137,7 +137,7 @@ async def test_page_event_should_not_allow_is_mobile_with_null_viewport(
         await browser.new_context(no_viewport=True, is_mobile=True)
     assert (
         exc_info.value.message
-        == '"isMobile" option is not supported with null "viewport"'
+        == 'Browser.new_context: "isMobile" option is not supported with null "viewport"'
     )
 
 

tests/async/test_frames.py

@@ -71,7 +71,7 @@ async def test_frame_element_throw_when_detached(
     except Error as e:
         error = e
     assert error
-    assert error.message == "Frame has been detached."
+    assert error.message == "Frame.frame_element: Frame has been detached."
 
 
 async def test_evaluate_throw_for_detached_frames(

tests/async/test_keyboard.py

@@ -424,15 +424,15 @@ async def testEnterKey(key: str, expectedKey: str, expectedCode: str) -> None:
 async def test_should_throw_unknown_keys(page: Page, server: Server) -> None:
     with pytest.raises(Error) as exc:
         await page.keyboard.press("NotARealKey")
-    assert exc.value.message == 'Unknown key: "NotARealKey"'
+    assert exc.value.message == 'Keyboard.press: Unknown key: "NotARealKey"'
 
     with pytest.raises(Error) as exc:
         await page.keyboard.press("ё")
-    assert exc.value.message == 'Unknown key: "ё"'
+    assert exc.value.message == 'Keyboard.press: Unknown key: "ё"'
 
     with pytest.raises(Error) as exc:
         await page.keyboard.press("😊")
-    assert exc.value.message == 'Unknown key: "😊"'
+    assert exc.value.message == 'Keyboard.press: Unknown key: "😊"'
 
 
 async def test_should_type_emoji(page: Page, server: Server) -> None:

tests/async/test_locators.py

@@ -14,6 +14,7 @@
 
 import os
 import re
+import traceback
 from typing import Callable
 from urllib.parse import urlparse
 
@@ -1083,3 +1084,19 @@ async def test_locator_all_should_work(page: Page) -> None:
     for p in await page.locator("p").all():
         texts.append(await p.text_content())
     assert texts == ["A", "B", "C"]
+
+
+async def test_locator_click_timeout_error_should_contain_call_log(page: Page) -> None:
+    with pytest.raises(Error) as exc_info:
+        await page.get_by_role("button", name="Hello Python").click(timeout=42)
+    formatted_exception = "".join(
+        traceback.format_exception(type(exc_info.value), value=exc_info.value, tb=None)
+    )
+    assert "Locator.click: Timeout 42ms exceeded." in formatted_exception
+    assert (
+        'waiting for get_by_role("button", name="Hello Python")' in formatted_exception
+    )
+    assert (
+        "During handling of the above exception, another exception occurred"
+        not in formatted_exception
+    )

tests/async/test_network.py

@@ -830,7 +830,10 @@ async def test_set_extra_http_headers_should_throw_for_non_string_header_values(
     except Error as exc:
         error = exc
     assert error
-    assert error.message == "headers[0].value: expected string, got number"
+    assert (
+        error.message
+        == "Page.set_extra_http_headers: headers[0].value: expected string, got number"
+    )
 
 
 async def test_response_server_addr(page: Page, server: Server) -> None:

tests/async/test_queryselector.py

@@ -182,7 +182,7 @@ async def test_selectors_register_should_handle_errors(
         await selectors.register("$", dummy_selector_engine_script)
     assert (
         exc.value.message
-        == "Selector engine name may only contain [a-zA-Z0-9_] characters"
+        == "Selectors.register: Selector engine name may only contain [a-zA-Z0-9_] characters"
     )
 
     # Selector names are case-sensitive.
@@ -195,11 +195,16 @@ async def test_selectors_register_should_handle_errors(
 
     with pytest.raises(Error) as exc:
         await selectors.register("dummy", dummy_selector_engine_script)
-    assert exc.value.message == '"dummy" selector engine has been already registered'
+    assert (
+        exc.value.message
+        == 'Selectors.register: "dummy" selector engine has been already registered'
+    )
 
     with pytest.raises(Error) as exc:
         await selectors.register("css", dummy_selector_engine_script)
-    assert exc.value.message == '"css" is a predefined selector engine'
+    assert (
+        exc.value.message == 'Selectors.register: "css" is a predefined selector engine'
+    )
 
 
 async def test_should_work_with_layout_selectors(page: Page) -> None:

tests/sync/test_locators.py

@@ -14,6 +14,7 @@
 
 import os
 import re
+import traceback
 from typing import Callable
 from urllib.parse import urlparse
 
@@ -937,3 +938,19 @@ def test_locator_all_should_work(page: Page) -> None:
     for p in page.locator("p").all():
         texts.append(p.text_content())
     assert texts == ["A", "B", "C"]
+
+
+def test_locator_click_timeout_error_should_contain_call_log(page: Page) -> None:
+    with pytest.raises(Error) as exc_info:
+        page.get_by_role("button", name="Hello Python").click(timeout=42)
+    formatted_exception = "".join(
+        traceback.format_exception(type(exc_info.value), value=exc_info.value, tb=None)
+    )
+    assert "Locator.click: Timeout 42ms exceeded." in formatted_exception
+    assert (
+        'waiting for get_by_role("button", name="Hello Python")' in formatted_exception
+    )
+    assert (
+        "During handling of the above exception, another exception occurred"
+        not in formatted_exception
+    )

tests/sync/test_queryselector.py

@@ -156,7 +156,7 @@ def test_selectors_register_should_handle_errors(
         selectors.register("$", dummy_selector_engine_script)
     assert (
         exc.value.message
-        == "Selector engine name may only contain [a-zA-Z0-9_] characters"
+        == "Selectors.register: Selector engine name may only contain [a-zA-Z0-9_] characters"
     )
 
     # Selector names are case-sensitive.
@@ -165,11 +165,16 @@ def test_selectors_register_should_handle_errors(
 
     with pytest.raises(Error) as exc:
         selectors.register("dummy", dummy_selector_engine_script)
-    assert exc.value.message == '"dummy" selector engine has been already registered'
+    assert (
+        exc.value.message
+        == 'Selectors.register: "dummy" selector engine has been already registered'
+    )
 
     with pytest.raises(Error) as exc:
         selectors.register("css", dummy_selector_engine_script)
-    assert exc.value.message == '"css" is a predefined selector engine'
+    assert (
+        exc.value.message == 'Selectors.register: "css" is a predefined selector engine'
+    )
 
 
 def test_should_work_with_layout_selectors(page: Page) -> None: