diff --git a/openhands-sdk/openhands/sdk/workspace/remote/async_remote_workspace.py b/openhands-sdk/openhands/sdk/workspace/remote/async_remote_workspace.py index 128277d9aa..3e72d54e42 100644 --- a/openhands-sdk/openhands/sdk/workspace/remote/async_remote_workspace.py +++ b/openhands-sdk/openhands/sdk/workspace/remote/async_remote_workspace.py @@ -15,11 +15,42 @@ class AsyncRemoteWorkspace(RemoteWorkspaceMixin): _client: httpx.AsyncClient | None = PrivateAttr(default=None) + def _create_timeout(self) -> httpx.Timeout: + """Create the timeout configuration for HTTP requests. + + Subclasses can override this method to customize timeout values. + Default configuration: + - 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 + + Returns: + httpx.Timeout: The timeout configuration + """ + return httpx.Timeout(connect=10.0, read=60.0, write=10.0, pool=10.0) + + 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) + timeout = self._create_timeout() + client = httpx.AsyncClient( + base_url=self.host, timeout=timeout, headers=self._headers + ) self._client = client return client diff --git a/openhands-sdk/openhands/sdk/workspace/remote/base.py b/openhands-sdk/openhands/sdk/workspace/remote/base.py index 0b9f340f4d..998b597b9c 100644 --- a/openhands-sdk/openhands/sdk/workspace/remote/base.py +++ b/openhands-sdk/openhands/sdk/workspace/remote/base.py @@ -30,16 +30,39 @@ class RemoteWorkspace(RemoteWorkspaceMixin, BaseWorkspace): _client: httpx.Client | None = PrivateAttr(default=None) + def _create_timeout(self) -> httpx.Timeout: + """Create the timeout configuration for HTTP requests. + + Subclasses can override this method to customize timeout values. + Default configuration: + - 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 + + Returns: + httpx.Timeout: The timeout configuration + """ + return httpx.Timeout(connect=10.0, read=60.0, write=10.0, pool=10.0) + + 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 if client is None: - # 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) + timeout = self._create_timeout() client = httpx.Client( base_url=self.host, timeout=timeout, headers=self._headers ) diff --git a/openhands-workspace/openhands/workspace/remote_api/workspace.py b/openhands-workspace/openhands/workspace/remote_api/workspace.py index ea208e6706..b365956bcd 100644 --- a/openhands-workspace/openhands/workspace/remote_api/workspace.py +++ b/openhands-workspace/openhands/workspace/remote_api/workspace.py @@ -79,6 +79,19 @@ class APIRemoteWorkspace(RemoteWorkspace): _runtime_url: str | None = PrivateAttr(default=None) _session_api_key: str | None = PrivateAttr(default=None) + def _create_timeout(self) -> httpx.Timeout: + """Create timeout configuration using the api_timeout field. + + Uses api_timeout for the read timeout to allow longer operations + while keeping other timeouts at reasonable defaults. + """ + return httpx.Timeout( + connect=10.0, + read=self.api_timeout, + write=10.0, + pool=10.0, + ) + @property def _api_headers(self): """Headers for runtime API requests." @@ -120,8 +133,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 diff --git a/tests/workspace/test_api_remote_workspace.py b/tests/workspace/test_api_remote_workspace.py new file mode 100644 index 0000000000..443f18ff8b --- /dev/null +++ b/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()