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_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(exc_info.value)  # type: ignore
+    )
+    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_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(exc_info.value)  # type: ignore
+    )
+    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
+    )