Skip to content

Commit 66fcaba

Browse files
committed
: Improve OAuth tool fetch logging and depth handling
1 parent 1bf3267 commit 66fcaba

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
@@ -1344,7 +1344,17 @@ def validate_database(self) -> None:
13441344
validation_max_description_length: int = 8192 # 8KB
13451345
validation_max_template_length: int = 65536 # 64KB
13461346
validation_max_content_length: int = 1048576 # 1MB
1347-
validation_max_json_depth: int = 10
1347+
validation_max_json_depth: int = Field(
1348+
default=int(os.getenv("VALIDATION_MAX_JSON_DEPTH", "30")),
1349+
description=(
1350+
"Maximum allowed JSON nesting depth for tool/resource schemas. "
1351+
"Increased from 10 to 30 for compatibility with deeply nested schemas "
1352+
"like Notion MCP (issue #1542). Override with VALIDATION_MAX_JSON_DEPTH "
1353+
"environment variable. Minimum: 1, Maximum: 100"
1354+
),
1355+
ge=1,
1356+
le=100,
1357+
)
13481358
validation_max_url_length: int = 2048
13491359
validation_max_rpc_param_size: int = 262144 # 256KB
13501360

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
@@ -266,6 +267,10 @@ class GatewayConnectionError(GatewayError):
266267
"""
267268

268269

270+
class OAuthToolValidationError(GatewayConnectionError):
271+
"""Raised when tool validation fails during OAuth-driven fetch."""
272+
273+
269274
class GatewayService: # pylint: disable=too-many-instance-attributes
270275
"""Service for managing federated gateways.
271276
@@ -986,6 +991,10 @@ async def fetch_tools_after_oauth(self, db: Session, gateway_id: str, app_user_e
986991

987992
return {"capabilities": capabilities, "tools": tools, "resources": resources, "prompts": prompts}
988993

994+
except GatewayConnectionError as gce:
995+
# Surface validation or depth-related failures directly to the user
996+
logger.error(f"GatewayConnectionError during OAuth fetch for {gateway_id}: {gce}")
997+
raise GatewayConnectionError(f"Failed to fetch tools after OAuth: {str(gce)}")
989998
except Exception as e:
990999
logger.error(f"Failed to fetch tools after OAuth for gateway {gateway_id}: {e}")
9911000
raise GatewayConnectionError(f"Failed to fetch tools after OAuth: {str(e)}")
@@ -3146,6 +3155,66 @@ async def _publish_event(self, event: Dict[str, Any]) -> None:
31463155
"""
31473156
await self._event_service.publish_event(event)
31483157

3158+
def _validate_tools(self, tools: list[dict[str, Any]], context: str = "default") -> list[ToolCreate]:
3159+
"""Validate tools individually with richer logging and error aggregation.
3160+
3161+
Args:
3162+
tools: list of tool dicts
3163+
context: caller context, e.g. "oauth" to tailor errors/messages
3164+
"""
3165+
valid_tools: list[ToolCreate] = []
3166+
validation_errors: list[str] = []
3167+
3168+
for i, tool_dict in enumerate(tools):
3169+
tool_name = tool_dict.get("name", f"unknown_tool_{i}")
3170+
try:
3171+
logger.debug(f"Validating tool: {tool_name}")
3172+
validated_tool = ToolCreate.model_validate(tool_dict)
3173+
valid_tools.append(validated_tool)
3174+
logger.debug(f"Tool '{tool_name}' validated successfully")
3175+
except ValidationError as e:
3176+
error_msg = f"Validation failed for tool '{tool_name}': {e.errors()}"
3177+
logger.error(error_msg)
3178+
logger.debug(f"Failed tool schema: {tool_dict}")
3179+
validation_errors.append(error_msg)
3180+
except ValueError as e:
3181+
if "JSON structure exceeds maximum depth" in str(e):
3182+
error_msg = (
3183+
f"Tool '{tool_name}' schema too deeply nested. "
3184+
f"Current depth limit: {settings.validation_max_json_depth}"
3185+
)
3186+
logger.error(error_msg)
3187+
logger.warning("Consider increasing VALIDATION_MAX_JSON_DEPTH environment variable")
3188+
else:
3189+
error_msg = f"ValueError for tool '{tool_name}': {str(e)}"
3190+
logger.error(error_msg)
3191+
validation_errors.append(error_msg)
3192+
except Exception as e: # pragma: no cover - defensive
3193+
error_msg = f"Unexpected error validating tool '{tool_name}': {type(e).__name__}: {str(e)}"
3194+
logger.error(error_msg, exc_info=True)
3195+
validation_errors.append(error_msg)
3196+
3197+
if validation_errors:
3198+
logger.warning(
3199+
f"Tool validation completed with {len(validation_errors)} error(s). "
3200+
f"Successfully validated {len(valid_tools)} tool(s)."
3201+
)
3202+
for err in validation_errors[:3]:
3203+
logger.debug(f"Validation error: {err}")
3204+
3205+
if not valid_tools and validation_errors:
3206+
if context == "oauth":
3207+
raise OAuthToolValidationError(
3208+
f"OAuth tool fetch failed: all {len(tools)} tools failed validation. "
3209+
f"First error: {validation_errors[0][:200]}"
3210+
)
3211+
raise GatewayConnectionError(
3212+
f"Failed to fetch tools: All {len(tools)} tools failed validation. "
3213+
f"First error: {validation_errors[0][:200]}"
3214+
)
3215+
3216+
return valid_tools
3217+
31493218
async def _connect_to_sse_server_without_validation(self, server_url: str, authentication: Optional[Dict[str, str]] = None):
31503219
"""Connect to an MCP server running with SSE transport, skipping URL validation.
31513220
@@ -3173,9 +3242,11 @@ async def _connect_to_sse_server_without_validation(self, server_url: str, authe
31733242

31743243
response = await session.list_tools()
31753244
tools = response.tools
3176-
tools = [tool.model_dump(by_alias=True, exclude_none=True) for tool in tools]
3245+
tools = [
3246+
tool.model_dump(by_alias=True, exclude_none=True, exclude_unset=True) for tool in tools
3247+
]
31773248

3178-
tools = [ToolCreate.model_validate(tool) for tool in tools]
3249+
tools = self._validate_tools(tools, context="oauth")
31793250
if tools:
31803251
logger.info(f"Fetched {len(tools)} tools from gateway")
31813252
# Fetch resources if supported
@@ -3316,9 +3387,11 @@ def get_httpx_client_factory(
33163387

33173388
response = await session.list_tools()
33183389
tools = response.tools
3319-
tools = [tool.model_dump(by_alias=True, exclude_none=True) for tool in tools]
3390+
tools = [
3391+
tool.model_dump(by_alias=True, exclude_none=True, exclude_unset=True) for tool in tools
3392+
]
33203393

3321-
tools = [ToolCreate.model_validate(tool) for tool in tools]
3394+
tools = self._validate_tools(tools)
33223395
if tools:
33233396
logger.info(f"Fetched {len(tools)} tools from gateway")
33243397
# Fetch resources if supported
@@ -3456,9 +3529,11 @@ def get_httpx_client_factory(
34563529

34573530
response = await session.list_tools()
34583531
tools = response.tools
3459-
tools = [tool.model_dump(by_alias=True, exclude_none=True) for tool in tools]
3532+
tools = [
3533+
tool.model_dump(by_alias=True, exclude_none=True, exclude_unset=True) for tool in tools
3534+
]
34603535

3461-
tools = [ToolCreate.model_validate(tool) for tool in tools]
3536+
tools = self._validate_tools(tools)
34623537
for tool in tools:
34633538
tool.request_type = "STREAMABLEHTTP"
34643539
if tools:

0 commit comments

Comments
 (0)