Skip to content

Commit 4783148

Browse files
mxschmittmxschmitt
authored
fix: brush up exception messages to include apiName & call log (#2371)
1 parent 5a4779e commit 4783148

File tree

11 files changed

+86
-26
lines changed

11 files changed

+86
-26
lines changed

‎playwright/_impl/_connection.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
from pyee.asyncio import AsyncIOEventEmitter
3838

3939
import playwright
40-
from playwright._impl._errors import TargetClosedError
40+
from playwright._impl._errors import TargetClosedError, rewrite_error
4141
from playwright._impl._greenlets import EventGreenlet
4242
from playwright._impl._helper import Error, ParsedMessagePayload, parse_error
4343
from playwright._impl._transport import Transport
@@ -374,11 +374,12 @@ def dispatch(self, msg: ParsedMessagePayload) -> None:
374374
return
375375
error = msg.get("error")
376376
if error and not msg.get("result"):
377-
parsed_error = parse_error(error["error"]) # type: ignore
377+
parsed_error = parse_error(
378+
error["error"], format_call_log(msg.get("log")) # type: ignore
379+
)
378380
parsed_error._stack = "".join(
379381
traceback.format_list(callback.stack_trace)[-10:]
380382
)
381-
parsed_error._message += format_call_log(msg.get("log")) # type: ignore
382383
callback.future.set_exception(parsed_error)
383384
else:
384385
result = self._replace_guids_with_channels(msg.get("result"))
@@ -504,9 +505,12 @@ async def wrap_api_call(
504505
return await cb()
505506
task = asyncio.current_task(self._loop)
506507
st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack())
507-
self._api_zone.set(_extract_stack_trace_information_from_stack(st, is_internal))
508+
parsed_st = _extract_stack_trace_information_from_stack(st, is_internal)
509+
self._api_zone.set(parsed_st)
508510
try:
509511
return await cb()
512+
except Exception as error:
513+
raise rewrite_error(error, f"{parsed_st['apiName']}: {error}") from None
510514
finally:
511515
self._api_zone.set(None)
512516

@@ -517,9 +521,12 @@ def wrap_api_call_sync(
517521
return cb()
518522
task = asyncio.current_task(self._loop)
519523
st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack())
520-
self._api_zone.set(_extract_stack_trace_information_from_stack(st, is_internal))
524+
parsed_st = _extract_stack_trace_information_from_stack(st, is_internal)
525+
self._api_zone.set(parsed_st)
521526
try:
522527
return cb()
528+
except Exception as error:
529+
raise rewrite_error(error, f"{parsed_st['apiName']}: {error}") from None
523530
finally:
524531
self._api_zone.set(None)
525532

@@ -546,7 +553,7 @@ class ParsedStackTrace(TypedDict):
546553

547554
def _extract_stack_trace_information_from_stack(
548555
st: List[inspect.FrameInfo], is_internal: bool
549-
) -> Optional[ParsedStackTrace]:
556+
) -> ParsedStackTrace:
550557
playwright_module_path = str(Path(playwright.__file__).parents[0])
551558
last_internal_api_name = ""
552559
api_name = ""

‎playwright/_impl/_errors.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,3 +50,11 @@ class TimeoutError(Error):
5050
class TargetClosedError(Error):
5151
def __init__(self, message: str = None) -> None:
5252
super().__init__(message or "Target page, context or browser has been closed")
53+
54+
55+
def rewrite_error(error: Exception, message: str) -> Exception:
56+
rewritten_exc = type(error)(message)
57+
if isinstance(rewritten_exc, Error) and isinstance(error, Error):
58+
rewritten_exc._name = error.name
59+
rewritten_exc._stack = error.stack
60+
return rewritten_exc

‎playwright/_impl/_helper.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -210,26 +210,24 @@ def serialize_error(ex: Exception, tb: Optional[TracebackType]) -> ErrorPayload:
210210
)
211211

212212

213-
def parse_error(error: ErrorPayload) -> Error:
213+
def parse_error(error: ErrorPayload, log: Optional[str] = None) -> Error:
214214
base_error_class = Error
215215
if error.get("name") == "TimeoutError":
216216
base_error_class = TimeoutError
217217
if error.get("name") == "TargetClosedError":
218218
base_error_class = TargetClosedError
219-
exc = base_error_class(cast(str, patch_error_message(error.get("message"))))
219+
if not log:
220+
log = ""
221+
exc = base_error_class(patch_error_message(error["message"]) + log)
220222
exc._name = error["name"]
221223
exc._stack = error["stack"]
222224
return exc
223225

224226

225-
def patch_error_message(message: Optional[str]) -> Optional[str]:
226-
if message is None:
227-
return None
228-
227+
def patch_error_message(message: str) -> str:
229228
match = re.match(r"(\w+)(: expected .*)", message)
230229
if match:
231230
message = to_snake_case(match.group(1)) + match.group(2)
232-
assert message is not None
233231
message = message.replace(
234232
"Pass { acceptDownloads: true }", "Pass { accept_downloads: True }"
235233
)

‎tests/async/test_browsercontext.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ async def test_page_event_should_not_allow_device_scale_factor_with_null_viewpor
126126
await browser.new_context(no_viewport=True, device_scale_factor=1)
127127
assert (
128128
exc_info.value.message
129-
== '"deviceScaleFactor" option is not supported with null "viewport"'
129+
== 'Browser.new_context: "deviceScaleFactor" option is not supported with null "viewport"'
130130
)
131131

132132

@@ -137,7 +137,7 @@ async def test_page_event_should_not_allow_is_mobile_with_null_viewport(
137137
await browser.new_context(no_viewport=True, is_mobile=True)
138138
assert (
139139
exc_info.value.message
140-
== '"isMobile" option is not supported with null "viewport"'
140+
== 'Browser.new_context: "isMobile" option is not supported with null "viewport"'
141141
)
142142

143143

‎tests/async/test_frames.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ async def test_frame_element_throw_when_detached(
7171
except Error as e:
7272
error = e
7373
assert error
74-
assert error.message == "Frame has been detached."
74+
assert error.message == "Frame.frame_element: Frame has been detached."
7575

7676

7777
async def test_evaluate_throw_for_detached_frames(

‎tests/async/test_keyboard.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -424,15 +424,15 @@ async def testEnterKey(key: str, expectedKey: str, expectedCode: str) -> None:
424424
async def test_should_throw_unknown_keys(page: Page, server: Server) -> None:
425425
with pytest.raises(Error) as exc:
426426
await page.keyboard.press("NotARealKey")
427-
assert exc.value.message == 'Unknown key: "NotARealKey"'
427+
assert exc.value.message == 'Keyboard.press: Unknown key: "NotARealKey"'
428428

429429
with pytest.raises(Error) as exc:
430430
await page.keyboard.press("ё")
431-
assert exc.value.message == 'Unknown key: "ё"'
431+
assert exc.value.message == 'Keyboard.press: Unknown key: "ё"'
432432

433433
with pytest.raises(Error) as exc:
434434
await page.keyboard.press("😊")
435-
assert exc.value.message == 'Unknown key: "😊"'
435+
assert exc.value.message == 'Keyboard.press: Unknown key: "😊"'
436436

437437

438438
async def test_should_type_emoji(page: Page, server: Server) -> None:

‎tests/async/test_locators.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import os
1616
import re
17+
import traceback
1718
from typing import Callable
1819
from urllib.parse import urlparse
1920

@@ -1083,3 +1084,19 @@ async def test_locator_all_should_work(page: Page) -> None:
10831084
for p in await page.locator("p").all():
10841085
texts.append(await p.text_content())
10851086
assert texts == ["A", "B", "C"]
1087+
1088+
1089+
async def test_locator_click_timeout_error_should_contain_call_log(page: Page) -> None:
1090+
with pytest.raises(Error) as exc_info:
1091+
await page.get_by_role("button", name="Hello Python").click(timeout=42)
1092+
formatted_exception = "".join(
1093+
traceback.format_exception(type(exc_info.value), value=exc_info.value, tb=None)
1094+
)
1095+
assert "Locator.click: Timeout 42ms exceeded." in formatted_exception
1096+
assert (
1097+
'waiting for get_by_role("button", name="Hello Python")' in formatted_exception
1098+
)
1099+
assert (
1100+
"During handling of the above exception, another exception occurred"
1101+
not in formatted_exception
1102+
)

‎tests/async/test_network.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -830,7 +830,10 @@ async def test_set_extra_http_headers_should_throw_for_non_string_header_values(
830830
except Error as exc:
831831
error = exc
832832
assert error
833-
assert error.message == "headers[0].value: expected string, got number"
833+
assert (
834+
error.message
835+
== "Page.set_extra_http_headers: headers[0].value: expected string, got number"
836+
)
834837

835838

836839
async def test_response_server_addr(page: Page, server: Server) -> None:

‎tests/async/test_queryselector.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ async def test_selectors_register_should_handle_errors(
182182
await selectors.register("$", dummy_selector_engine_script)
183183
assert (
184184
exc.value.message
185-
== "Selector engine name may only contain [a-zA-Z0-9_] characters"
185+
== "Selectors.register: Selector engine name may only contain [a-zA-Z0-9_] characters"
186186
)
187187

188188
# Selector names are case-sensitive.
@@ -195,11 +195,16 @@ async def test_selectors_register_should_handle_errors(
195195

196196
with pytest.raises(Error) as exc:
197197
await selectors.register("dummy", dummy_selector_engine_script)
198-
assert exc.value.message == '"dummy" selector engine has been already registered'
198+
assert (
199+
exc.value.message
200+
== 'Selectors.register: "dummy" selector engine has been already registered'
201+
)
199202

200203
with pytest.raises(Error) as exc:
201204
await selectors.register("css", dummy_selector_engine_script)
202-
assert exc.value.message == '"css" is a predefined selector engine'
205+
assert (
206+
exc.value.message == 'Selectors.register: "css" is a predefined selector engine'
207+
)
203208

204209

205210
async def test_should_work_with_layout_selectors(page: Page) -> None:

‎tests/sync/test_locators.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import os
1616
import re
17+
import traceback
1718
from typing import Callable
1819
from urllib.parse import urlparse
1920

@@ -937,3 +938,19 @@ def test_locator_all_should_work(page: Page) -> None:
937938
for p in page.locator("p").all():
938939
texts.append(p.text_content())
939940
assert texts == ["A", "B", "C"]
941+
942+
943+
def test_locator_click_timeout_error_should_contain_call_log(page: Page) -> None:
944+
with pytest.raises(Error) as exc_info:
945+
page.get_by_role("button", name="Hello Python").click(timeout=42)
946+
formatted_exception = "".join(
947+
traceback.format_exception(type(exc_info.value), value=exc_info.value, tb=None)
948+
)
949+
assert "Locator.click: Timeout 42ms exceeded." in formatted_exception
950+
assert (
951+
'waiting for get_by_role("button", name="Hello Python")' in formatted_exception
952+
)
953+
assert (
954+
"During handling of the above exception, another exception occurred"
955+
not in formatted_exception
956+
)

‎tests/sync/test_queryselector.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def test_selectors_register_should_handle_errors(
156156
selectors.register("$", dummy_selector_engine_script)
157157
assert (
158158
exc.value.message
159-
== "Selector engine name may only contain [a-zA-Z0-9_] characters"
159+
== "Selectors.register: Selector engine name may only contain [a-zA-Z0-9_] characters"
160160
)
161161

162162
# Selector names are case-sensitive.
@@ -165,11 +165,16 @@ def test_selectors_register_should_handle_errors(
165165

166166
with pytest.raises(Error) as exc:
167167
selectors.register("dummy", dummy_selector_engine_script)
168-
assert exc.value.message == '"dummy" selector engine has been already registered'
168+
assert (
169+
exc.value.message
170+
== 'Selectors.register: "dummy" selector engine has been already registered'
171+
)
169172

170173
with pytest.raises(Error) as exc:
171174
selectors.register("css", dummy_selector_engine_script)
172-
assert exc.value.message == '"css" is a predefined selector engine'
175+
assert (
176+
exc.value.message == 'Selectors.register: "css" is a predefined selector engine'
177+
)
173178

174179

175180
def test_should_work_with_layout_selectors(page: Page) -> None:

0 commit comments

Comments
 (0)