Skip to content

Commit 8820f30

Browse files
mxschmittmxschmitt
authored
fix(tracing): apiName calculation for route handlers (microsoft#1409)
test: clean up tracing tests
1 parent 5b3cb55 commit 8820f30

File tree

5 files changed

+78
-30
lines changed

5 files changed

+78
-30
lines changed

‎playwright/_impl/_connection.py‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ async def send_return_as_dict(self, method: str, params: Dict = None) -> Any:
5050
)
5151

5252
def send_no_reply(self, method: str, params: Dict = None) -> None:
53-
self._connection.wrap_api_call(
53+
self._connection.wrap_api_call_sync(
5454
lambda: self._connection._send_message_to_server(
5555
self._guid, method, {} if params is None else params
5656
)
@@ -355,26 +355,35 @@ def _replace_guids_with_channels(self, payload: Any) -> Any:
355355
return result
356356
return payload
357357

358-
def wrap_api_call(self, cb: Callable[[], Any], is_internal: bool = False) -> Any:
358+
async def wrap_api_call(
359+
self, cb: Callable[[], Any], is_internal: bool = False
360+
) -> Any:
359361
if self._api_zone.get():
360-
return cb()
362+
return await cb()
361363
task = asyncio.current_task(self._loop)
362364
st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack())
363365
metadata = _extract_metadata_from_stack(st, is_internal)
364366
if metadata:
365367
self._api_zone.set(metadata)
366-
result = cb()
367-
368-
async def _() -> None:
369-
try:
370-
return await result
371-
finally:
372-
self._api_zone.set(None)
368+
try:
369+
return await cb()
370+
finally:
371+
self._api_zone.set(None)
373372

374-
if asyncio.iscoroutine(result):
375-
return _()
376-
self._api_zone.set(None)
377-
return result
373+
def wrap_api_call_sync(
374+
self, cb: Callable[[], Any], is_internal: bool = False
375+
) -> Any:
376+
if self._api_zone.get():
377+
return cb()
378+
task = asyncio.current_task(self._loop)
379+
st: List[inspect.FrameInfo] = getattr(task, "__pw_stack__", inspect.stack())
380+
metadata = _extract_metadata_from_stack(st, is_internal)
381+
if metadata:
382+
self._api_zone.set(metadata)
383+
try:
384+
return cb()
385+
finally:
386+
self._api_zone.set(None)
378387

379388

380389
def from_channel(channel: Channel) -> Any:
@@ -388,6 +397,12 @@ def from_nullable_channel(channel: Optional[Channel]) -> Optional[Any]:
388397
def _extract_metadata_from_stack(
389398
st: List[inspect.FrameInfo], is_internal: bool
390399
) -> Optional[Dict]:
400+
if is_internal:
401+
return {
402+
"apiName": "",
403+
"stack": [],
404+
"internal": True,
405+
}
391406
playwright_module_path = str(Path(playwright.__file__).parents[0])
392407
last_internal_api_name = ""
393408
api_name = ""
@@ -419,6 +434,5 @@ def _extract_metadata_from_stack(
419434
return {
420435
"apiName": api_name,
421436
"stack": stack,
422-
"isInternal": is_internal,
423437
}
424438
return None

‎playwright/_impl/_network.py‎

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

1515
import asyncio
1616
import base64
17+
import inspect
1718
import json
1819
import mimetypes
1920
from collections import defaultdict
@@ -343,11 +344,14 @@ async def continue_route() -> None:
343344
params["headers"] = serialize_headers(params["headers"])
344345
if post_data_for_wire is not None:
345346
params["postData"] = post_data_for_wire
346-
await self._race_with_page_close(
347-
self._channel.send(
348-
"continue",
349-
params,
350-
)
347+
await self._connection.wrap_api_call(
348+
lambda: self._race_with_page_close(
349+
self._channel.send(
350+
"continue",
351+
params,
352+
)
353+
),
354+
is_internal,
351355
)
352356
except Exception as e:
353357
if not is_internal:
@@ -369,6 +373,14 @@ async def _race_with_page_close(self, future: Coroutine) -> None:
369373
# Note that page could be missing when routing popup's initial request that
370374
# does not have a Page initialized just yet.
371375
fut = asyncio.create_task(future)
376+
# Rewrite the user's stack to the new task which runs in the background.
377+
setattr(
378+
fut,
379+
"__pw_stack__",
380+
getattr(
381+
asyncio.current_task(self._loop), "__pw_stack__", inspect.stack()
382+
),
383+
)
372384
await asyncio.wait(
373385
[fut, page._closed_or_crashed_future],
374386
return_when=asyncio.FIRST_COMPLETED,

‎playwright/_impl/_wait_helper.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def _wait_for_event_info_before(self, wait_id: str, event: str) -> None:
4848
)
4949

5050
def _wait_for_event_info_after(self, wait_id: str, error: Exception = None) -> None:
51-
self._channel._connection.wrap_api_call(
51+
self._channel._connection.wrap_api_call_sync(
5252
lambda: self._channel.send_no_reply(
5353
"waitForEventInfo",
5454
{
@@ -127,7 +127,7 @@ def result(self) -> asyncio.Future:
127127
def log(self, message: str) -> None:
128128
self._logs.append(message)
129129
try:
130-
self._channel._connection.wrap_api_call(
130+
self._channel._connection.wrap_api_call_sync(
131131
lambda: self._channel.send_no_reply(
132132
"waitForEventInfo",
133133
{

‎tests/async/test_tracing.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,13 @@ async def test_should_collect_trace_with_resources_but_no_js(
8989
await page.mouse.dblclick(30, 30)
9090
await page.keyboard.insert_text("abc")
9191
await page.wait_for_timeout(2000) # Give it some time to produce screenshots.
92-
await page.route("**/empty.html", lambda route: route.continue_())
92+
await page.route(
93+
"**/empty.html", lambda route: route.continue_()
94+
) # should produce a route.continue_ entry.
9395
await page.goto(server.EMPTY_PAGE)
96+
await page.goto(
97+
server.PREFIX + "/one-style.html"
98+
) # should not produce a route.continue_ entry since we continue all routes if no match.
9499
await page.close()
95100
trace_file_path = tmpdir / "trace.zip"
96101
await context.tracing.stop(path=trace_file_path)
@@ -107,8 +112,8 @@ async def test_should_collect_trace_with_resources_but_no_js(
107112
"Page.wait_for_timeout",
108113
"Page.route",
109114
"Page.goto",
110-
# FIXME: https://github.com/microsoft/playwright-python/issues/1397
111-
"Channel.send",
115+
"Route.continue_",
116+
"Page.goto",
112117
"Page.close",
113118
"Tracing.stop",
114119
]
@@ -215,7 +220,13 @@ def parse_trace(path: Path) -> Tuple[Dict[str, bytes], List[Any]]:
215220

216221
def get_actions(events: List[Any]) -> List[str]:
217222
action_events = sorted(
218-
list(filter(lambda e: e["type"] == "action", events)),
223+
list(
224+
filter(
225+
lambda e: e["type"] == "action"
226+
and e["metadata"].get("internal", False) is False,
227+
events,
228+
)
229+
),
219230
key=lambda e: e["metadata"]["startTime"],
220231
)
221232
return [e["metadata"]["apiName"] for e in action_events]

‎tests/sync/test_tracing.py‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,13 @@ def test_should_collect_trace_with_resources_but_no_js(
8989
page.mouse.dblclick(30, 30)
9090
page.keyboard.insert_text("abc")
9191
page.wait_for_timeout(2000) # Give it some time to produce screenshots.
92-
page.route("**/empty.html", lambda route: route.continue_())
92+
page.route(
93+
"**/empty.html", lambda route: route.continue_()
94+
) # should produce a route.continue_ entry.
9395
page.goto(server.EMPTY_PAGE)
96+
page.goto(
97+
server.PREFIX + "/one-style.html"
98+
) # should not produce a route.continue_ entry since we continue all routes if no match.
9499
page.close()
95100
trace_file_path = tmpdir / "trace.zip"
96101
context.tracing.stop(path=trace_file_path)
@@ -107,8 +112,8 @@ def test_should_collect_trace_with_resources_but_no_js(
107112
"Page.wait_for_timeout",
108113
"Page.route",
109114
"Page.goto",
110-
# FIXME: https://github.com/microsoft/playwright-python/issues/1397
111-
"Channel.send",
115+
"Route.continue_",
116+
"Page.goto",
112117
"Page.close",
113118
"Tracing.stop",
114119
]
@@ -215,7 +220,13 @@ def parse_trace(path: Path) -> Tuple[Dict[str, bytes], List[Any]]:
215220

216221
def get_actions(events: List[Any]) -> List[str]:
217222
action_events = sorted(
218-
list(filter(lambda e: e["type"] == "action", events)),
223+
list(
224+
filter(
225+
lambda e: e["type"] == "action"
226+
and e["metadata"].get("internal", False) is False,
227+
events,
228+
)
229+
),
219230
key=lambda e: e["metadata"]["startTime"],
220231
)
221232
return [e["metadata"]["apiName"] for e in action_events]

0 commit comments

Comments
 (0)