diff --git a/src/browser_harness/daemon.py b/src/browser_harness/daemon.py index 0f0f2555..6a92b3ba 100644 --- a/src/browser_harness/daemon.py +++ b/src/browser_harness/daemon.py @@ -179,6 +179,25 @@ def is_real_page(t): return t["type"] == "page" and not t.get("url", "").startswith(INTERNAL) +STALE_SESSION_MARKERS = ( + "Session with given id not found", +) + + +def _is_recoverable_current_session_error(msg, sid, current_session): + """True when daemon-owned re-attach is safe for a stale default session. + + Recovery changes the daemon's default tab/session, so only do it for calls + that used the current default session. Browser-level calls (sid=None) and + caller-specified stale sessions should surface the original error instead. + """ + return bool( + sid + and sid == current_session + and any(marker in (msg or "") for marker in STALE_SESSION_MARKERS) + ) + + class Daemon: def __init__(self): self.cdp = None @@ -349,7 +368,7 @@ async def disable_old(): return {"result": await self.cdp.send_raw(method, params, session_id=sid)} except Exception as e: msg = str(e) - if "Session with given id not found" in msg and sid == self.session and sid: + if _is_recoverable_current_session_error(msg, sid, self.session): log(f"stale session {sid}, re-attaching") if await self.attach_first_page(): return {"result": await self.cdp.send_raw(method, params, session_id=self.session)} diff --git a/tests/unit/test_daemon.py b/tests/unit/test_daemon.py index 90c5bc85..f05b4522 100644 --- a/tests/unit/test_daemon.py +++ b/tests/unit/test_daemon.py @@ -245,6 +245,71 @@ async def run(): ) +def test_recoverable_current_session_error_requires_current_default_session(): + """Stale-session recovery is only safe for the daemon's current default + session. Browser-level calls and explicit stale session IDs should surface + their original error instead of silently changing the caller's target.""" + assert daemon._is_recoverable_current_session_error( + "Session with given id not found", "session-A", "session-A" + ) is True + assert daemon._is_recoverable_current_session_error( + "Session with given id not found", None, "session-A" + ) is False + assert daemon._is_recoverable_current_session_error( + "Session with given id not found", "session-OLD", "session-NEW" + ) is False + assert daemon._is_recoverable_current_session_error( + "CDP WS handshake failed", "session-A", "session-A" + ) is False + + +def test_handle_reattaches_and_retries_current_session_after_stale_error(): + """First contract slice for #352: if a renderer-bound helper uses the + daemon's current session and Chrome reports that session as stale, the + daemon owns re-attach + retry so callers don't need per-helper recovery.""" + class _StaleOnceCDP(_FakeCDP): + def __init__(self): + super().__init__() + self.failed_once = False + + async def send_raw(self, method, params=None, session_id=None): + self.calls.append((method, params, session_id)) + if ( + method == "Runtime.evaluate" + and session_id == "session-OLD" + and not self.failed_once + ): + self.failed_once = True + raise RuntimeError("Session with given id not found") + if method == "Runtime.evaluate" and session_id == "session-NEW": + return {"type": "number", "value": 2} + return {} + + async def run(): + d = daemon.Daemon() + d.cdp = _StaleOnceCDP() + d.session = "session-OLD" + d.target_id = "target-OLD" + + async def attach_first_page(): + d.session = "session-NEW" + d.target_id = "target-NEW" + return {"targetId": "target-NEW", "type": "page", "url": "about:blank"} + + d.attach_first_page = attach_first_page + result = await d.handle({ + "method": "Runtime.evaluate", + "params": {"expression": "1 + 1"}, + }) + return result, d.cdp.calls + + result, calls = asyncio.run(run()) + + assert result == {"result": {"type": "number", "value": 2}} + runtime_sessions = [sid for (method, _params, sid) in calls if method == "Runtime.evaluate"] + assert runtime_sessions == ["session-OLD", "session-NEW"] + + def test_current_tab_meta_passes_attached_target_id(): """Regression for issue #304: helpers.current_tab() previously sent Target.getTargetInfo with no targetId. The daemon strips session_id for