Skip to content

fix(cdp): avoid unhandled detach by returning original sendCDP promise#1644

Merged
monadoid merged 1 commit intomainfrom
STG-1240
Feb 1, 2026
Merged

fix(cdp): avoid unhandled detach by returning original sendCDP promise#1644
monadoid merged 1 commit intomainfrom
STG-1240

Conversation

@monadoid
Copy link
Contributor

@monadoid monadoid commented Jan 30, 2026

why

  • We saw worker crashes caused by unhandledRejection: Error("CDP session detached") when a Playwright‑connected page closed while Stagehand had inflight CDP calls.
  • Root cause: Page.sendCDP was async, which wraps the underlying CDP promise. The detach rejection hit the inner promise before the outer wrapper was awaited, so Node treated it as unhandled.

what changed

  • Fix by making Page.sendCDP return the original promise (remove async) so callers receive the same promise that is already handled internally. This eliminates the unhandled rejection race without changing semantics.

test plan

  • The new regression test reproduces this by starting a long‑running CDP command and closing the page; it failed on main with an unhandled rejection.

Summary by cubic

Fixes worker crashes from unhandled CDP session detaches when a page closes during in‑flight calls. Addresses STG-1240 by returning the original sendCDP promise and guarding internal CDP promises.

  • Bug Fixes
    • Page.sendCDP now returns the original promise (removed async) so rejections are handled by callers.
    • Added catch guards in CdpConnection to prevent early unhandledRejection on detach.
    • New regression test: long Runtime.evaluate + page close rejects cleanly with no unhandledRejection.
    • Patch changeset for @browserbasehq/stagehand.

Written for commit d5e86da. Summary will update on new commits. Review in cubic

@changeset-bot
Copy link

changeset-bot bot commented Jan 30, 2026

🦋 Changeset detected

Latest commit: d5e86da

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@browserbasehq/stagehand Patch
@browserbasehq/stagehand-evals Patch
@browserbasehq/stagehand-server Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 30, 2026

Greptile Overview

Greptile Summary

This PR fixes worker crashes caused by unhandled CDP session detachment rejections when a Playwright-connected page closes during inflight CDP calls.

Key changes:

  • Removed async keyword from Page.sendCDP method to return the original promise directly instead of wrapping it
  • Added void p.catch(() => {}) to both CDP promise creation points (CdpConnection.send and CdpConnection._sendViaSession) to ensure promises have an internal rejection handler
  • Added comprehensive regression test that verifies no unhandled rejections occur when CDP session detaches during inflight operations

Root cause: The async wrapper in Page.sendCDP created a new outer promise. When the browser closed and triggered a CDP session detachment, the inner promise was rejected before the outer promise was awaited, causing Node.js to treat it as unhandled.

Solution: By returning the original promise and adding internal catch handlers at the promise creation site, the promise chain maintains proper error handling while preserving the same external API semantics.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The fix is minimal, surgical, and addresses the specific race condition without changing API semantics. The regression test validates the fix works correctly, and the changes preserve all existing behavior while eliminating the unhandled rejection.
  • No files require special attention

Important Files Changed

Filename Overview
packages/core/lib/v3/understudy/page.ts Removed async keyword from sendCDP method to return the original promise directly, preventing unhandled rejection race condition
packages/core/lib/v3/understudy/cdp.ts Added void p.catch(() => {}) to internally handle promise rejections in both send and _sendViaSession methods, preventing unhandled rejections when CDP session detaches
packages/core/lib/v3/tests/cdp-session-detached.spec.ts Added regression test that verifies no unhandled rejections occur when CDP calls are inflight during page closure

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Page.sendCDP
    participant CdpSession
    participant CdpConnection
    participant WebSocket
    participant Browser

    Note over Caller,Browser: Before Fix: async wrapper creates unhandled rejection

    Caller->>Page.sendCDP: sendCDP("Runtime.evaluate", params)
    Note over Page.sendCDP: OLD: async wrapper creates new Promise
    Page.sendCDP->>CdpSession: send(method, params)
    CdpSession->>CdpConnection: _sendViaSession(sessionId, method, params)
    CdpConnection->>CdpConnection: Create promise p, store in inflight map
    CdpConnection->>WebSocket: send(JSON.stringify(payload))
    CdpConnection-->>Page.sendCDP: Return promise p
    Note over Page.sendCDP: Wrapper creates outer promise
    Page.sendCDP-->>Caller: Return wrapped promise

    Browser->>Browser: Page closes
    Browser->>WebSocket: Target.detachedFromTarget event
    WebSocket->>CdpConnection: onMessage(detach event)
    CdpConnection->>CdpConnection: Reject inflight promise p
    Note over CdpConnection,Caller: Inner promise rejected before outer awaited
    Note over Caller: UNHANDLED REJECTION!

    Note over Caller,Browser: After Fix: return original promise

    Caller->>Page.sendCDP: sendCDP("Runtime.evaluate", params)
    Note over Page.sendCDP: NEW: returns original promise directly
    Page.sendCDP->>CdpSession: send(method, params)
    CdpSession->>CdpConnection: _sendViaSession(sessionId, method, params)
    CdpConnection->>CdpConnection: Create promise p, store in inflight map
    CdpConnection->>CdpConnection: void p.catch(() => {})
    Note over CdpConnection: Promise already has internal handler
    CdpConnection->>WebSocket: send(JSON.stringify(payload))
    CdpConnection-->>Caller: Return promise p (same reference)

    Browser->>Browser: Page closes
    Browser->>WebSocket: Target.detachedFromTarget event
    WebSocket->>CdpConnection: onMessage(detach event)
    CdpConnection->>CdpConnection: Reject inflight promise p
    Note over CdpConnection: Internal catch handler prevents unhandled rejection
    Note over Caller: Caller receives rejection when they await
    Note over Caller: No unhandled rejection!
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 4 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.
Architecture diagram
sequenceDiagram
    participant Caller as Test/Application
    participant Page as Stagehand Page
    participant CDP as CdpConnection
    participant WS as Browser WebSocket

    Note over Caller,WS: CDP Command Execution Flow

    Caller->>>Page: sendCDP(method, params)
    
    Page->>>CDP: CHANGED: mainSession.send(method, params)
    Note right of Page: Now returns promise directly (removed async)

    CDP->>CDP: Create pending promise (p)
    
    rect rgb(23, 37, 84)
    Note right of CDP: NEW: Internal Guard
    CDP->>CDP: void p.catch(() => {})
    Note right of CDP: Prevents global unhandledRejection
    end

    CDP->>WS: Send JSON payload
    CDP-->>Page: Return promise (p)
    Page-->>Caller: Return promise (p)

    alt Happy Path
        WS-->>CDP: Response received
        CDP->>Caller: Resolve promise (p)
    else Unhappy Path (Session Detached)
        Note over Caller,WS: Page/Target closes while promise is inflight
        WS-->>CDP: Socket closed / Session detached
        CDP->>CDP: Reject all pending promises
        
        alt Node.js Runtime Check
            CDP-->>Caller: CHANGED: Rejection propagates via original promise
            Note right of Caller: Caller receives "CDP session detached" error
        else Global Error Handling
            Note over CDP: Internal catch guard prevents process crash
        end
    end
Loading

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.

2 participants

Comments