Wrap ensure_injection in try/catch to handle page navigation errors

[derekmeegan]

This was breaking for stagehand extract/act/observe calls that occur right after an interaction that triggers a navigatin

[cubic-dev-ai]

1 issue found across 1 file

Confidence score: 3/5

• Catching a bare Exception in stagehand/page.py can mask real programming errors (e.g., TypeError, AttributeError) and replace them with a misleading navigation warning, creating user-impacting debugging risk.
• Score 3 reflects a concrete medium-severity issue that could hide real failures, but it’s localized and fixable.
• Pay close attention to stagehand/page.py - overly broad exception handling may swallow genuine errors.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="stagehand/page.py">

<violation number="1" location="stagehand/page.py:71">
P2: Catching bare `Exception` is too broad here and will silently swallow genuine programming errors (e.g., `TypeError`, `AttributeError`) with a misleading "page may be navigating" warning. Since the intent is specifically to handle Playwright page navigation/context destruction errors, catch `playwright.async_api.Error` instead. This is already importable from the same module used for `Page` and `CDPSession`.</violation>
</file>

Architecture diagram

sequenceDiagram
    participant S as Stagehand Page
    participant B as Browser (Playwright)
    participant FS as File System
    participant L as Logger

    Note over S, B: ensure_injection() Flow
    
    S->>B: evaluate("typeof window.getScrollableElementXpaths")
    
    alt NEW: Page navigates or becomes detached
        B-->>S: Throws Exception (Execution context destroyed)
        S->>L: NEW: Catch Exception & Log Warning
    else Success Path
        B-->>S: boolean (exists_before)
        
        opt Script not present
            alt Script not cached in memory
                S->>FS: Read domScripts.js
                FS-->>S: Script content
            end
            
            S->>B: evaluate(_INJECTION_SCRIPT)
            S->>B: add_init_script(_INJECTION_SCRIPT)
            
            alt NEW: Error during injection
                B-->>S: Throws Exception
                S->>L: NEW: Catch Exception & Log Warning
            end
        end
    end

    Note over S, L: Method completes without crashing caller (extract/act/observe)

Loading

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="stagehand/page.py">

<violation number="1" location="stagehand/page.py:79">
P2: The error message check is too narrow — only `"Execution context was destroyed"` is caught, but Playwright can also raise `"Target closed"` or `"frame was detached"` errors during navigation. Consider matching on multiple navigation-related error patterns to make this more robust against all navigation scenarios.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P2: The error message check is too narrow — only "Execution context was destroyed" is caught, but Playwright can also raise "Target closed" or "frame was detached" errors during navigation. Consider matching on multiple navigation-related error patterns to make this more robust against all navigation scenarios.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At stagehand/page.py, line 79:

<comment>The error message check is too narrow — only `"Execution context was destroyed"` is caught, but Playwright can also raise `"Target closed"` or `"frame was detached"` errors during navigation. Consider matching on multiple navigation-related error patterns to make this more robust against all navigation scenarios.</comment>

<file context>
@@ -76,10 +76,13 @@ async def ensure_injection(self):
-                f"ensure_injection failed (page may be navigating): {e}",
-                category="page",
-            )
+            if "Execution context was destroyed" in str(e):
+                self._stagehand.logger.warning(
+                    f"ensure_injection failed (page may be navigating): {e}",
</file context>

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="stagehand/page.py">

<violation number="1" location="stagehand/page.py:79">
P1: Catching only `TargetClosedError` drops the previous handling of `"Execution context was destroyed"` errors, which are a different Playwright exception type (`playwright._impl._errors.Error`). Both errors occur during page navigation and should be caught. This is a regression — the original navigation error will now bubble up unhandled.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

P1: Catching only TargetClosedError drops the previous handling of "Execution context was destroyed" errors, which are a different Playwright exception type (playwright._impl._errors.Error). Both errors occur during page navigation and should be caught. This is a regression — the original navigation error will now bubble up unhandled.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At stagehand/page.py, line 79:

<comment>Catching only `TargetClosedError` drops the previous handling of `"Execution context was destroyed"` errors, which are a different Playwright exception type (`playwright._impl._errors.Error`). Both errors occur during page navigation and should be caught. This is a regression — the original navigation error will now bubble up unhandled.</comment>

<file context>
@@ -75,14 +76,11 @@ async def ensure_injection(self):
-                )
-            else:
-                raise
+        except TargetClosedError as e:
+            self._stagehand.logger.warning(
+                f"ensure_injection failed (page may be navigating): {e}",
</file context>