diff --git a/packages/notte-sdk/src/notte_sdk/endpoints/sessions.py b/packages/notte-sdk/src/notte_sdk/endpoints/sessions.py index 2bfc3fec4..3d00adb0e 100644 --- a/packages/notte-sdk/src/notte_sdk/endpoints/sessions.py +++ b/packages/notte-sdk/src/notte_sdk/endpoints/sessions.py @@ -107,6 +107,17 @@ def _install_server_owned_dialog_policy(browser: Any) -> None: context.on("dialog", _observe_server_owned_dialog) +def _playwright_object_id(value: Any) -> str: + """Return a useful, best-effort identity for a Playwright object.""" + try: + guid = getattr(getattr(value, "_impl_obj", None), "_guid", None) + if guid is not None: + return str(guid) + except Exception: + pass + return hex(id(value)) + + try: from playwright.sync_api import Browser as BrowserSync from playwright.sync_api import Page as PageSync @@ -684,6 +695,172 @@ def __init__( self._async_playwright_context: "PlaywrightAsync | None" = None self._async_playwright_browser: "BrowserAsync | None" = None self._async_playwright_page: "PageAsync | None" = None + self._playwright_connection_generation = 0 + self._playwright_observed_pages: set[int] = set() + self._playwright_observed_contexts: set[int] = set() + self._playwright_cleanup_in_progress = False + + def _playwright_session_id(self) -> str: + try: + return self.session_id + except Exception: + return "unknown" + + @staticmethod + def _playwright_page_state(page: Any) -> dict[str, Any]: + state: dict[str, Any] = {"page_id": _playwright_object_id(page)} + try: + state["url"] = page.url + except Exception as exc: + state["url_error"] = f"{type(exc).__name__}: {exc}" + try: + state["is_closed"] = page.is_closed() + except Exception as exc: + state["is_closed_error"] = f"{type(exc).__name__}: {exc}" + return state + + def _playwright_state(self, browser: Any, cached_page: Any | None) -> dict[str, Any]: + state: dict[str, Any] = { + "connection_generation": self._playwright_connection_generation, + "cleanup_in_progress": self._playwright_cleanup_in_progress, + "cached_page_id": _playwright_object_id(cached_page) if cached_page is not None else None, + } + try: + state["browser_connected"] = browser.is_connected() + except Exception as exc: + state["browser_connected_error"] = f"{type(exc).__name__}: {exc}" + + contexts: list[dict[str, Any]] = [] + try: + for context in browser.contexts: + context_state: dict[str, Any] = {"context_id": _playwright_object_id(context)} + try: + pages = list(context.pages) + context_state["pages"] = [self._playwright_page_state(page) for page in pages] + context_state["contains_cached_page"] = cached_page in pages if cached_page is not None else False + except Exception as exc: + context_state["pages_error"] = f"{type(exc).__name__}: {exc}" + contexts.append(context_state) + except Exception as exc: + state["contexts_error"] = f"{type(exc).__name__}: {exc}" + state["contexts"] = contexts + return state + + def _log_playwright_event( + self, + event: str, + *, + browser: Any, + cached_page: Any | None, + observed_page: Any | None = None, + observed_context: Any | None = None, + level: Literal["info", "warning"] = "warning", + ) -> None: + details = self._playwright_state(browser, cached_page) + if observed_page is not None: + details["observed_page"] = self._playwright_page_state(observed_page) + if observed_context is not None: + details["observed_context_id"] = _playwright_object_id(observed_context) + bound_logger = logger.bind( + event=event, + session_id=self._playwright_session_id(), + **details, + ) + getattr(bound_logger, level)( + f"[Session Playwright] {event}: session={self._playwright_session_id()} state={details}" + ) + + def _log_unhealthy_cached_playwright_page(self, browser: Any, page: Any) -> None: + state = self._playwright_state(browser, page) + page_state = self._playwright_page_state(page) + browser_disconnected = state.get("browser_connected") is False + page_closed = page_state.get("is_closed") is True + page_missing = not any(context.get("contains_cached_page") for context in state["contexts"]) + if browser_disconnected or page_closed or page_missing: + self._log_playwright_event( + "sdk_playwright_cached_page_unhealthy", + browser=browser, + cached_page=page, + ) + + def _observe_playwright_page(self, browser: Any, page: Any, *, is_async: bool) -> None: + if id(page) in self._playwright_observed_pages: + return + self._playwright_observed_pages.add(id(page)) + + def on_close(*_args: Any) -> None: + cached_page = self._async_playwright_page if is_async else self._playwright_page + self._log_playwright_event( + "sdk_playwright_page_closed", + browser=browser, + cached_page=cached_page, + observed_page=page, + ) + + def on_crash(*_args: Any) -> None: + cached_page = self._async_playwright_page if is_async else self._playwright_page + self._log_playwright_event( + "sdk_playwright_page_crashed", + browser=browser, + cached_page=cached_page, + observed_page=page, + ) + + page.on("close", on_close) + page.on("crash", on_crash) + + def _observe_playwright_browser(self, browser: Any, *, is_async: bool) -> None: + cached_page = self._async_playwright_page if is_async else self._playwright_page + + def on_disconnected(*_args: Any) -> None: + current_page = self._async_playwright_page if is_async else self._playwright_page + self._log_playwright_event( + "sdk_playwright_browser_disconnected", + browser=browser, + cached_page=current_page, + ) + + browser.on("disconnected", on_disconnected) + for context in browser.contexts: + if id(context) not in self._playwright_observed_contexts: + self._playwright_observed_contexts.add(id(context)) + + def on_page(page: Any, *, _browser: Any = browser, _is_async: bool = is_async) -> None: + self._observe_playwright_page(_browser, page, is_async=_is_async) + current_page = self._async_playwright_page if _is_async else self._playwright_page + self._log_playwright_event( + "sdk_playwright_page_created", + browser=_browser, + cached_page=current_page, + observed_page=page, + level="info", + ) + + def on_context_close( + *_args: Any, + _browser: Any = browser, + _context: Any = context, + _is_async: bool = is_async, + ) -> None: + current_page = self._async_playwright_page if _is_async else self._playwright_page + self._log_playwright_event( + "sdk_playwright_context_closed", + browser=_browser, + cached_page=current_page, + observed_context=_context, + ) + + context.on("page", on_page) + context.on("close", on_context_close) + for page in context.pages: + self._observe_playwright_page(browser, page, is_async=is_async) + + self._log_playwright_event( + "sdk_playwright_connected", + browser=browser, + cached_page=cached_page, + level="info", + ) @override def __exit__( # pyright: ignore [reportMissingSuperCall] @@ -693,6 +870,7 @@ def __exit__( # pyright: ignore [reportMissingSuperCall] logger.warning(f"Session exiting because of exception: {exc_val}") # Clean up sync playwright resources + self._playwright_cleanup_in_progress = True if self._playwright_browser is not None: self._playwright_browser.close() self._playwright_browser = None @@ -731,6 +909,7 @@ async def __aexit__( logger.warning(f"Session exiting because of exception: {exc_val}") # Clean up async playwright resources + self._playwright_cleanup_in_progress = True if self._async_playwright_browser is not None: await self._async_playwright_browser.close() self._async_playwright_browser = None @@ -863,6 +1042,16 @@ def stop(self, close_reason: Literal["manual", "error"] = "manual") -> None: ValueError: If the session hasn't been started (no session_id available). RuntimeError: If the session fails to close properly. """ + self._playwright_cleanup_in_progress = True + browser = self._playwright_browser or self._async_playwright_browser + cached_page = self._playwright_page or self._async_playwright_page + if browser is not None: + self._log_playwright_event( + "sdk_playwright_session_stop_requested", + browser=browser, + cached_page=cached_page, + level="info", + ) if self._cookie_file is not None: try: cookies = self.get_cookies() @@ -1140,6 +1329,8 @@ def page(self) -> "PageSync": # Return cached page if already connected if self._playwright_page is not None: + if self._playwright_browser is not None: + self._log_unhealthy_cached_playwright_page(self._playwright_browser, self._playwright_page) return self._playwright_page if self._async_playwright_page is not None: @@ -1158,9 +1349,11 @@ def page(self) -> "PageSync": cdp_url = self.cdp_url() self._playwright_browser = self._playwright_context.chromium.connect_over_cdp(cdp_url) _install_server_owned_dialog_policy(self._playwright_browser) + self._playwright_connection_generation += 1 # Get the first page from the first context self._playwright_page = self._playwright_browser.contexts[0].pages[0] + self._observe_playwright_browser(self._playwright_browser, is_async=False) return self._playwright_page except Exception as e: raise RuntimeError("Failed to access the playwright page from CDP") from e @@ -1197,6 +1390,8 @@ async def apage(self) -> "PageAsync": # Return cached page if already connected if self._async_playwright_page is not None: + if self._async_playwright_browser is not None: + self._log_unhealthy_cached_playwright_page(self._async_playwright_browser, self._async_playwright_page) return self._async_playwright_page if self._playwright_browser is not None: @@ -1214,9 +1409,11 @@ async def apage(self) -> "PageAsync": cdp_url = self.cdp_url() self._async_playwright_browser = await self._async_playwright_context.chromium.connect_over_cdp(cdp_url) _install_server_owned_dialog_policy(self._async_playwright_browser) + self._playwright_connection_generation += 1 # Get the first page from the first context self._async_playwright_page = self._async_playwright_browser.contexts[0].pages[0] + self._observe_playwright_browser(self._async_playwright_browser, is_async=True) return self._async_playwright_page except Exception as e: raise RuntimeError("Failed to access the async playwright page from CDP") from e diff --git a/tests/sdk/test_session_playwright_dialog_policy.py b/tests/sdk/test_session_playwright_dialog_policy.py index 9f2615a14..542c62801 100644 --- a/tests/sdk/test_session_playwright_dialog_policy.py +++ b/tests/sdk/test_session_playwright_dialog_policy.py @@ -1,4 +1,4 @@ -from unittest.mock import AsyncMock, MagicMock +from unittest.mock import ANY, AsyncMock, MagicMock import pytest from notte_sdk.endpoints import sessions @@ -13,6 +13,11 @@ def _bare_session() -> RemoteSession: session._async_playwright_browser = None session._async_playwright_context = None session._async_playwright_page = None + session._playwright_connection_generation = 0 + session._playwright_observed_pages = set() + session._playwright_observed_contexts = set() + session._playwright_cleanup_in_progress = False + session.response = MagicMock(session_id="session-id") session.cdp_url = MagicMock(return_value="ws://notte.example/cdp") return session @@ -38,7 +43,12 @@ def test_sync_page_makes_backend_the_native_dialog_owner(monkeypatch: pytest.Mon assert session.page is page - context.on.assert_called_once_with("dialog", sessions._observe_server_owned_dialog) + context.on.assert_any_call("dialog", sessions._observe_server_owned_dialog) + context.on.assert_any_call("page", ANY) + context.on.assert_any_call("close", ANY) + page.on.assert_any_call("close", ANY) + page.on.assert_any_call("crash", ANY) + browser.on.assert_called_once_with("disconnected", ANY) assert sessions._observe_server_owned_dialog(MagicMock()) is None @@ -55,5 +65,87 @@ async def test_async_page_makes_backend_the_native_dialog_owner(monkeypatch: pyt assert await session.apage is page - context.on.assert_called_once_with("dialog", sessions._observe_server_owned_dialog) + context.on.assert_any_call("dialog", sessions._observe_server_owned_dialog) + context.on.assert_any_call("page", ANY) + context.on.assert_any_call("close", ANY) + page.on.assert_any_call("close", ANY) + page.on.assert_any_call("crash", ANY) + browser.on.assert_called_once_with("disconnected", ANY) assert sessions._observe_server_owned_dialog(MagicMock()) is None + + +def test_sync_page_lifecycle_callbacks_capture_state(monkeypatch: pytest.MonkeyPatch) -> None: + session = _bare_session() + browser, context, page = _browser_with_page() + page.url = "https://example.com" + page.is_closed.return_value = False + browser.is_connected.return_value = True + playwright = MagicMock() + playwright.chromium.connect_over_cdp.return_value = browser + starter = MagicMock() + starter.start.return_value = playwright + monkeypatch.setattr(sessions, "_playwright_available", True) + monkeypatch.setattr(sessions, "_sync_playwright", MagicMock(return_value=starter)) + log_event = MagicMock() + session._log_playwright_event = log_event + + assert session.page is page + + close_callback = next(call.args[1] for call in page.on.call_args_list if call.args[0] == "close") + crash_callback = next(call.args[1] for call in page.on.call_args_list if call.args[0] == "crash") + disconnect_callback = browser.on.call_args.args[1] + context_close_callback = next(call.args[1] for call in context.on.call_args_list if call.args[0] == "close") + close_callback(page) + crash_callback(page) + context_close_callback(context) + disconnect_callback(browser) + + assert [call.args[0] for call in log_event.call_args_list] == [ + "sdk_playwright_connected", + "sdk_playwright_page_closed", + "sdk_playwright_page_crashed", + "sdk_playwright_context_closed", + "sdk_playwright_browser_disconnected", + ] + + +def test_cached_page_logs_when_it_is_no_longer_live(monkeypatch: pytest.MonkeyPatch) -> None: + session = _bare_session() + browser, _context, page = _browser_with_page() + session._playwright_browser = browser + session._playwright_page = page + browser.is_connected.return_value = True + page.is_closed.return_value = True + monkeypatch.setattr(sessions, "_playwright_available", True) + log_event = MagicMock() + session._log_playwright_event = log_event + + assert session.page is page + + log_event.assert_called_once_with( + "sdk_playwright_cached_page_unhealthy", + browser=browser, + cached_page=page, + ) + + +def test_new_page_callback_is_instrumented(monkeypatch: pytest.MonkeyPatch) -> None: + session = _bare_session() + browser, context, page = _browser_with_page() + playwright = MagicMock() + playwright.chromium.connect_over_cdp.return_value = browser + starter = MagicMock() + starter.start.return_value = playwright + monkeypatch.setattr(sessions, "_playwright_available", True) + monkeypatch.setattr(sessions, "_sync_playwright", MagicMock(return_value=starter)) + log_event = MagicMock() + session._log_playwright_event = log_event + + assert session.page is page + new_page = MagicMock() + page_callback = next(call.args[1] for call in context.on.call_args_list if call.args[0] == "page") + page_callback(new_page) + + new_page.on.assert_any_call("close", ANY) + new_page.on.assert_any_call("crash", ANY) + assert log_event.call_args_list[-1].args[0] == "sdk_playwright_page_created"