Fix api_timeout not being used in APIRemoteWorkspace (#1206)

Co-authored-by: openhands <openhands@all-hands.dev>

Author: xingyaoww

openhands-sdk/openhands/sdk/workspace/remote/async_remote_workspace.py

@@ -15,11 +15,32 @@ class AsyncRemoteWorkspace(RemoteWorkspaceMixin):
 
     _client: httpx.AsyncClient | None = PrivateAttr(default=None)
 
+    async def reset_client(self) -> None:
+        """Reset the HTTP client to force re-initialization.
+
+        This is useful when connection parameters (host, api_key) have changed
+        and the client needs to be recreated with new values.
+        """
+        if self._client is not None:
+            try:
+                await self._client.aclose()
+            except Exception:
+                pass
+        self._client = None
+
     @property
     def client(self) -> httpx.AsyncClient:
         client = self._client
         if client is None:
-            client = httpx.AsyncClient(base_url=self.host)
+            # Configure reasonable timeouts for HTTP requests
+            # - connect: 10 seconds to establish connection
+            # - read: 60 seconds to read response (for LLM operations)
+            # - write: 10 seconds to send request
+            # - pool: 10 seconds to get connection from pool
+            timeout = httpx.Timeout(connect=10.0, read=60.0, write=10.0, pool=10.0)
+            client = httpx.AsyncClient(
+                base_url=self.host, timeout=timeout, headers=self._headers
+            )
             self._client = client
         return client
 

openhands-sdk/openhands/sdk/workspace/remote/base.py

@@ -30,6 +30,19 @@ class RemoteWorkspace(RemoteWorkspaceMixin, BaseWorkspace):
 
     _client: httpx.Client | None = PrivateAttr(default=None)
 
+    def reset_client(self) -> None:
+        """Reset the HTTP client to force re-initialization.
+
+        This is useful when connection parameters (host, api_key) have changed
+        and the client needs to be recreated with new values.
+        """
+        if self._client is not None:
+            try:
+                self._client.close()
+            except Exception:
+                pass
+        self._client = None
+
     @property
     def client(self) -> httpx.Client:
         client = self._client

openhands-workspace/openhands/workspace/remote_api/workspace.py

@@ -79,6 +79,24 @@ class APIRemoteWorkspace(RemoteWorkspace):
     _runtime_url: str | None = PrivateAttr(default=None)
     _session_api_key: str | None = PrivateAttr(default=None)
 
+    @property
+    def client(self) -> httpx.Client:
+        """Override client property to use api_timeout for HTTP requests."""
+        client = self._client
+        if client is None:
+            # Use api_timeout for the read timeout to allow longer operations
+            timeout = httpx.Timeout(
+                connect=10.0,
+                read=self.api_timeout,
+                write=10.0,
+                pool=10.0,
+            )
+            client = httpx.Client(
+                base_url=self.host, timeout=timeout, headers=self._headers
+            )
+            self._client = client
+        return client
+
     @property
     def _api_headers(self):
         """Headers for runtime API requests."
@@ -120,8 +138,9 @@ def _start_or_attach_to_runtime(self) -> None:
         logger.info(f"Runtime ready at {self._runtime_url}")
         self.host = self._runtime_url.rstrip("/")
         self.api_key = self._session_api_key
-        self._client = None  # Reset HTTP client with new host and API key
-        _ = self.client  # Initialize client by accessing the property
+        # Reset HTTP client with new host and API key
+        self.reset_client()
+        # Verify client is properly initialized
         assert self.client is not None
         assert self.client.base_url == self.host
 

tests/workspace/test_api_remote_workspace.py

@@ -0,0 +1,107 @@
+"""Test APIRemoteWorkspace timeout configuration."""
+
+from unittest.mock import patch
+
+import httpx
+
+
+def test_api_timeout_is_used_in_client():
+    """Test that api_timeout parameter is used for the HTTP client timeout."""
+    from openhands.workspace import APIRemoteWorkspace
+
+    # Mock the entire initialization process
+    with patch.object(APIRemoteWorkspace, "_start_or_attach_to_runtime") as mock_init:
+        mock_init.return_value = None
+
+        # Create a workspace with custom api_timeout
+        custom_timeout = 300.0
+        workspace = APIRemoteWorkspace(
+            runtime_api_url="https://example.com",
+            runtime_api_key="test-key",
+            server_image="test-image",
+            api_timeout=custom_timeout,
+        )
+
+        # The runtime properties need to be set for client initialization
+        workspace._runtime_id = "test-runtime-id"
+        workspace._runtime_url = "https://test-runtime.com"
+        workspace._session_api_key = "test-session-key"
+        workspace.host = workspace._runtime_url
+
+        # Access the client property to trigger initialization
+        client = workspace.client
+
+        # Verify that the client's timeout uses the custom api_timeout
+        assert isinstance(client, httpx.Client)
+        assert client.timeout.read == custom_timeout
+        assert client.timeout.connect == 10.0
+        assert client.timeout.write == 10.0
+        assert client.timeout.pool == 10.0
+
+        # Clean up
+        workspace._runtime_id = None  # Prevent cleanup from trying to stop runtime
+        workspace.cleanup()
+
+
+def test_api_timeout_default_value():
+    """Test that the default api_timeout is 60 seconds."""
+    from openhands.workspace import APIRemoteWorkspace
+
+    with patch.object(APIRemoteWorkspace, "_start_or_attach_to_runtime") as mock_init:
+        mock_init.return_value = None
+
+        workspace = APIRemoteWorkspace(
+            runtime_api_url="https://example.com",
+            runtime_api_key="test-key",
+            server_image="test-image",
+        )
+
+        # The runtime properties need to be set for client initialization
+        workspace._runtime_id = "test-runtime-id"
+        workspace._runtime_url = "https://test-runtime.com"
+        workspace._session_api_key = "test-session-key"
+        workspace.host = workspace._runtime_url
+
+        # Access the client property to trigger initialization
+        client = workspace.client
+
+        # Verify default timeout is 60 seconds
+        assert client.timeout.read == 60.0
+
+        # Clean up
+        workspace._runtime_id = None
+        workspace.cleanup()
+
+
+def test_different_timeout_values():
+    """Test that different api_timeout values are correctly applied."""
+    from openhands.workspace import APIRemoteWorkspace
+
+    test_timeouts = [30.0, 120.0, 600.0]
+
+    for timeout_value in test_timeouts:
+        with patch.object(
+            APIRemoteWorkspace, "_start_or_attach_to_runtime"
+        ) as mock_init:
+            mock_init.return_value = None
+
+            workspace = APIRemoteWorkspace(
+                runtime_api_url="https://example.com",
+                runtime_api_key="test-key",
+                server_image="test-image",
+                api_timeout=timeout_value,
+            )
+
+            workspace._runtime_id = "test-runtime-id"
+            workspace._runtime_url = "https://test-runtime.com"
+            workspace._session_api_key = "test-session-key"
+            workspace.host = workspace._runtime_url
+
+            client = workspace.client
+
+            assert client.timeout.read == timeout_value, (
+                f"Expected timeout {timeout_value}, got {client.timeout.read}"
+            )
+
+            workspace._runtime_id = None
+            workspace.cleanup()