fix(tracing): apiName determination with event listeners (#2651)

Author: mxschmitt

playwright/_impl/_connection.py

@@ -37,6 +37,7 @@
 from pyee.asyncio import AsyncIOEventEmitter
 
 import playwright
+import playwright._impl._impl_to_api_mapping
 from playwright._impl._errors import TargetClosedError, rewrite_error
 from playwright._impl._greenlets import EventGreenlet
 from playwright._impl._helper import Error, ParsedMessagePayload, parse_error
@@ -573,6 +574,12 @@ def _extract_stack_trace_information_from_stack(
     api_name = ""
     parsed_frames: List[StackFrame] = []
     for frame in st:
+        # Sync and Async implementations can have event handlers. When these are sync, they
+        # get evaluated in the context of the event loop, so they contain the stack trace of when
+        # the message was received. _impl_to_api_mapping is glue between the user-code and internal
+        # code to translate impl classes to api classes. We want to ignore these frames.
+        if playwright._impl._impl_to_api_mapping.__file__ == frame.filename:
+            continue
         is_playwright_internal = frame.filename.startswith(playwright_module_path)
 
         method_name = ""

tests/async/test_tracing.py

@@ -12,11 +12,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import asyncio
 import re
 from pathlib import Path
 from typing import Dict, List
 
-from playwright.async_api import Browser, BrowserContext, BrowserType, Page
+from playwright.async_api import Browser, BrowserContext, BrowserType, Page, Response
 from tests.server import Server
 from tests.utils import get_trace_actions, parse_trace
 
@@ -145,6 +146,35 @@ async def test_should_collect_trace_with_resources_but_no_js(
     assert script["snapshot"]["response"]["content"].get("_sha1") is None
 
 
+async def test_should_correctly_determine_sync_apiname(
+    context: BrowserContext, page: Page, server: Server, tmpdir: Path
+) -> None:
+    await context.tracing.start(screenshots=True, snapshots=True)
+
+    received_response: "asyncio.Future[None]" = asyncio.Future()
+
+    async def _handle_response(response: Response) -> None:
+        await response.request.all_headers()
+        await response.text()
+        received_response.set_result(None)
+
+    page.once("response", _handle_response)
+    await page.goto(server.PREFIX + "/grid.html")
+    await received_response
+    await page.close()
+    trace_file_path = tmpdir / "trace.zip"
+    await context.tracing.stop(path=trace_file_path)
+
+    (_, events) = parse_trace(trace_file_path)
+    assert events[0]["type"] == "context-options"
+    assert get_trace_actions(events) == [
+        "Page.goto",
+        "Request.all_headers",
+        "Response.text",
+        "Page.close",
+    ]
+
+
 async def test_should_collect_two_traces(
     context: BrowserContext, page: Page, server: Server, tmpdir: Path
 ) -> None:

tests/sync/test_tracing.py

@@ -13,10 +13,11 @@
 # limitations under the License.
 
 import re
+import threading
 from pathlib import Path
 from typing import Any, Dict, List
 
-from playwright.sync_api import Browser, BrowserContext, BrowserType, Page
+from playwright.sync_api import Browser, BrowserContext, BrowserType, Page, Response
 from tests.server import Server
 from tests.utils import get_trace_actions, parse_trace
 
@@ -138,6 +139,35 @@ def test_should_collect_trace_with_resources_but_no_js(
     assert script["snapshot"]["response"]["content"].get("_sha1") is None
 
 
+def test_should_correctly_determine_sync_apiname(
+    context: BrowserContext, page: Page, server: Server, tmpdir: Path
+) -> None:
+    context.tracing.start(screenshots=True, snapshots=True)
+    received_response = threading.Event()
+
+    def _handle_response(response: Response) -> None:
+        response.request.all_headers()
+        response.text()
+        received_response.set()
+
+    page.once("response", _handle_response)
+    page.goto(server.PREFIX + "/grid.html")
+    received_response.wait()
+
+    page.close()
+    trace_file_path = tmpdir / "trace.zip"
+    context.tracing.stop(path=trace_file_path)
+
+    (_, events) = parse_trace(trace_file_path)
+    assert events[0]["type"] == "context-options"
+    assert get_trace_actions(events) == [
+        "Page.goto",
+        "Request.all_headers",
+        "Response.text",
+        "Page.close",
+    ]
+
+
 def test_should_collect_two_traces(
     context: BrowserContext, page: Page, server: Server, tmpdir: Path
 ) -> None: