From c9fb075abd7638d658bc9024d82705468efe8873 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 2 Jan 2026 10:20:38 +0100 Subject: [PATCH 1/7] Fix remote run polling on terminal errors --- .../conversation/impl/remote_conversation.py | 37 +++++++- .../remote/test_remote_conversation.py | 89 +++++++++++++++++++ 2 files changed, 122 insertions(+), 4 deletions(-) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index b8d073da76..c104a77b40 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -741,7 +741,9 @@ def _wait_for_run_completion( timeout: Maximum time in seconds to wait. Raises: - ConversationRunError: If the wait times out. + ConversationRunError: If the run fails, the conversation disappears, + or the wait times out. Transient network errors, 429s, and 5xx + responses are retried until timeout. """ start_time = time.monotonic() @@ -785,10 +787,37 @@ def _wait_for_run_completion( ) return - except Exception as e: - # Log but continue polling - transient network errors shouldn't - # stop us from waiting for the run to complete + except ConversationRunError: + raise + except httpx.HTTPStatusError as e: + status_code = e.response.status_code + reason = e.response.reason_phrase + if status_code == 404: + raise ConversationRunError( + self._id, + RuntimeError( + "Remote conversation not found (404). " + "The runtime may have been deleted." + ), + ) from e + if 400 <= status_code < 500 and status_code != 429: + raise ConversationRunError( + self._id, + RuntimeError( + f"Polling failed with HTTP {status_code} {reason}" + ), + ) from e + # Retry on 429s and 5xx errors + logger.warning( + "Error polling status (will retry): HTTP %d %s", + status_code, + reason, + ) + except httpx.RequestError as e: + # Transient network errors shouldn't stop us from waiting logger.warning(f"Error polling status (will retry): {e}") + except Exception as e: + raise ConversationRunError(self._id, e) from e time.sleep(poll_interval) diff --git a/tests/sdk/conversation/remote/test_remote_conversation.py b/tests/sdk/conversation/remote/test_remote_conversation.py index 40c1c70962..56a81dc85b 100644 --- a/tests/sdk/conversation/remote/test_remote_conversation.py +++ b/tests/sdk/conversation/remote/test_remote_conversation.py @@ -8,6 +8,7 @@ from pydantic import SecretStr from openhands.sdk.agent import Agent +from openhands.sdk.conversation.exceptions import ConversationRunError from openhands.sdk.conversation.impl.remote_conversation import RemoteConversation from openhands.sdk.conversation.secret_registry import SecretValue from openhands.sdk.conversation.visualizer import DefaultConversationVisualizer @@ -461,6 +462,94 @@ def custom_side_effect(method, url, **kwargs): f"Should have polled 3 times (2 running + 1 finished), got {poll_count[0]}" ) + @patch( + "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient" + ) + def test_remote_conversation_run_error_status_raises(self, mock_ws_client): + """Test that error status raises ConversationRunError.""" + conversation_id = str(uuid.uuid4()) + mock_client_instance = self.setup_mock_client(conversation_id=conversation_id) + + original_side_effect = mock_client_instance.request.side_effect + + def custom_side_effect(method, url, **kwargs): + if method == "GET" and url == f"/api/conversations/{conversation_id}": + response = Mock() + response.raise_for_status.return_value = None + response.json.return_value = { + "id": conversation_id, + "execution_status": "error", + } + return response + return original_side_effect(method, url, **kwargs) + + mock_client_instance.request.side_effect = custom_side_effect + + mock_ws_instance = Mock() + mock_ws_client.return_value = mock_ws_instance + + conversation = RemoteConversation(agent=self.agent, workspace=self.workspace) + with pytest.raises(ConversationRunError) as exc_info: + conversation.run(poll_interval=0.01) + assert "error" in str(exc_info.value).lower() + + @patch( + "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient" + ) + def test_remote_conversation_run_stuck_status_raises(self, mock_ws_client): + """Test that stuck status raises ConversationRunError.""" + conversation_id = str(uuid.uuid4()) + mock_client_instance = self.setup_mock_client(conversation_id=conversation_id) + + original_side_effect = mock_client_instance.request.side_effect + + def custom_side_effect(method, url, **kwargs): + if method == "GET" and url == f"/api/conversations/{conversation_id}": + response = Mock() + response.raise_for_status.return_value = None + response.json.return_value = { + "id": conversation_id, + "execution_status": "stuck", + } + return response + return original_side_effect(method, url, **kwargs) + + mock_client_instance.request.side_effect = custom_side_effect + + mock_ws_instance = Mock() + mock_ws_client.return_value = mock_ws_instance + + conversation = RemoteConversation(agent=self.agent, workspace=self.workspace) + with pytest.raises(ConversationRunError) as exc_info: + conversation.run(poll_interval=0.01) + assert "stuck" in str(exc_info.value).lower() + + @patch( + "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient" + ) + def test_remote_conversation_run_404_raises(self, mock_ws_client): + """Test that 404s during polling raise ConversationRunError.""" + conversation_id = str(uuid.uuid4()) + mock_client_instance = self.setup_mock_client(conversation_id=conversation_id) + + original_side_effect = mock_client_instance.request.side_effect + + def custom_side_effect(method, url, **kwargs): + if method == "GET" and url == f"/api/conversations/{conversation_id}": + request = httpx.Request("GET", f"http://localhost{url}") + return httpx.Response(404, request=request, text="Not Found") + return original_side_effect(method, url, **kwargs) + + mock_client_instance.request.side_effect = custom_side_effect + + mock_ws_instance = Mock() + mock_ws_client.return_value = mock_ws_instance + + conversation = RemoteConversation(agent=self.agent, workspace=self.workspace) + with pytest.raises(ConversationRunError) as exc_info: + conversation.run(poll_interval=0.01) + assert "not found" in str(exc_info.value).lower() + @patch( "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient" ) From d67a8646630bdb432f82afa2bb3c4e2027b51cfb Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 2 Jan 2026 11:02:27 +0100 Subject: [PATCH 2/7] Refactor remote run polling --- .../conversation/impl/remote_conversation.py | 119 +++++++++--------- 1 file changed, 61 insertions(+), 58 deletions(-) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index c104a77b40..886f8a6832 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -759,68 +759,71 @@ def _wait_for_run_completion( ) try: - resp = _send_request( - self._client, - "GET", - f"/api/conversations/{self._id}", - timeout=30, - ) - info = resp.json() - status = info.get("execution_status") - - if status != ConversationExecutionStatus.RUNNING.value: - if status == ConversationExecutionStatus.ERROR.value: - detail = self._get_last_error_detail() - raise ConversationRunError( - self._id, - RuntimeError( - detail or "Remote conversation ended with error" - ), - ) - if status == ConversationExecutionStatus.STUCK.value: - raise ConversationRunError( - self._id, - RuntimeError("Remote conversation got stuck"), - ) - logger.info( - f"Run completed with status: {status} (elapsed: {elapsed:.1f}s)" - ) + status = self._poll_status_once() + if self._handle_terminal_status(status, elapsed): return - - except ConversationRunError: - raise - except httpx.HTTPStatusError as e: - status_code = e.response.status_code - reason = e.response.reason_phrase - if status_code == 404: - raise ConversationRunError( - self._id, - RuntimeError( - "Remote conversation not found (404). " - "The runtime may have been deleted." - ), - ) from e - if 400 <= status_code < 500 and status_code != 429: - raise ConversationRunError( - self._id, - RuntimeError( - f"Polling failed with HTTP {status_code} {reason}" - ), - ) from e - # Retry on 429s and 5xx errors - logger.warning( - "Error polling status (will retry): HTTP %d %s", - status_code, - reason, - ) - except httpx.RequestError as e: - # Transient network errors shouldn't stop us from waiting - logger.warning(f"Error polling status (will retry): {e}") - except Exception as e: - raise ConversationRunError(self._id, e) from e + except Exception as exc: + self._handle_poll_exception(exc) time.sleep(poll_interval) + def _poll_status_once(self) -> str | None: + resp = _send_request( + self._client, + "GET", + f"/api/conversations/{self._id}", + timeout=30, + ) + info = resp.json() + return info.get("execution_status") + + def _handle_terminal_status(self, status: str | None, elapsed: float) -> bool: + if status == ConversationExecutionStatus.RUNNING.value: + return False + if status == ConversationExecutionStatus.ERROR.value: + detail = self._get_last_error_detail() + raise ConversationRunError( + self._id, + RuntimeError(detail or "Remote conversation ended with error"), + ) + if status == ConversationExecutionStatus.STUCK.value: + raise ConversationRunError( + self._id, + RuntimeError("Remote conversation got stuck"), + ) + logger.info(f"Run completed with status: {status} (elapsed: {elapsed:.1f}s)") + return True + + def _handle_poll_exception(self, exc: Exception) -> None: + if isinstance(exc, ConversationRunError): + raise + if isinstance(exc, httpx.HTTPStatusError): + status_code = exc.response.status_code + reason = exc.response.reason_phrase + if status_code == 404: + raise ConversationRunError( + self._id, + RuntimeError( + "Remote conversation not found (404). " + "The runtime may have been deleted." + ), + ) from exc + if 400 <= status_code < 500 and status_code != 429: + raise ConversationRunError( + self._id, + RuntimeError(f"Polling failed with HTTP {status_code} {reason}"), + ) from exc + logger.warning( + "Error polling status (will retry): HTTP %d %s", + status_code, + reason, + ) + return + if isinstance(exc, httpx.RequestError): + logger.warning(f"Error polling status (will retry): {exc}") + return + raise ConversationRunError(self._id, exc) from exc + def _get_last_error_detail(self) -> str | None: """Return the most recent ConversationErrorEvent detail, if available.""" try: From e83fe452bbf9b094e2c9550b6f5de5daa54e8599 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 2 Jan 2026 11:05:11 +0100 Subject: [PATCH 3/7] Document remote polling helpers --- .../openhands/sdk/conversation/impl/remote_conversation.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index 886f8a6832..a56cf5b7a7 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -768,6 +768,7 @@ def _wait_for_run_completion( time.sleep(poll_interval) def _poll_status_once(self) -> str | None: + """Fetch the current execution status from the remote conversation.""" resp = _send_request( self._client, "GET", @@ -778,6 +779,7 @@ def _poll_status_once(self) -> str | None: return info.get("execution_status") def _handle_terminal_status(self, status: str | None, elapsed: float) -> bool: + """Handle non-running statuses; return True if the run is complete.""" if status == ConversationExecutionStatus.RUNNING.value: return False if status == ConversationExecutionStatus.ERROR.value: @@ -795,6 +797,7 @@ def _handle_terminal_status(self, status: str | None, elapsed: float) -> bool: return True def _handle_poll_exception(self, exc: Exception) -> None: + """Classify polling exceptions into retryable vs terminal failures.""" if isinstance(exc, ConversationRunError): raise if isinstance(exc, httpx.HTTPStatusError): From 3b7e0a29cc17ac830564a2b0a1b3638d30e6b3f4 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 2 Jan 2026 18:57:36 +0100 Subject: [PATCH 4/7] Align remote stuck handling with local --- .../sdk/conversation/impl/remote_conversation.py | 11 +++++++---- .../conversation/remote/test_remote_conversation.py | 8 +++----- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index c2d19ef8c8..2e7646a83f 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -739,7 +739,8 @@ def _wait_for_run_completion( Raises: ConversationRunError: If the run fails, the conversation disappears, or the wait times out. Transient network errors, 429s, and 5xx - responses are retried until timeout. + responses are retried until timeout. A STUCK status is treated + as a terminal completion (matching local behavior). """ start_time = time.monotonic() @@ -785,10 +786,12 @@ def _handle_terminal_status(self, status: str | None, elapsed: float) -> bool: RuntimeError(detail or "Remote conversation ended with error"), ) if status == ConversationExecutionStatus.STUCK.value: - raise ConversationRunError( - self._id, - RuntimeError("Remote conversation got stuck"), + logger.warning( + "Run completed with status: %s (elapsed: %.1fs)", + status, + elapsed, ) + return True logger.info(f"Run completed with status: {status} (elapsed: {elapsed:.1f}s)") return True diff --git a/tests/sdk/conversation/remote/test_remote_conversation.py b/tests/sdk/conversation/remote/test_remote_conversation.py index 56a81dc85b..90e217f89c 100644 --- a/tests/sdk/conversation/remote/test_remote_conversation.py +++ b/tests/sdk/conversation/remote/test_remote_conversation.py @@ -496,8 +496,8 @@ def custom_side_effect(method, url, **kwargs): @patch( "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient" ) - def test_remote_conversation_run_stuck_status_raises(self, mock_ws_client): - """Test that stuck status raises ConversationRunError.""" + def test_remote_conversation_run_stuck_status_returns(self, mock_ws_client): + """Test that stuck status returns without raising.""" conversation_id = str(uuid.uuid4()) mock_client_instance = self.setup_mock_client(conversation_id=conversation_id) @@ -520,9 +520,7 @@ def custom_side_effect(method, url, **kwargs): mock_ws_client.return_value = mock_ws_instance conversation = RemoteConversation(agent=self.agent, workspace=self.workspace) - with pytest.raises(ConversationRunError) as exc_info: - conversation.run(poll_interval=0.01) - assert "stuck" in str(exc_info.value).lower() + conversation.run(poll_interval=0.01) @patch( "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient" From b475af1e428167e21f7020934a4418c270c76186 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 2 Jan 2026 20:18:50 +0100 Subject: [PATCH 5/7] Revert "Align remote stuck handling with local" This reverts commit 3b7e0a29cc17ac830564a2b0a1b3638d30e6b3f4. --- .../sdk/conversation/impl/remote_conversation.py | 11 ++++------- .../conversation/remote/test_remote_conversation.py | 8 +++++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index 2e7646a83f..c2d19ef8c8 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -739,8 +739,7 @@ def _wait_for_run_completion( Raises: ConversationRunError: If the run fails, the conversation disappears, or the wait times out. Transient network errors, 429s, and 5xx - responses are retried until timeout. A STUCK status is treated - as a terminal completion (matching local behavior). + responses are retried until timeout. """ start_time = time.monotonic() @@ -786,12 +785,10 @@ def _handle_terminal_status(self, status: str | None, elapsed: float) -> bool: RuntimeError(detail or "Remote conversation ended with error"), ) if status == ConversationExecutionStatus.STUCK.value: - logger.warning( - "Run completed with status: %s (elapsed: %.1fs)", - status, - elapsed, + raise ConversationRunError( + self._id, + RuntimeError("Remote conversation got stuck"), ) - return True logger.info(f"Run completed with status: {status} (elapsed: {elapsed:.1f}s)") return True diff --git a/tests/sdk/conversation/remote/test_remote_conversation.py b/tests/sdk/conversation/remote/test_remote_conversation.py index 90e217f89c..56a81dc85b 100644 --- a/tests/sdk/conversation/remote/test_remote_conversation.py +++ b/tests/sdk/conversation/remote/test_remote_conversation.py @@ -496,8 +496,8 @@ def custom_side_effect(method, url, **kwargs): @patch( "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient" ) - def test_remote_conversation_run_stuck_status_returns(self, mock_ws_client): - """Test that stuck status returns without raising.""" + def test_remote_conversation_run_stuck_status_raises(self, mock_ws_client): + """Test that stuck status raises ConversationRunError.""" conversation_id = str(uuid.uuid4()) mock_client_instance = self.setup_mock_client(conversation_id=conversation_id) @@ -520,7 +520,9 @@ def custom_side_effect(method, url, **kwargs): mock_ws_client.return_value = mock_ws_instance conversation = RemoteConversation(agent=self.agent, workspace=self.workspace) - conversation.run(poll_interval=0.01) + with pytest.raises(ConversationRunError) as exc_info: + conversation.run(poll_interval=0.01) + assert "stuck" in str(exc_info.value).lower() @patch( "openhands.sdk.conversation.impl.remote_conversation.WebSocketCallbackClient" From 84b592f9d7efd72ab8b1b6188f68d97988ad5ce2 Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 2 Jan 2026 20:26:08 +0100 Subject: [PATCH 6/7] Simplify remote polling exception flow --- .../openhands/sdk/conversation/impl/remote_conversation.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index c2d19ef8c8..c374555407 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -756,10 +756,11 @@ def _wait_for_run_completion( try: status = self._poll_status_once() - if self._handle_terminal_status(status, elapsed): - return except Exception as exc: self._handle_poll_exception(exc) + else: + if self._handle_terminal_status(status, elapsed): + return time.sleep(poll_interval) @@ -794,8 +795,6 @@ def _handle_terminal_status(self, status: str | None, elapsed: float) -> bool: def _handle_poll_exception(self, exc: Exception) -> None: """Classify polling exceptions into retryable vs terminal failures.""" - if isinstance(exc, ConversationRunError): - raise if isinstance(exc, httpx.HTTPStatusError): status_code = exc.response.status_code reason = exc.response.reason_phrase From 89c3139cf1094460783b0062396b5c17d49f564d Mon Sep 17 00:00:00 2001 From: Simon Rosenberg Date: Fri, 2 Jan 2026 20:28:37 +0100 Subject: [PATCH 7/7] Simplify status handling --- .../sdk/conversation/impl/remote_conversation.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py index c374555407..7fc3a5fa8c 100644 --- a/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py +++ b/openhands-sdk/openhands/sdk/conversation/impl/remote_conversation.py @@ -759,7 +759,12 @@ def _wait_for_run_completion( except Exception as exc: self._handle_poll_exception(exc) else: - if self._handle_terminal_status(status, elapsed): + if self._handle_conversation_status(status): + logger.info( + "Run completed with status: %s (elapsed: %.1fs)", + status, + elapsed, + ) return time.sleep(poll_interval) @@ -775,7 +780,7 @@ def _poll_status_once(self) -> str | None: info = resp.json() return info.get("execution_status") - def _handle_terminal_status(self, status: str | None, elapsed: float) -> bool: + def _handle_conversation_status(self, status: str | None) -> bool: """Handle non-running statuses; return True if the run is complete.""" if status == ConversationExecutionStatus.RUNNING.value: return False @@ -790,7 +795,6 @@ def _handle_terminal_status(self, status: str | None, elapsed: float) -> bool: self._id, RuntimeError("Remote conversation got stuck"), ) - logger.info(f"Run completed with status: {status} (elapsed: {elapsed:.1f}s)") return True def _handle_poll_exception(self, exc: Exception) -> None: