Skip to content

Commit e58d4e5

Browse files
committed
fix(gateway): improve tool validation logging and increase JSON depth limit
- Increase default validation_max_json_depth from 10 to 30 for compatibility with deeply nested schemas (e.g., Notion MCP) - Add VALIDATION_MAX_JSON_DEPTH environment variable for configuration - Add _validate_tools method for per-tool validation with detailed error logging instead of silent failures - Add OAuthToolValidationError for OAuth-specific validation failures - Provide helpful error messages suggesting depth limit increase - Add exclude_unset=True to model_dump to prevent validation issues Users will now see actionable error messages instead of generic "unhandled errors in TaskGroup" when tool validation fails. Closes #1542 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
1 parent 8ca34d5 commit e58d4e5

File tree

2 files changed

+92
-7
lines changed

2 files changed

+92
-7
lines changed

mcpgateway/config.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1435,7 +1435,17 @@ def validate_database(self) -> None:
14351435
validation_max_description_length: int = 8192 # 8KB
14361436
validation_max_template_length: int = 65536 # 64KB
14371437
validation_max_content_length: int = 1048576 # 1MB
1438-
validation_max_json_depth: int = 10
1438+
validation_max_json_depth: int = Field(
1439+
default=int(os.getenv("VALIDATION_MAX_JSON_DEPTH", "30")),
1440+
description=(
1441+
"Maximum allowed JSON nesting depth for tool/resource schemas. "
1442+
"Increased from 10 to 30 for compatibility with deeply nested schemas "
1443+
"like Notion MCP (issue #1542). Override with VALIDATION_MAX_JSON_DEPTH "
1444+
"environment variable. Minimum: 1, Maximum: 100"
1445+
),
1446+
ge=1,
1447+
le=100,
1448+
)
14391449
validation_max_url_length: int = 2048
14401450
validation_max_rpc_param_size: int = 262144 # 256KB
14411451

mcpgateway/services/gateway_service.py

Lines changed: 81 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
from mcp import ClientSession
5757
from mcp.client.sse import sse_client
5858
from mcp.client.streamable_http import streamablehttp_client
59+
from pydantic import ValidationError
5960
from sqlalchemy import and_, or_, select
6061
from sqlalchemy.exc import IntegrityError
6162
from sqlalchemy.orm import Session
@@ -272,6 +273,10 @@ class GatewayConnectionError(GatewayError):
272273
"""
273274

274275

276+
class OAuthToolValidationError(GatewayConnectionError):
277+
"""Raised when tool validation fails during OAuth-driven fetch."""
278+
279+
275280
class GatewayService: # pylint: disable=too-many-instance-attributes
276281
"""Service for managing federated gateways.
277282
@@ -1110,6 +1115,10 @@ async def fetch_tools_after_oauth(self, db: Session, gateway_id: str, app_user_e
11101115

11111116
return {"capabilities": capabilities, "tools": tools, "resources": resources, "prompts": prompts}
11121117

1118+
except GatewayConnectionError as gce:
1119+
# Surface validation or depth-related failures directly to the user
1120+
logger.error(f"GatewayConnectionError during OAuth fetch for {gateway_id}: {gce}")
1121+
raise GatewayConnectionError(f"Failed to fetch tools after OAuth: {str(gce)}")
11131122
except Exception as e:
11141123
logger.error(f"Failed to fetch tools after OAuth for gateway {gateway_id}: {e}")
11151124
raise GatewayConnectionError(f"Failed to fetch tools after OAuth: {str(e)}")
@@ -3514,6 +3523,66 @@ async def _publish_event(self, event: Dict[str, Any]) -> None:
35143523
"""
35153524
await self._event_service.publish_event(event)
35163525

3526+
def _validate_tools(self, tools: list[dict[str, Any]], context: str = "default") -> list[ToolCreate]:
3527+
"""Validate tools individually with richer logging and error aggregation.
3528+
3529+
Args:
3530+
tools: list of tool dicts
3531+
context: caller context, e.g. "oauth" to tailor errors/messages
3532+
"""
3533+
valid_tools: list[ToolCreate] = []
3534+
validation_errors: list[str] = []
3535+
3536+
for i, tool_dict in enumerate(tools):
3537+
tool_name = tool_dict.get("name", f"unknown_tool_{i}")
3538+
try:
3539+
logger.debug(f"Validating tool: {tool_name}")
3540+
validated_tool = ToolCreate.model_validate(tool_dict)
3541+
valid_tools.append(validated_tool)
3542+
logger.debug(f"Tool '{tool_name}' validated successfully")
3543+
except ValidationError as e:
3544+
error_msg = f"Validation failed for tool '{tool_name}': {e.errors()}"
3545+
logger.error(error_msg)
3546+
logger.debug(f"Failed tool schema: {tool_dict}")
3547+
validation_errors.append(error_msg)
3548+
except ValueError as e:
3549+
if "JSON structure exceeds maximum depth" in str(e):
3550+
error_msg = (
3551+
f"Tool '{tool_name}' schema too deeply nested. "
3552+
f"Current depth limit: {settings.validation_max_json_depth}"
3553+
)
3554+
logger.error(error_msg)
3555+
logger.warning("Consider increasing VALIDATION_MAX_JSON_DEPTH environment variable")
3556+
else:
3557+
error_msg = f"ValueError for tool '{tool_name}': {str(e)}"
3558+
logger.error(error_msg)
3559+
validation_errors.append(error_msg)
3560+
except Exception as e: # pragma: no cover - defensive
3561+
error_msg = f"Unexpected error validating tool '{tool_name}': {type(e).__name__}: {str(e)}"
3562+
logger.error(error_msg, exc_info=True)
3563+
validation_errors.append(error_msg)
3564+
3565+
if validation_errors:
3566+
logger.warning(
3567+
f"Tool validation completed with {len(validation_errors)} error(s). "
3568+
f"Successfully validated {len(valid_tools)} tool(s)."
3569+
)
3570+
for err in validation_errors[:3]:
3571+
logger.debug(f"Validation error: {err}")
3572+
3573+
if not valid_tools and validation_errors:
3574+
if context == "oauth":
3575+
raise OAuthToolValidationError(
3576+
f"OAuth tool fetch failed: all {len(tools)} tools failed validation. "
3577+
f"First error: {validation_errors[0][:200]}"
3578+
)
3579+
raise GatewayConnectionError(
3580+
f"Failed to fetch tools: All {len(tools)} tools failed validation. "
3581+
f"First error: {validation_errors[0][:200]}"
3582+
)
3583+
3584+
return valid_tools
3585+
35173586
async def _connect_to_sse_server_without_validation(self, server_url: str, authentication: Optional[Dict[str, str]] = None):
35183587
"""Connect to an MCP server running with SSE transport, skipping URL validation.
35193588
@@ -3541,9 +3610,11 @@ async def _connect_to_sse_server_without_validation(self, server_url: str, authe
35413610

35423611
response = await session.list_tools()
35433612
tools = response.tools
3544-
tools = [tool.model_dump(by_alias=True, exclude_none=True) for tool in tools]
3613+
tools = [
3614+
tool.model_dump(by_alias=True, exclude_none=True, exclude_unset=True) for tool in tools
3615+
]
35453616

3546-
tools = [ToolCreate.model_validate(tool) for tool in tools]
3617+
tools = self._validate_tools(tools, context="oauth")
35473618
if tools:
35483619
logger.info(f"Fetched {len(tools)} tools from gateway")
35493620
# Fetch resources if supported
@@ -3684,9 +3755,11 @@ def get_httpx_client_factory(
36843755

36853756
response = await session.list_tools()
36863757
tools = response.tools
3687-
tools = [tool.model_dump(by_alias=True, exclude_none=True) for tool in tools]
3758+
tools = [
3759+
tool.model_dump(by_alias=True, exclude_none=True, exclude_unset=True) for tool in tools
3760+
]
36883761

3689-
tools = [ToolCreate.model_validate(tool) for tool in tools]
3762+
tools = self._validate_tools(tools)
36903763
if tools:
36913764
logger.info(f"Fetched {len(tools)} tools from gateway")
36923765
# Fetch resources if supported
@@ -3824,9 +3897,11 @@ def get_httpx_client_factory(
38243897

38253898
response = await session.list_tools()
38263899
tools = response.tools
3827-
tools = [tool.model_dump(by_alias=True, exclude_none=True) for tool in tools]
3900+
tools = [
3901+
tool.model_dump(by_alias=True, exclude_none=True, exclude_unset=True) for tool in tools
3902+
]
38283903

3829-
tools = [ToolCreate.model_validate(tool) for tool in tools]
3904+
tools = self._validate_tools(tools)
38303905
for tool in tools:
38313906
tool.request_type = "STREAMABLEHTTP"
38323907
if tools:

0 commit comments

Comments
 (0)