fix(tools): Add browser crash detection and automatic session recovery

openhands-tools/openhands/tools/browser_use/impl.py

@@ -11,7 +11,7 @@
 import subprocess
 from collections.abc import Callable, Coroutine
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, TypeVar
+from typing import TYPE_CHECKING, Any, Final, TypeVar
 
 
 if TYPE_CHECKING:
@@ -90,7 +90,13 @@ async def wrapper(self: BrowserToolExecutor, *args: Any, **kwargs: Any) -> Any:
 
 logger = get_logger(__name__)
 
-DEFAULT_BROWSER_ACTION_TIMEOUT_SECONDS = 300.0
+DEFAULT_BROWSER_ACTION_TIMEOUT_SECONDS: Final[float] = 300.0
+# After this many consecutive failures, reset the browser session
+# (assumes the browser has crashed or become unrecoverable).
+MAX_CONSECUTIVE_FAILURES: Final[int] = 3
+# Shorter timeout used after a failure to avoid long cascading waits
+# against a dead browser.
+DEGRADED_TIMEOUT_SECONDS: Final[float] = 30.0
 
 
 def _format_browser_operation_error(
@@ -324,28 +330,83 @@ def init_logic():
         self._async_executor = AsyncExecutor()
         self._cleanup_initiated = False
         self._action_timeout_seconds = action_timeout_seconds
+        self._consecutive_failures = 0
 
     def __call__(
         self,
         action: BrowserAction,
         conversation: LocalConversation | None = None,  # noqa: ARG002
     ):
         """Submit an action to run in the background loop and wait for result."""
+        # Use a shorter timeout after multiple consecutive timeout failures
+        # to avoid long cascading waits against a dead browser.
+        effective_timeout = (
+            DEGRADED_TIMEOUT_SECONDS
+            if self._consecutive_failures >= 2
+            else self._action_timeout_seconds
+        )
+
         try:
-            return self._async_executor.run_async(
+            result = self._async_executor.run_async(
                 self._execute_action,
                 action,
-                timeout=self._action_timeout_seconds,
+                timeout=effective_timeout,
             )
         except builtins.TimeoutError as error:
-            return BrowserObservation.from_text(
-                text=_format_browser_operation_error(
-                    error, timeout_seconds=self._action_timeout_seconds
-                ),
-                is_error=True,
-                full_output_save_dir=self.full_output_save_dir,
+            # Timeouts indicate the browser may be dead/hung — track them
+            # for crash detection. Regular action errors (invalid selector,
+            # missing element) are NOT counted since those are normal agent
+            # mistakes, not browser crashes.
+            return self._handle_timeout_failure(
+                _format_browser_operation_error(
+                    error, timeout_seconds=effective_timeout
+                )
             )
 
+        self._consecutive_failures = 0
+        return result
+
+    def _handle_timeout_failure(self, error_text: str):
+        """Track consecutive timeout failures and reset session if needed."""
+        self._consecutive_failures += 1
+        logger.debug(
+            "Browser timeout failure %d/%d",
+            self._consecutive_failures,
+            MAX_CONSECUTIVE_FAILURES,
+        )
+
+        if self._consecutive_failures >= MAX_CONSECUTIVE_FAILURES:
+            logger.warning(
+                "Browser appears crashed (%d consecutive failures). "
+                "Resetting session for automatic recovery.",
+                self._consecutive_failures,
+            )
+            # Best-effort cleanup of the old browser process/session.
+            # If the browser truly crashed this will fail fast; if it's
+            # wedged this avoids leaking the process.
+            try:
+                self._async_executor.run_async(self.cleanup, timeout=5.0)
+            except Exception as e:
+                logger.debug(
+                    "Cleanup during session reset failed "
+                    "(expected if browser crashed): %s",
+                    e,
+                )
+            self._initialized = False
+            self._consecutive_failures = 0
+            error_text = (
+                f"{error_text}\n\n"
+                "The browser session has been reset after multiple consecutive "
+                "failures (possible crash). The browser will be restarted on "
+                "the next action. Please retry your action."
+            )
+
+        return BrowserObservation.from_text(
+            text=error_text,
+            is_error=True,
+            full_output_save_dir=self.full_output_save_dir,
+        )
+
     async def _execute_action(self, action):
         """Execute browser action asynchronously."""
         from openhands.tools.browser_use.definition import (

tests/tools/browser_use/test_browser_executor.py

@@ -45,6 +45,7 @@ def __init__(self, action_timeout_seconds: float):
         self._cleanup_initiated = False
         self._action_timeout_seconds = action_timeout_seconds
         self.full_output_save_dir = None
+        self._consecutive_failures = 0
 
     async def navigate(self, url: str, new_tab: bool = False) -> str:
         del new_tab
@@ -247,6 +248,152 @@ def test_browser_executor_timeout_wrapping(mock_browser_executor):
     assert "timed out after 7 seconds" in result.text
 
 
+def test_issue_2412_consecutive_failures_reset_session(mock_browser_executor):
+    """After MAX_CONSECUTIVE_FAILURES timeouts, the session should be reset.
+
+    When a browser crashes, every subsequent action times out against
+    the dead session. After enough consecutive failures the executor
+    should set _initialized=False so the next call re-creates the
+    browser session instead of looping on the dead one.
+
+    See: https://github.com/OpenHands/software-agent-sdk/issues/2412
+    """
+    from openhands.tools.browser_use.impl import MAX_CONSECUTIVE_FAILURES
+
+    mock_browser_executor._initialized = True
+
+    with patch.object(
+        mock_browser_executor._async_executor,
+        "run_async",
+        side_effect=builtins.TimeoutError(),
+    ):
+        action = BrowserNavigateAction(url="https://example.com")
+
+        # First (MAX_CONSECUTIVE_FAILURES - 1) failures should NOT reset
+        for i in range(MAX_CONSECUTIVE_FAILURES - 1):
+            result = mock_browser_executor(action)
+            assert result.is_error is True
+            assert mock_browser_executor._initialized is True, (
+                f"Session reset too early on failure {i + 1}"
+            )
+            assert "reset" not in result.text.lower()
+
+        # The next failure triggers the reset
+        result = mock_browser_executor(action)
+        assert result.is_error is True
+        assert mock_browser_executor._initialized is False
+        assert "reset" in result.text.lower()
+        assert mock_browser_executor._consecutive_failures == 0
+
+
+def test_issue_2412_success_resets_failure_counter(mock_browser_executor):
+    """A successful action should reset the consecutive failure counter.
+
+    See: https://github.com/OpenHands/software-agent-sdk/issues/2412
+    """
+    mock_browser_executor._initialized = True
+
+    # Simulate 2 failures
+    with patch.object(
+        mock_browser_executor._async_executor,
+        "run_async",
+        side_effect=builtins.TimeoutError(),
+    ):
+        action = BrowserNavigateAction(url="https://example.com")
+        mock_browser_executor(action)
+        mock_browser_executor(action)
+
+    assert mock_browser_executor._consecutive_failures == 2
+
+    # Now a success
+    success_result = BrowserObservation.from_text(text="OK")
+    with patch.object(
+        mock_browser_executor._async_executor,
+        "run_async",
+        return_value=success_result,
+    ):
+        result = mock_browser_executor(action)
+
+    assert result.is_error is False
+    assert mock_browser_executor._consecutive_failures == 0
+
+
+def test_issue_2412_action_errors_do_not_trigger_reset(mock_browser_executor):
+    """Regular action errors should NOT count toward crash detection.
+
+    Only timeouts indicate a potentially dead browser. Errors like
+    invalid selector or missing element are normal agent mistakes.
+
+    See: https://github.com/OpenHands/software-agent-sdk/issues/2412
+    """
+    from openhands.tools.browser_use.impl import MAX_CONSECUTIVE_FAILURES
+
+    mock_browser_executor._initialized = True
+
+    error_result = BrowserObservation.from_text(
+        text="Element not found", is_error=True
+    )
+    with patch.object(
+        mock_browser_executor._async_executor,
+        "run_async",
+        return_value=error_result,
+    ):
+        action = BrowserNavigateAction(url="https://example.com")
+        for _ in range(MAX_CONSECUTIVE_FAILURES + 1):
+            mock_browser_executor(action)
+
+    # Session should NOT be reset despite many action errors
+    assert mock_browser_executor._initialized is True
+    assert mock_browser_executor._consecutive_failures == 0
+
+
+def test_issue_2412_degraded_timeout_after_failures(mock_browser_executor):
+    """Degraded timeout kicks in after 2+ consecutive timeout failures.
+
+    See: https://github.com/OpenHands/software-agent-sdk/issues/2412
+    """
+    from openhands.tools.browser_use.impl import DEGRADED_TIMEOUT_SECONDS
+
+    mock_browser_executor._initialized = True
+    mock_browser_executor._action_timeout_seconds = 300.0
+
+    action = BrowserNavigateAction(url="https://example.com")
+
+    # First call fails — uses normal timeout
+    with patch.object(
+        mock_browser_executor._async_executor,
+        "run_async",
+        side_effect=builtins.TimeoutError(),
+    ) as mock_run:
+        mock_browser_executor(action)
+        _, kwargs = mock_run.call_args
+        assert kwargs["timeout"] == 300.0
+
+    # Second call still uses normal timeout (degraded kicks in at 2+)
+    with patch.object(
+        mock_browser_executor._async_executor,
+        "run_async",
+        side_effect=builtins.TimeoutError(),
+    ) as mock_run:
+        mock_browser_executor(action)
+        _, kwargs = mock_run.call_args
+        assert kwargs["timeout"] == 300.0
+
+    # Third call should use degraded timeout for the action.
+    # Note: the reset also calls run_async for cleanup, so we check
+    # the first call (the action), not the last (the cleanup).
+    with patch.object(
+        mock_browser_executor._async_executor,
+        "run_async",
+        side_effect=builtins.TimeoutError(),
+    ) as mock_run:
+        mock_browser_executor(action)
+        # First call is the action (degraded timeout),
+        # second call may be cleanup (5s) if reset triggers.
+        _, kwargs = mock_run.call_args_list[0]
+        assert kwargs["timeout"] == DEGRADED_TIMEOUT_SECONDS
+
+
 async def test_browser_executor_initialization_lazy(mock_browser_executor):
     """Test that browser session initialization is lazy."""
     assert mock_browser_executor._initialized is False

tests/tools/browser_use/test_browser_initialization.py

@@ -169,10 +169,12 @@ def test_initialization_chromium_not_available(self):
 
     def test_call_method_delegates_to_async_executor(self):
         """Test that __call__ method properly delegates to async executor."""
+        from openhands.tools.browser_use.definition import BrowserObservation
+
         mock_server = MagicMock()
         mock_async_executor = MagicMock()
         mock_action = MagicMock()
-        expected_result = MagicMock()
+        expected_result = BrowserObservation.from_text(text="OK")
 
         mock_async_executor.run_async.return_value = expected_result
 
@@ -201,8 +203,13 @@ def test_call_method_delegates_to_async_executor(self):
 
     def test_call_method_timeout_configuration(self):
         """Test that __call__ method uses correct timeout."""
+        from openhands.tools.browser_use.definition import BrowserObservation
+
         mock_server = MagicMock()
         mock_async_executor = MagicMock()
+        mock_async_executor.run_async.return_value = BrowserObservation.from_text(
+            text="OK"
+        )
         mock_action = MagicMock()
 
         with (