fix(sdk): instrument Playwright lifecycle failures

packages/notte-sdk/src/notte_sdk/endpoints/sessions.py

@@ -107,6 +107,17 @@
         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 @@
         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 @@
             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 @@
             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 @@
             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 @@
 
         # 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 @@
                 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 @@
 
         # 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 @@
                 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

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"