From db0bdea3d0878119c6d41465d7eb03f4fbe16084 Mon Sep 17 00:00:00 2001 From: masnwilliams <43387599+masnwilliams@users.noreply.github.com> Date: Tue, 5 May 2026 01:34:19 +0000 Subject: [PATCH] fix(cua): make session.stop() idempotent on repeated call After PR #155 reset session state inside the finally, a follow-up stop() from the caller's error path reliably hit the throwing 'sessionId' getter via 'this.info' / 'self.info' (info accesses sessionId, which raises 'Session not started' when _sessionId is null). The exception masked the original error. stop() now short-circuits with a snapshot of the cleared state when no session is active, and builds the live-session info from local fields without going through the throwing getter. Applied symmetrically in TS and Python. Co-Authored-By: Claude Opus 4.7 --- pkg/templates/python/cua/session.py | 60 ++++++++++++++++--------- pkg/templates/typescript/cua/session.ts | 58 +++++++++++++++--------- 2 files changed, 77 insertions(+), 41 deletions(-) diff --git a/pkg/templates/python/cua/session.py b/pkg/templates/python/cua/session.py index 9aadbf7..1b5d85b 100644 --- a/pkg/templates/python/cua/session.py +++ b/pkg/templates/python/cua/session.py @@ -111,28 +111,46 @@ async def start(self) -> SessionInfo: return self.info async def stop(self) -> SessionInfo: - info = self.info + session_id = self._session_id + if not session_id: + # Already stopped — return a snapshot of the (cleared) state without + # touching `self.info`, whose `session_id` property raises on None. + return SessionInfo( + session_id="", + live_view_url=self._live_view_url or "", + replay_id=self._replay_id, + replay_view_url=self._replay_view_url, + viewport_width=self.opts.viewport_width, + viewport_height=self.opts.viewport_height, + ) + + info = SessionInfo( + session_id=session_id, + live_view_url=self._live_view_url or "", + replay_id=self._replay_id, + replay_view_url=self._replay_view_url, + viewport_width=self.opts.viewport_width, + viewport_height=self.opts.viewport_height, + ) - if self._session_id: - session_id = self._session_id - try: - if self.opts.record_replay and self._replay_id: - if self.opts.replay_grace_period > 0: - await asyncio.sleep(self.opts.replay_grace_period) - await self._stop_replay() - info.replay_view_url = self._replay_view_url - finally: - # Reset state up front so that if browser deletion or a thrown replay - # error propagates, a follow-up stop() call from the caller's error path - # is a no-op instead of attempting to delete the same session twice. - self._session_id = None - self._live_view_url = None - self._replay_id = None - self._replay_view_url = None - print(f"Destroying browser session: {session_id}") - await asyncio.to_thread( - self.kernel.browsers.delete_by_id, session_id, - ) + try: + if self.opts.record_replay and self._replay_id: + if self.opts.replay_grace_period > 0: + await asyncio.sleep(self.opts.replay_grace_period) + await self._stop_replay() + info.replay_view_url = self._replay_view_url + finally: + # Reset state up front so that if browser deletion or a thrown replay + # error propagates, a follow-up stop() call from the caller's error path + # is a no-op instead of attempting to delete the same session twice. + self._session_id = None + self._live_view_url = None + self._replay_id = None + self._replay_view_url = None + print(f"Destroying browser session: {session_id}") + await asyncio.to_thread( + self.kernel.browsers.delete_by_id, session_id, + ) return info diff --git a/pkg/templates/typescript/cua/session.ts b/pkg/templates/typescript/cua/session.ts index 8b34b60..b2ebd63 100644 --- a/pkg/templates/typescript/cua/session.ts +++ b/pkg/templates/typescript/cua/session.ts @@ -103,29 +103,47 @@ export class KernelBrowserSession { } async stop(): Promise { - const info = this.info; + const sessionId = this._sessionId; + if (!sessionId) { + // Already stopped — return a snapshot of the (cleared) state without + // touching `this.info`, whose `sessionId` getter throws on null. + return { + sessionId: '', + liveViewUrl: this._liveViewUrl ?? '', + replayId: this._replayId ?? undefined, + replayViewUrl: this._replayViewUrl ?? undefined, + viewportWidth: this.opts.viewportWidth, + viewportHeight: this.opts.viewportHeight, + }; + } - if (this._sessionId) { - const sessionId = this._sessionId; - try { - if (this.opts.recordReplay && this._replayId) { - if (this.opts.replayGracePeriod > 0) { - await sleep(this.opts.replayGracePeriod * 1000); - } - await this.stopReplay(); - info.replayViewUrl = this._replayViewUrl || undefined; + const info: SessionInfo = { + sessionId, + liveViewUrl: this._liveViewUrl ?? '', + replayId: this._replayId ?? undefined, + replayViewUrl: this._replayViewUrl ?? undefined, + viewportWidth: this.opts.viewportWidth, + viewportHeight: this.opts.viewportHeight, + }; + + try { + if (this.opts.recordReplay && this._replayId) { + if (this.opts.replayGracePeriod > 0) { + await sleep(this.opts.replayGracePeriod * 1000); } - } finally { - // Reset state up front so that if browser deletion or a thrown replay error - // propagates, a follow-up stop() call from the caller's error path is a no-op - // instead of attempting to delete the same session twice. - this._sessionId = null; - this._liveViewUrl = null; - this._replayId = null; - this._replayViewUrl = null; - console.log(`Destroying browser session: ${sessionId}`); - await this.kernel.browsers.deleteByID(sessionId); + await this.stopReplay(); + info.replayViewUrl = this._replayViewUrl || undefined; } + } finally { + // Reset state up front so that if browser deletion or a thrown replay error + // propagates, a follow-up stop() call from the caller's error path is a no-op + // instead of attempting to delete the same session twice. + this._sessionId = null; + this._liveViewUrl = null; + this._replayId = null; + this._replayViewUrl = null; + console.log(`Destroying browser session: ${sessionId}`); + await this.kernel.browsers.deleteByID(sessionId); } return info;