Skip to content
Merged
62 changes: 54 additions & 8 deletions openhands-tools/openhands/tools/browser_use/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,12 @@ async def wrapper(self: BrowserToolExecutor, *args: Any, **kwargs: Any) -> Any:
logger = get_logger(__name__)

DEFAULT_BROWSER_ACTION_TIMEOUT_SECONDS = 300.0
# After this many consecutive failures, reset the browser session
# (assumes the browser has crashed or become unrecoverable).
MAX_CONSECUTIVE_FAILURES = 3
# Shorter timeout used after a failure to avoid long cascading waits
# against a dead browser.
DEGRADED_TIMEOUT_SECONDS = 30.0


def _format_browser_operation_error(
Expand Down Expand Up @@ -324,28 +330,68 @@ def init_logic():
self._async_executor = AsyncExecutor()
Comment thread
VascoSch92 marked this conversation as resolved.
Resolved
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 recent failures to avoid long waits
# against a potentially dead browser.
effective_timeout = (
DEGRADED_TIMEOUT_SECONDS
if self._consecutive_failures > 0
else self._action_timeout_seconds
Comment thread
VascoSch92 marked this conversation as resolved.
Resolved
)
Comment thread
VascoSch92 marked this conversation as resolved.
Resolved

try:
Comment thread
VascoSch92 marked this conversation as resolved.
Resolved
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,
return self._handle_action_failure(
_format_browser_operation_error(
error, timeout_seconds=effective_timeout
)
)

if result.is_error:
Comment thread
VascoSch92 marked this conversation as resolved.
Resolved
return self._handle_action_failure(result.text)

self._consecutive_failures = 0
return result

def _handle_action_failure(self, error_text: str):
"""Track consecutive failures and reset the session if needed."""
from openhands.tools.browser_use.definition import BrowserObservation

self._consecutive_failures += 1

Comment thread
VascoSch92 marked this conversation as resolved.
Resolved
if self._consecutive_failures >= MAX_CONSECUTIVE_FAILURES:
logger.warning(
"Browser appears crashed (%d consecutive failures). "
"Resetting session for automatic recovery.",
self._consecutive_failures,
)
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 (
Expand Down
104 changes: 104 additions & 0 deletions tests/tools/browser_use/test_browser_executor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -247,6 +248,109 @@ 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

Comment thread
VascoSch92 marked this conversation as resolved.
Resolved
# 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_degraded_timeout_after_failure(mock_browser_executor):
"""After a failure, subsequent actions should use a shorter timeout.

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 should use degraded 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"] == 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
Expand Down
9 changes: 8 additions & 1 deletion tests/tools/browser_use/test_browser_initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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 (
Expand Down
Loading