Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions playwright/_impl/_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = ""
Expand Down
32 changes: 31 additions & 1 deletion tests/async/test_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
32 changes: 31 additions & 1 deletion tests/sync/test_tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
Loading