Skip to content

fix(sdk): instrument Playwright lifecycle failures - #940

Draft
leo-notte wants to merge 1 commit into
mainfrom
fix/sdk-playwright-lifecycle-instrumentation
Draft

fix(sdk): instrument Playwright lifecycle failures#940
leo-notte wants to merge 1 commit into
mainfrom
fix/sdk-playwright-lifecycle-instrumentation

Conversation

@leo-notte

@leo-notte leo-notte commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Context

Several production SDK consumers received Playwright TargetClosedError while the corresponding Notte browser sessions and browser-worker pods remained alive. The current SDK caches the first Playwright page indefinitely, which leaves too little evidence to distinguish a page replacement or detach from a context close or a full CDP disconnect.

Changes

  • observe sync and async Playwright browser, context, and page lifecycle events
  • log browser connection state and the complete context/page inventory
  • track whether the cached page is closed or absent from the current context
  • record new pages, page close/crash, context close, browser disconnect, connection generation, and intentional SDK cleanup
  • emit a pre-stop snapshot so intentional cleanup can be separated from the triggering failure
  • keep behavior unchanged: this PR adds diagnostics only and does not reconnect, replace pages, or retry operations

Emitted events

  • sdk_playwright_connected
  • sdk_playwright_page_created
  • sdk_playwright_page_closed
  • sdk_playwright_page_crashed
  • sdk_playwright_context_closed
  • sdk_playwright_browser_disconnected
  • sdk_playwright_cached_page_unhealthy
  • sdk_playwright_session_stop_requested

Validation

  • uv run ruff format packages/notte-sdk/src/notte_sdk/endpoints/sessions.py tests/sdk/test_session_playwright_dialog_policy.py
  • uv run ruff check packages/notte-sdk/src/notte_sdk/endpoints/sessions.py tests/sdk/test_session_playwright_dialog_policy.py
  • uv run pytest tests/sdk/test_session_playwright_dialog_policy.py tests/sdk/test_session_context_close_reason.py -q (8 passed)
  • git diff --check

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds sync and async Playwright lifecycle instrumentation to RemoteSession while preserving the existing cached-page behavior.

  • Records browser, context, page, connection-generation, and cleanup state around lifecycle events.
  • Adds diagnostics for unhealthy cached pages and intentional session shutdown.
  • Extends unit coverage for listener registration, lifecycle callbacks, unhealthy-page detection, and newly created pages.

Confidence Score: 3/5

This PR should not merge until sensitive query parameters and fragments are removed from Playwright URLs before lifecycle snapshots are logged.

Caller-controlled browser URLs are copied verbatim into lifecycle log messages, exposing credentials whenever automation visits token-bearing or signed URLs.

Files Needing Attention: packages/notte-sdk/src/notte_sdk/endpoints/sessions.py

Security Review

The lifecycle snapshot logs complete page URLs, allowing OAuth codes, signed URL parameters, or other URL credentials to leak into application logs. Query parameters and fragments should be removed or selectively redacted before emission.

Important Files Changed

Filename Overview
packages/notte-sdk/src/notte_sdk/endpoints/sessions.py Adds comprehensive Playwright lifecycle snapshots and listeners, but emits complete browser URLs into application logs without redaction.
tests/sdk/test_session_playwright_dialog_policy.py Extends unit coverage for sync and async lifecycle listener registration and emitted diagnostic events.

Fix all with Greploop Fix All in Codex

Prompt To Fix All With AI
### Issue 1
packages/notte-sdk/src/notte_sdk/endpoints/sessions.py:713
**Lifecycle logs expose URL credentials**

When an SDK consumer visits an OAuth callback, signed URL, password-reset link, or another credential-bearing URL, `_playwright_page_state` copies the complete query string and fragment into lifecycle log messages, exposing tokens and signatures to application log readers and collection systems.

**How this was verified:** The full `page.url` flows into the state dictionary interpolated by `_log_playwright_event`, and the configured stderr logger applies no URL redaction.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "fix(sdk): instrument Playwright lifecycl..." | Re-trigger Greptile

def _playwright_page_state(page: Any) -> dict[str, Any]:
state: dict[str, Any] = {"page_id": _playwright_object_id(page)}
try:
state["url"] = page.url

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Lifecycle logs expose URL credentials

When an SDK consumer visits an OAuth callback, signed URL, password-reset link, or another credential-bearing URL, _playwright_page_state copies the complete query string and fragment into lifecycle log messages, exposing tokens and signatures to application log readers and collection systems.

How this was verified: The full page.url flows into the state dictionary interpolated by _log_playwright_event, and the configured stderr logger applies no URL redaction.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/notte-sdk/src/notte_sdk/endpoints/sessions.py
Line: 713

Comment:
**Lifecycle logs expose URL credentials**

When an SDK consumer visits an OAuth callback, signed URL, password-reset link, or another credential-bearing URL, `_playwright_page_state` copies the complete query string and fragment into lifecycle log messages, exposing tokens and signatures to application log readers and collection systems.

**How this was verified:** The full `page.url` flows into the state dictionary interpolated by `_log_playwright_event`, and the configured stderr logger applies no URL redaction.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Codex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant