diff --git a/mcp_servers/hummingbot_api/hummingbot_client.py b/mcp_servers/hummingbot_api/hummingbot_client.py index 1b224737f..139ec7f80 100644 --- a/mcp_servers/hummingbot_api/hummingbot_client.py +++ b/mcp_servers/hummingbot_api/hummingbot_client.py @@ -7,8 +7,10 @@ import os import platform import time +from contextlib import asynccontextmanager from typing import Any +import aiohttp from hummingbot_api_client import HummingbotAPIClient from mcp_servers.hummingbot_api.exceptions import MaxConnectionsAttemptError @@ -148,6 +150,25 @@ async def get_client(self) -> HummingbotAPIClient: return await self.initialize() return self._client + @asynccontextmanager + async def deployment_client(self): + """Allow slow bot creation without changing the shared client's timeout.""" + # The SDK has no per-call timeout. Use its public client lifecycle for + # this one POST; never retry a deployment whose outcome is unknown. + client = HummingbotAPIClient( + base_url=settings.api_url, + username=settings.api_username, + password=settings.api_password, + timeout=aiohttp.ClientTimeout( + total=max(120.0, settings.connection_timeout) + ), + ) + try: + await client.init() + yield client + finally: + await client.close() + async def close(self): """Close the client connection and reset state""" if self._client: diff --git a/mcp_servers/hummingbot_api/server.py b/mcp_servers/hummingbot_api/server.py index 740f48984..60302ba08 100644 --- a/mcp_servers/hummingbot_api/server.py +++ b/mcp_servers/hummingbot_api/server.py @@ -616,7 +616,9 @@ async def manage_bots( multi-strategy bots with centralized risk management. Actions: - - deploy: Deploy a new bot with controller configurations (requires bot_name + controllers_config) + - deploy: Deploy a new bot with controller configurations (requires bot_name + controllers_config). + Allows at least 120 seconds for creation. A timeout means the outcome is unknown; + reconcile this bot's status/config before attempting another deployment. - status: Get status of all active bots (no additional params needed) - logs: Get detailed logs for a specific bot (requires bot_name) - stop_bot: Stop and archive a bot forever (requires bot_name) @@ -645,25 +647,26 @@ async def manage_bots( config_data: New configuration data (required for update_config). Must include 'controller_type' and 'controller_name'. confirm_override: Required True if overwriting existing config in a running bot (update_config only). """ - client = await hummingbot_client.get_client() - if action == "deploy": if not bot_name: return "Error: 'bot_name' is required for deploy action" if not controllers_config: return "Error: 'controllers_config' is required for deploy action" - result = await controllers_tools.deploy_bot( - client=client, - bot_name=bot_name, - controllers_config=controllers_config, - account_name=account_name, - max_global_drawdown_quote=max_global_drawdown_quote, - max_controller_drawdown_quote=max_controller_drawdown_quote, - image=image, - ) + async with hummingbot_client.deployment_client() as client: + result = await controllers_tools.deploy_bot( + client=client, + bot_name=bot_name, + controllers_config=controllers_config, + account_name=account_name, + max_global_drawdown_quote=max_global_drawdown_quote, + max_controller_drawdown_quote=max_controller_drawdown_quote, + image=image, + ) return result["message"] - elif action == "status": + client = await hummingbot_client.get_client() + + if action == "status": result = await bot_management_tools.get_active_bots_status(client) return ( f"Active Bots Status Summary:\n" diff --git a/mcp_servers/hummingbot_api/tools/controllers.py b/mcp_servers/hummingbot_api/tools/controllers.py index 8343d3d23..90484fac7 100644 --- a/mcp_servers/hummingbot_api/tools/controllers.py +++ b/mcp_servers/hummingbot_api/tools/controllers.py @@ -8,6 +8,8 @@ import asyncio from typing import Any, Literal +from mcp_servers.hummingbot_api.exceptions import ToolError + # Internal/auto-managed fields that should be skipped during schema validation _SKIP_FIELDS = { "id", @@ -563,14 +565,23 @@ async def deploy_bot( Returns: Dictionary containing deployment results """ - result = await client.bot_orchestration.deploy_v2_controllers( - instance_name=bot_name, - controllers_config=controllers_config, - credentials_profile=account_name, - max_global_drawdown_quote=max_global_drawdown_quote, - max_controller_drawdown_quote=max_controller_drawdown_quote, - image=image, - ) + try: + result = await client.bot_orchestration.deploy_v2_controllers( + instance_name=bot_name, + controllers_config=controllers_config, + credentials_profile=account_name, + max_global_drawdown_quote=max_global_drawdown_quote, + max_controller_drawdown_quote=max_controller_drawdown_quote, + image=image, + ) + except TimeoutError as exc: + raise ToolError( + f"Deployment request for bot '{bot_name}' timed out ({type(exc).__name__}). " + "Outcome unknown: the API may have created the bot. " + "Reconcile this bot's status and controller configs " + f"{controllers_config!r} on account '{account_name}' before retrying. " + "No deployment retry was attempted." + ) from exc return { "bot_name": bot_name, diff --git a/tests/test_bot_deploy_timeout.py b/tests/test_bot_deploy_timeout.py new file mode 100644 index 000000000..1c615c7be --- /dev/null +++ b/tests/test_bot_deploy_timeout.py @@ -0,0 +1,184 @@ +"""Purpose: verify deployment timeouts against a local API, never a live bot.""" + +import asyncio +from unittest.mock import AsyncMock + +import aiohttp +import pytest +from aiohttp import web +from aiohttp.test_utils import TestServer +from hummingbot_api_client import HummingbotAPIClient + +import mcp_servers.hummingbot_api.hummingbot_client as client_module +import mcp_servers.hummingbot_api.server as server +from mcp_servers.hummingbot_api.exceptions import ToolError +from mcp_servers.hummingbot_api.settings import Settings + + +@pytest.mark.parametrize( + "ordinary_timeout,deploy_timeout", [(30.0, 120.0), (180.0, 180.0)] +) +def test_deployment_budget_does_not_change_shared_client( + monkeypatch, ordinary_timeout, deploy_timeout +): + monkeypatch.setattr( + client_module, "settings", Settings(connection_timeout=ordinary_timeout) + ) + clients = [] + + def make_client(**kwargs): + client = AsyncMock() + client.timeout = kwargs["timeout"] + clients.append(client) + return client + + monkeypatch.setattr(client_module, "HummingbotAPIClient", make_client) + + async def run(): + wrapper = client_module.HummingbotClient() + shared = await wrapper.get_client() + async with wrapper.deployment_client() as deployment: + assert deployment is not shared + assert deployment.timeout.total == deploy_timeout + assert shared.timeout.total == ordinary_timeout + assert await wrapper.get_client() is shared + deployment.close.assert_awaited_once() + shared.close.assert_not_awaited() + await wrapper.close() + + asyncio.run(run()) + assert len(clients) == 2 + + +@pytest.mark.parametrize("timeout", [False, True]) +def test_manage_bots_delayed_http_response_and_unknown_timeout(monkeypatch, timeout): + """Use the real SDK/HTTP path, scaling the budgets to keep this test fast.""" + + async def run(): + requests = [] + clients = [] + + async def deploy(request): + requests.append(await request.json()) + await asyncio.sleep(0.2) + return web.json_response({"success": True, "message": "deployed"}) + + async def accounts(request): + return web.json_response(["test_account"]) + + app = web.Application() + app.router.add_post("/bot-orchestration/deploy-v2-controllers", deploy) + app.router.add_get("/accounts/", accounts) + async with TestServer(app) as api: + monkeypatch.setattr( + client_module, + "settings", + Settings( + api_url=str(api.make_url("")), + api_username="test", + api_password="test", + connection_timeout=0.05, + ), + ) + + def make_client(**kwargs): + if kwargs["timeout"].total == 120.0: + kwargs["timeout"] = aiohttp.ClientTimeout( + total=0.05 if timeout else 1.0 + ) + client = HummingbotAPIClient(**kwargs) + client.close = AsyncMock(wraps=client.close) + clients.append(client) + return client + + monkeypatch.setattr(client_module, "HummingbotAPIClient", make_client) + wrapper = client_module.HummingbotClient() + monkeypatch.setattr(server, "hummingbot_client", wrapper) + shared = await wrapper.get_client() + try: + call = server.manage_bots( + action="deploy", + bot_name="test-bot", + controllers_config=["test-config"], + account_name="test_account", + max_global_drawdown_quote=10.0, + max_controller_drawdown_quote=5.0, + image="test-image", + ) + if timeout: + with pytest.raises(ToolError) as caught: + await call + message = str(caught.value) + for text in ( + "test-bot", + "TimeoutError", + "Outcome unknown", + "test-config", + "test_account", + "before retrying", + "No deployment retry was attempted", + ): + assert text in message + assert isinstance(caught.value.__cause__, TimeoutError) + else: + assert "deployed" in await call + assert len(clients) == 2 + clients[1].close.assert_awaited_once() + assert await wrapper.get_client() is shared + shared.close.assert_not_awaited() + assert shared.timeout.total == 0.05 + # Wait for the mock backend to finish even if the client timed out. + await asyncio.sleep(0.25) + assert requests == [ + { + "instance_name": "test-bot", + "credentials_profile": "test_account", + "controllers_config": ["test-config"], + "max_global_drawdown_quote": 10.0, + "max_controller_drawdown_quote": 5.0, + "image": "test-image", + } + ] + finally: + await wrapper.close() + + asyncio.run(run()) + + +@pytest.mark.parametrize("during_init", [True, False]) +def test_deployment_client_closes_on_init_failure_or_cancellation( + monkeypatch, during_init +): + client = AsyncMock() + monkeypatch.setattr(client_module, "HummingbotAPIClient", lambda **kwargs: client) + if during_init: + client.init.side_effect = RuntimeError("init failed") + + async def run(): + with pytest.raises(RuntimeError if during_init else asyncio.CancelledError): + async with client_module.HummingbotClient().deployment_client(): + raise asyncio.CancelledError() + client.close.assert_awaited_once() + + asyncio.run(run()) + + +def test_other_bot_actions_keep_shared_client(monkeypatch): + wrapper = AsyncMock() + monkeypatch.setattr(server, "hummingbot_client", wrapper) + status = AsyncMock(return_value={"total_bots": 0, "bots_table": "empty"}) + monkeypatch.setattr(server.bot_management_tools, "get_active_bots_status", status) + assert "Total Active Bots: 0" in asyncio.run(server.manage_bots(action="status")) + status.assert_awaited_once_with(wrapper.get_client.return_value) + wrapper.deployment_client.assert_not_called() + + +def test_invalid_deploy_does_not_open_a_client(monkeypatch): + wrapper = AsyncMock() + monkeypatch.setattr(server, "hummingbot_client", wrapper) + assert "bot_name" in asyncio.run(server.manage_bots(action="deploy")) + assert "controllers_config" in asyncio.run( + server.manage_bots(action="deploy", bot_name="test-bot") + ) + wrapper.get_client.assert_not_called() + wrapper.deployment_client.assert_not_called()