Skip to content

Commit 61caf4a

Browse files
committed
fix: allow browser session reinitialization after close
Set _started = False in _async_cleanup() to allow playwright to reinitialize when creating new sessions after close. Previously, _started remained True after cleanup, preventing _start() from being called and leaving _playwright as None, which caused "Playwright not initialized" errors. Add test to verify sessions can be created successfully after close.
1 parent e1e9336 commit 61caf4a

File tree

2 files changed

+70
-7
lines changed

2 files changed

+70
-7
lines changed

src/strands_tools/browser/browser.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -961,6 +961,7 @@ async def _async_cleanup(self) -> None:
961961
except Exception as e:
962962
cleanup_errors.append(f"Error stopping Playwright: {str(e)}")
963963
self._playwright = None
964+
self._started = False
964965

965966
self.close_platform()
966967
self._sessions.clear()

tests/browser/test_agent_core_browser.py

Lines changed: 69 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ def test_bedrock_browser_registers_atexit_handler():
2222

2323
with patch("strands_tools.browser.agent_core_browser.atexit.register") as mock_atexit:
2424
AgentCoreBrowser()
25-
25+
2626
# Verify atexit.register was called once
2727
mock_atexit.assert_called_once()
28-
28+
2929
# Verify it registered the parent class's close_platform method
3030
# (atexit.register is called with super().close_platform)
3131
registered_func = mock_atexit.call_args[0][0]
@@ -79,28 +79,28 @@ async def test_bedrock_browser_create_browser_session_hydrates_client_dict():
7979
from unittest.mock import AsyncMock, patch
8080

8181
browser = AgentCoreBrowser()
82-
82+
8383
# Mock playwright
8484
mock_playwright = Mock()
8585
mock_chromium = Mock()
8686
mock_browser = Mock()
8787
mock_playwright.chromium = mock_chromium
8888
mock_chromium.connect_over_cdp = AsyncMock(return_value=mock_browser)
8989
browser._playwright = mock_playwright
90-
90+
9191
# Mock BrowserClient
9292
mock_client = Mock()
9393
mock_session_id = "test-session-123"
9494
mock_client.start.return_value = mock_session_id
9595
mock_client.generate_ws_headers.return_value = ("ws://test-url", {"header": "value"})
96-
96+
9797
with patch("strands_tools.browser.agent_core_browser.AgentCoreBrowserClient", return_value=mock_client):
9898
# Verify _client_dict is empty before
9999
assert browser._client_dict == {}
100-
100+
101101
# Create browser session
102102
await browser.create_browser_session()
103-
103+
104104
# Verify _client_dict is hydrated with the session
105105
assert mock_session_id in browser._client_dict
106106
assert browser._client_dict[mock_session_id] == mock_client
@@ -147,3 +147,65 @@ def test_bedrock_browser_close_platform_with_errors():
147147
# Verify all clients were attempted to be stopped
148148
mock_client1.stop.assert_called_once()
149149
mock_client2.stop.assert_called_once()
150+
151+
152+
def test_bedrock_browser_reinitializes_playwright_after_close():
153+
"""Test that playwright reinitializes when creating a new session after close.
154+
155+
This verifies the fix for the issue where:
156+
1. A session is created successfully (playwright initialized)
157+
2. The browser is closed (sets _playwright to None and _started to False)
158+
3. A new session is created successfully (playwright reinitializes via _start())
159+
"""
160+
from unittest.mock import AsyncMock, patch
161+
162+
browser = AgentCoreBrowser()
163+
164+
# Mock playwright for initial session
165+
mock_playwright_instance = Mock()
166+
mock_chromium = Mock()
167+
mock_browser_obj = Mock()
168+
mock_context = Mock()
169+
mock_page = Mock()
170+
171+
mock_playwright_instance.chromium = mock_chromium
172+
mock_chromium.connect_over_cdp = AsyncMock(return_value=mock_browser_obj)
173+
mock_browser_obj.contexts = [mock_context]
174+
mock_context.new_page = AsyncMock(return_value=mock_page)
175+
mock_playwright_instance.stop = AsyncMock()
176+
177+
mock_client = Mock()
178+
mock_session_id = "test-session-123"
179+
mock_client.start.return_value = mock_session_id
180+
mock_client.generate_ws_headers.return_value = ("ws://test-url", {"header": "value"})
181+
182+
with patch("strands_tools.browser.agent_core_browser.AgentCoreBrowserClient", return_value=mock_client):
183+
with patch("strands_tools.browser.browser.async_playwright") as mock_async_pw:
184+
mock_async_pw.return_value.start = AsyncMock(return_value=mock_playwright_instance)
185+
186+
# Create initial session through browser() method
187+
result = browser.browser(
188+
{"action": {"type": "init_session", "session_name": "test-session", "description": "Test"}}
189+
)
190+
assert result["status"] == "success"
191+
assert mock_session_id in browser._client_dict
192+
assert browser._started is True
193+
assert browser._playwright is not None
194+
195+
# Close the browser - this sets _playwright to None and _started to False
196+
close_result = browser.close({"session_name": "test-session"})
197+
assert close_result["status"] == "success"
198+
assert browser._playwright is None
199+
assert browser._started is False
200+
201+
# Create a new session - _start() should reinitialize playwright automatically
202+
result2 = browser.browser(
203+
{"action": {"type": "init_session", "session_name": "test-session-2", "description": "Test 2"}}
204+
)
205+
206+
# Verify playwright was reinitialized and new session created successfully
207+
assert result2["status"] == "success"
208+
assert browser._started is True
209+
assert browser._playwright is not None
210+
# Verify async_playwright().start() was called twice (once for each session init)
211+
assert mock_async_pw.return_value.start.call_count == 2

0 commit comments

Comments
 (0)