feat: Add configurable timeouts for MCP tool invocations (Good first issues: #10684) (#11029)

Co-authored-by: Tejas Goyal <tejas@Tejass-MacBook-Pro.local>
This commit is contained in:
Tejas Goyal 2025-09-24 08:43:54 -04:00 committed by GitHub
parent 73eb53a379
commit 16004426a2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 391 additions and 7 deletions

View File

@ -489,6 +489,47 @@ type = "noop"
# Run the runtime sandbox container in privileged mode for use with docker-in-docker
#privileged = false
#################################### MCP #####################################
# Configuration for Model Context Protocol (MCP) servers
# MCP allows OpenHands to communicate with external tool servers
##############################################################################
[mcp]
# SSE servers - Server-Sent Events transport (legacy)
#sse_servers = [
# # Basic SSE server with just a URL
# "http://localhost:8080/mcp/sse",
#
# # SSE server with authentication
# {url = "https://api.example.com/mcp/sse", api_key = "your-api-key"}
#]
# SHTTP servers - Streamable HTTP transport (recommended)
#shttp_servers = [
# # Basic SHTTP server with default 60s timeout
# "https://api.example.com/mcp/shttp",
#
# # SHTTP server with custom timeout for long-running tools
# {
# url = "https://api.example.com/mcp/shttp",
# api_key = "your-api-key",
# timeout = 180 # 3 minutes for processing-heavy tools (1-3600 seconds)
# }
#]
# Stdio servers - Direct process communication (development only)
#stdio_servers = [
# # Basic stdio server
# {name = "filesystem", command = "npx", args = ["@modelcontextprotocol/server-filesystem", "/"]},
#
# # Stdio server with environment variables
# {
# name = "fetch",
# command = "uvx",
# args = ["mcp-server-fetch"],
# env = {DEBUG = "true"}
# }
#]
#################################### Model Routing ############################
# Configuration for experimental model routing feature
# Enables intelligent switching between different LLM models for specific purposes

View File

@ -67,6 +67,19 @@ sse_servers = [
# External MCP service with authentication
{url="https://api.example.com/mcp/sse", api_key="your-api-key"}
]
# SHTTP Servers - Modern streamable HTTP transport (recommended)
shttp_servers = [
# Basic SHTTP server with default 60s timeout
"https://api.example.com/mcp/shttp",
# Server with custom timeout for heavy operations
{
url = "https://files.example.com/mcp/shttp",
api_key = "your-api-key",
timeout = 1800 # 30 minutes for large file processing
}
]
```
@ -118,6 +131,17 @@ SHTTP (Streamable HTTP) servers are configured using either a string URL or an o
- Type: `str`
- Description: API key for authentication
- `timeout` (optional)
- Type: `int`
- Default: `60`
- Range: `1-3600` seconds (1 hour maximum)
- Description: Timeout in seconds for tool execution. This prevents tool calls from hanging indefinitely.
- **Use Cases:**
- **Short timeout (1-30s)**: For lightweight operations like status checks or simple queries
- **Medium timeout (30-300s)**: For standard processing tasks like data analysis or API calls
- **Long timeout (300-3600s)**: For heavy operations like file processing, complex calculations, or batch operations
- **Note**: This timeout only applies to individual tool calls, not server connection establishment.
### Stdio Servers
**Note**: While stdio servers are supported, we recommend using MCP proxies (see above) for better reliability and performance.
@ -192,5 +216,27 @@ SHTTP is the modern HTTP-based transport protocol that provides enhanced feature
SHTTP is the recommended transport for HTTP-based MCP servers as it provides better reliability and features compared to the legacy SSE transport.
#### SHTTP Timeout Best Practices
When configuring SHTTP timeouts, consider these guidelines:
**Timeout Selection:**
- **Database queries**: 30-60 seconds
- **File operations**: 60-300 seconds (depending on file size)
- **Web scraping**: 60-120 seconds
- **Complex calculations**: 300-1800 seconds
- **Batch processing**: 1800-3600 seconds (maximum)
**Error Handling:**
When a tool call exceeds the configured timeout:
- The operation is cancelled with an `asyncio.TimeoutError`
- The agent receives a timeout error message
- The server connection remains active for subsequent requests
**Monitoring:**
- Set timeouts based on your tool's actual performance characteristics
- Monitor timeout occurrences to optimize timeout values
- Consider implementing server-side timeout handling for graceful degradation
### Standard Input/Output (stdio)
Stdio transport enables communication through standard input and output streams, making it ideal for local integrations and command-line tools. This transport is used for locally executed MCP servers that run as separate processes.

View File

@ -7,7 +7,7 @@
* - Please do NOT modify this file.
*/
const PACKAGE_VERSION = '2.10.5'
const PACKAGE_VERSION = '2.11.1'
const INTEGRITY_CHECKSUM = 'f5825c521429caf22a4dd13b66e243af'
const IS_MOCKED_RESPONSE = Symbol('isMockedResponse')
const activeClientIds = new Set()

View File

@ -15,6 +15,7 @@ interface MCPServerConfig {
name?: string;
url?: string;
api_key?: string;
timeout?: number;
command?: string;
args?: string[];
env?: Record<string, string>;
@ -120,6 +121,22 @@ export function MCPServerForm({
return null;
};
const validateTimeout = (timeoutStr: string): string | null => {
if (!timeoutStr.trim()) return null; // Optional field
const timeout = parseInt(timeoutStr.trim(), 10);
if (Number.isNaN(timeout)) {
return t(I18nKey.SETTINGS$MCP_ERROR_TIMEOUT_INVALID_NUMBER);
}
if (timeout <= 0) {
return t(I18nKey.SETTINGS$MCP_ERROR_TIMEOUT_POSITIVE);
}
if (timeout > 3600) {
return t(I18nKey.SETTINGS$MCP_ERROR_TIMEOUT_MAX_EXCEEDED);
}
return null;
};
const validateStdioServer = (formData: FormData): string | null => {
const name = formData.get("name")?.toString().trim() || "";
const command = formData.get("command")?.toString().trim() || "";
@ -148,6 +165,14 @@ export function MCPServerForm({
if (urlError) return urlError;
const urlDupError = validateUrlUniqueness(url);
if (urlDupError) return urlDupError;
// Validate timeout for SHTTP servers only
if (serverType === "shttp") {
const timeoutStr = formData.get("timeout")?.toString() || "";
const timeoutError = validateTimeout(timeoutStr);
if (timeoutError) return timeoutError;
}
return null;
}
@ -203,12 +228,23 @@ export function MCPServerForm({
if (serverType === "sse" || serverType === "shttp") {
const url = formData.get("url")?.toString().trim();
const apiKey = formData.get("api_key")?.toString().trim();
const timeoutStr = formData.get("timeout")?.toString().trim();
onSubmit({
const serverConfig: MCPServerConfig = {
...baseConfig,
url: url!,
...(apiKey && { api_key: apiKey }),
});
};
// Only add timeout for SHTTP servers
if (serverType === "shttp" && timeoutStr) {
const timeoutValue = parseInt(timeoutStr, 10);
if (!Number.isNaN(timeoutValue)) {
serverConfig.timeout = timeoutValue;
}
}
onSubmit(serverConfig);
} else if (serverType === "stdio") {
const name = formData.get("name")?.toString().trim();
const command = formData.get("command")?.toString().trim();
@ -283,6 +319,21 @@ export function MCPServerForm({
defaultValue={server?.api_key || ""}
placeholder={t(I18nKey.SETTINGS$MCP_API_KEY_PLACEHOLDER)}
/>
{serverType === "shttp" && (
<SettingsInput
testId="timeout-input"
name="timeout"
type="number"
label="Timeout (seconds)"
className="w-full max-w-[680px]"
showOptionalTag
defaultValue={server?.timeout?.toString() || ""}
placeholder="60"
min={1}
max={3600}
/>
)}
</>
)}

View File

@ -8,6 +8,7 @@ interface MCPServerConfig {
name?: string;
url?: string;
api_key?: string;
timeout?: number;
command?: string;
args?: string[];
env?: Record<string, string>;

View File

@ -8,6 +8,7 @@ interface MCPServerConfig {
name?: string;
url?: string;
api_key?: string;
timeout?: number;
command?: string;
args?: string[];
env?: Record<string, string>;

View File

@ -10,6 +10,7 @@ interface MCPServerConfig {
name?: string;
url?: string;
api_key?: string;
timeout?: number;
command?: string;
args?: string[];
env?: Record<string, string>;
@ -49,6 +50,7 @@ export function useAddMcpServer() {
const shttpServer: MCPSHTTPServer = {
url: server.url!,
...(server.api_key && { api_key: server.api_key }),
...(server.timeout !== undefined && { timeout: server.timeout }),
};
newConfig.shttp_servers.push(shttpServer);
}

View File

@ -10,6 +10,7 @@ interface MCPServerConfig {
name?: string;
url?: string;
api_key?: string;
timeout?: number;
command?: string;
args?: string[];
env?: Record<string, string>;
@ -51,6 +52,7 @@ export function useUpdateMcpServer() {
const shttpServer: MCPSHTTPServer = {
url: server.url!,
...(server.api_key && { api_key: server.api_key }),
...(server.timeout !== undefined && { timeout: server.timeout }),
};
newConfig.shttp_servers[index] = shttpServer;
}

View File

@ -806,6 +806,9 @@ export enum I18nKey {
SETTINGS$MCP_ERROR_COMMAND_NO_SPACES = "SETTINGS$MCP_ERROR_COMMAND_NO_SPACES",
SETTINGS$MCP_ERROR_URL_DUPLICATE = "SETTINGS$MCP_ERROR_URL_DUPLICATE",
SETTINGS$MCP_ERROR_ENV_INVALID_FORMAT = "SETTINGS$MCP_ERROR_ENV_INVALID_FORMAT",
SETTINGS$MCP_ERROR_TIMEOUT_INVALID_NUMBER = "SETTINGS$MCP_ERROR_TIMEOUT_INVALID_NUMBER",
SETTINGS$MCP_ERROR_TIMEOUT_POSITIVE = "SETTINGS$MCP_ERROR_TIMEOUT_POSITIVE",
SETTINGS$MCP_ERROR_TIMEOUT_MAX_EXCEEDED = "SETTINGS$MCP_ERROR_TIMEOUT_MAX_EXCEEDED",
SETTINGS$MCP_SERVER_TYPE = "SETTINGS$MCP_SERVER_TYPE",
SETTINGS$MCP_API_KEY_PLACEHOLDER = "SETTINGS$MCP_API_KEY_PLACEHOLDER",
SETTINGS$MCP_COMMAND_ARGUMENTS = "SETTINGS$MCP_COMMAND_ARGUMENTS",

View File

@ -12895,6 +12895,54 @@
"de": "Environment variables must follow KEY=value format",
"uk": "Environment variables must follow KEY=value format"
},
"SETTINGS$MCP_ERROR_TIMEOUT_INVALID_NUMBER": {
"en": "Timeout must be a valid number",
"ja": "タイムアウトは有効な数値である必要があります",
"zh-CN": "超时必须是有效数字",
"zh-TW": "超時必須是有效數字",
"ko-KR": "타임아웃은 유효한 숫자여야 합니다",
"no": "Timeout må være et gyldig tall",
"it": "Il timeout deve essere un numero valido",
"pt": "O timeout deve ser um número válido",
"es": "El timeout debe ser un número válido",
"ar": "يجب أن يكون المهلة الزمنية رقمًا صالحًا",
"fr": "Le timeout doit être un nombre valide",
"tr": "Zaman aşımı geçerli bir sayı olmalıdır",
"de": "Timeout muss eine gültige Zahl sein",
"uk": "Таймаут повинен бути дійсним числом"
},
"SETTINGS$MCP_ERROR_TIMEOUT_POSITIVE": {
"en": "Timeout must be positive",
"ja": "タイムアウトは正の値である必要があります",
"zh-CN": "超时必须为正数",
"zh-TW": "超時必須為正數",
"ko-KR": "타임아웃은 양수여야 합니다",
"no": "Timeout må være positiv",
"it": "Il timeout deve essere positivo",
"pt": "O timeout deve ser positivo",
"es": "El timeout debe ser positivo",
"ar": "يجب أن تكون المهلة الزمنية موجبة",
"fr": "Le timeout doit être positif",
"tr": "Zaman aşımı pozitif olmalıdır",
"de": "Timeout muss positiv sein",
"uk": "Таймаут повинен бути позитивним"
},
"SETTINGS$MCP_ERROR_TIMEOUT_MAX_EXCEEDED": {
"en": "Timeout cannot exceed 3600 seconds (1 hour)",
"ja": "タイムアウトは3600秒1時間を超えることはできません",
"zh-CN": "超时不能超过3600秒1小时",
"zh-TW": "超時不能超過3600秒1小時",
"ko-KR": "타임아웃은 3600초(1시간)을 초과할 수 없습니다",
"no": "Timeout kan ikke overstige 3600 sekunder (1 time)",
"it": "Il timeout non può superare 3600 secondi (1 ora)",
"pt": "O timeout não pode exceder 3600 segundos (1 hora)",
"es": "El timeout no puede exceder 3600 segundos (1 hora)",
"ar": "لا يمكن أن تتجاوز المهلة الزمنية 3600 ثانية (ساعة واحدة)",
"fr": "Le timeout ne peut pas dépasser 3600 secondes (1 heure)",
"tr": "Zaman aşımı 3600 saniyeyi (1 saat) aşamaz",
"de": "Timeout kann 3600 Sekunden (1 Stunde) nicht überschreiten",
"uk": "Таймаут не може перевищувати 3600 секунд (1 година)"
},
"SETTINGS$MCP_SERVER_TYPE": {
"en": "Server Type",
"ja": "サーバータイプ",

View File

@ -20,6 +20,7 @@ interface MCPServerConfig {
name?: string;
url?: string;
api_key?: string;
timeout?: number;
command?: string;
args?: string[];
env?: Record<string, string>;
@ -67,6 +68,7 @@ function MCPSettingsScreen() {
type: "shttp" as const,
url: typeof server === "string" ? server : server.url,
api_key: typeof server === "object" ? server.api_key : undefined,
timeout: typeof server === "object" ? server.timeout : undefined,
})),
];

View File

@ -27,6 +27,7 @@ export type MCPStdioServer = {
export type MCPSHTTPServer = {
url: string;
api_key?: string;
timeout?: number;
};
export type MCPConfig = {

View File

@ -189,8 +189,17 @@ class MCPStdioServerConfig(BaseModel):
class MCPSHTTPServerConfig(BaseModel):
"""Configuration for a MCP server that uses SHTTP.
Attributes:
url: The server URL
api_key: Optional API key for authentication
timeout: Timeout in seconds for tool calls (default: 60s)
"""
url: str
api_key: str | None = None
timeout: int | None = 60
@field_validator('url', mode='before')
@classmethod
@ -198,6 +207,17 @@ class MCPSHTTPServerConfig(BaseModel):
"""Validate URL format for MCP servers."""
return _validate_mcp_url(v)
@field_validator('timeout')
@classmethod
def validate_timeout(cls, v: int | None) -> int | None:
"""Validate timeout value for MCP tool calls."""
if v is not None:
if v <= 0:
raise ValueError('Timeout must be positive')
if v > 3600: # 1 hour max
raise ValueError('Timeout cannot exceed 3600 seconds')
return v
class MCPConfig(BaseModel):
"""Configuration for MCP (Message Control Protocol) settings.

View File

@ -1,3 +1,4 @@
import asyncio
from typing import Optional
from fastmcp import Client
@ -29,6 +30,7 @@ class MCPClient(BaseModel):
description: str = 'MCP client tools for server interaction'
tools: list[MCPClientTool] = Field(default_factory=list)
tool_map: dict[str, MCPClientTool] = Field(default_factory=dict)
server_timeout: Optional[float] = None # Timeout from server config for tool calls
async def _initialize_and_list_tools(self) -> None:
"""Initialize session and populate tool map."""
@ -60,7 +62,7 @@ class MCPClient(BaseModel):
conversation_id: str | None = None,
timeout: float = 30.0,
):
"""Connect to MCP server using SHTTP or SSE transport"""
"""Connect to MCP server using SHTTP or SSE transport."""
server_url = server.url
api_key = server.api_key
@ -123,7 +125,7 @@ class MCPClient(BaseModel):
raise
async def connect_stdio(self, server: MCPStdioServerConfig, timeout: float = 30.0):
"""Connect to MCP server using stdio transport"""
"""Connect to MCP server using stdio transport."""
try:
transport = StdioTransport(
command=server.command, args=server.args or [], env=server.env
@ -145,7 +147,20 @@ class MCPClient(BaseModel):
raise
async def call_tool(self, tool_name: str, args: dict) -> CallToolResult:
"""Call a tool on the MCP server."""
"""Call a tool on the MCP server with timeout from server configuration.
Args:
tool_name: Name of the tool to call
args: Arguments to pass to the tool
Returns:
CallToolResult from the MCP server
Raises:
asyncio.TimeoutError: If the tool call times out
ValueError: If the tool is not found
RuntimeError: If the client session is not available
"""
if tool_name not in self.tool_map:
raise ValueError(f'Tool {tool_name} not found.')
# The MCPClientTool is primarily for metadata; use the session to call the actual tool.
@ -153,4 +168,11 @@ class MCPClient(BaseModel):
raise RuntimeError('Client session is not available.')
async with self.client:
return await self.client.call_tool_mcp(name=tool_name, arguments=args)
# Use server timeout if configured
if self.server_timeout is not None:
return await asyncio.wait_for(
self.client.call_tool_mcp(name=tool_name, arguments=args),
timeout=self.server_timeout,
)
else:
return await self.client.call_tool_mcp(name=tool_name, arguments=args)

View File

@ -1,3 +1,4 @@
import asyncio
import json
import shutil
from typing import TYPE_CHECKING
@ -128,6 +129,11 @@ async def create_mcp_clients(
)
client = MCPClient()
# Set server timeout for SHTTP servers
if isinstance(server, MCPSHTTPServerConfig) and server.timeout is not None:
client.server_timeout = float(server.timeout)
logger.debug(f'Set SHTTP server timeout to {server.timeout}s')
try:
await client.connect_http(server, conversation_id=conversation_id)
@ -253,6 +259,22 @@ async def call_tool_mcp(mcp_clients: list[MCPClient], action: MCPAction) -> Obse
name=action.name,
arguments=action.arguments,
)
except asyncio.TimeoutError:
# Handle timeout errors specifically
timeout_val = getattr(matching_client, 'server_timeout', 'unknown')
logger.error(f'MCP tool {action.name} timed out after {timeout_val}s')
error_content = json.dumps(
{
'isError': True,
'error': f'Tool "{action.name}" timed out after {timeout_val} seconds',
'content': [],
}
)
return MCPObservation(
content=error_content,
name=action.name,
arguments=action.arguments,
)
except McpError as e:
# Handle MCP errors by returning an error observation instead of raising
logger.error(f'MCP error when calling tool {action.name}: {e}')

View File

@ -0,0 +1,122 @@
"""Test MCP SHTTP server timeout configuration."""
import pytest
from pydantic import ValidationError
from openhands.core.config.mcp_config import MCPSHTTPServerConfig
class TestMCPSHTTPServerConfig:
"""Test SHTTP server configuration with timeout field."""
def test_shttp_config_with_timeout(self):
"""Test SHTTP config accepts timeout parameter."""
config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=90)
assert config.timeout == 90
assert config.url == 'https://api.example.com/mcp'
assert config.api_key is None
def test_shttp_config_with_api_key_and_timeout(self):
"""Test SHTTP config with both API key and timeout."""
config = MCPSHTTPServerConfig(
url='https://api.example.com/mcp', api_key='test-key-123', timeout=120
)
assert config.timeout == 120
assert config.api_key == 'test-key-123'
def test_shttp_config_default_timeout(self):
"""Test SHTTP config uses default timeout when not specified."""
config = MCPSHTTPServerConfig(url='https://api.example.com/mcp')
assert config.timeout == 60 # Default value
def test_shttp_config_none_timeout(self):
"""Test SHTTP config accepts None timeout."""
config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=None)
assert config.timeout is None
def test_timeout_validation_positive_values(self):
"""Test timeout validation accepts positive values."""
# Test various valid timeout values
valid_timeouts = [1, 5, 30, 60, 120, 300, 600, 1800, 3600]
for timeout in valid_timeouts:
config = MCPSHTTPServerConfig(
url='https://api.example.com/mcp', timeout=timeout
)
assert config.timeout == timeout
def test_timeout_validation_zero_raises_error(self):
"""Test timeout validation rejects zero timeout."""
with pytest.raises(ValidationError) as exc_info:
MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=0)
assert 'Timeout must be positive' in str(exc_info.value)
def test_timeout_validation_negative_raises_error(self):
"""Test timeout validation rejects negative timeout."""
with pytest.raises(ValidationError) as exc_info:
MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=-10)
assert 'Timeout must be positive' in str(exc_info.value)
def test_timeout_validation_max_limit(self):
"""Test timeout validation enforces maximum limit."""
# Test exactly at the limit
config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=3600)
assert config.timeout == 3600
# Test exceeding the limit
with pytest.raises(ValidationError) as exc_info:
MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=3601)
assert 'Timeout cannot exceed 3600 seconds' in str(exc_info.value)
def test_timeout_validation_way_over_limit(self):
"""Test timeout validation rejects very large values."""
with pytest.raises(ValidationError) as exc_info:
MCPSHTTPServerConfig(
url='https://api.example.com/mcp',
timeout=86400, # 24 hours
)
assert 'Timeout cannot exceed 3600 seconds' in str(exc_info.value)
def test_url_validation_still_works(self):
"""Test that existing URL validation still works with timeout field."""
# Valid URL should work
config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=30)
assert config.url == 'https://api.example.com/mcp'
# Invalid URL should fail
with pytest.raises(ValidationError):
MCPSHTTPServerConfig(url='not-a-url', timeout=30)
def test_backward_compatibility_no_timeout(self):
"""Test backward compatibility - config works without timeout field."""
# Should work exactly like before the timeout field was added
config = MCPSHTTPServerConfig(url='https://api.example.com/mcp')
assert config.url == 'https://api.example.com/mcp'
assert config.api_key is None
assert config.timeout == 60 # Default
def test_model_dump_includes_timeout(self):
"""Test that model serialization includes timeout field."""
config = MCPSHTTPServerConfig(
url='https://api.example.com/mcp', api_key='test-key', timeout=90
)
data = config.model_dump()
expected = {
'url': 'https://api.example.com/mcp',
'api_key': 'test-key',
'timeout': 90,
}
assert data == expected
def test_model_dump_with_none_timeout(self):
"""Test model serialization with None timeout."""
config = MCPSHTTPServerConfig(url='https://api.example.com/mcp', timeout=None)
data = config.model_dump()
expected = {
'url': 'https://api.example.com/mcp',
'api_key': None,
'timeout': None,
}
assert data == expected