joeyutong commented on code in PR #1074:
URL: https://github.com/apache/flink-agents/pull/1074#discussion_r3968137586
##########
python/flink_agents/integrations/mcp/mcp.py:
##########
@@ -65,12 +65,20 @@ def tool_type(cls) -> ToolType:
def call(self, *args: Any, **kwargs: Any) -> Any:
"""Call the MCP tool with the given arguments."""
if self.mcp_server is None:
- msg = "MCP tool call requires a reference to the MCP server"
- raise ValueError(msg)
+ return ToolResponse.error(
+ "MCP tool call requires a reference to the MCP server",
+ tool_name=self.metadata.name,
+ )
- return asyncio.run(
- self.mcp_server.call_tool_async(self.metadata.name, *args,
**kwargs)
- )
+ try:
+ return asyncio.run(
+ self.mcp_server.call_tool_async(self.metadata.name, *args,
**kwargs)
+ )
+ except Exception as e:
Review Comment:
Fixed. `PythonMCPTool` now uses the same bridge encoder/decoder as
`FunctionTool`, so an explicit Python `ToolResponse.error(...)` remains a
failed Java `ToolResponse`. I also added a unit regression and extended
`MCPCrossLanguageTest` with a real FastMCP protocol error over Pemja.
##########
python/flink_agents/integrations/mcp/mcp.py:
##########
@@ -237,6 +249,10 @@ async def call_tool_async(self, tool_name: str, *args:
Any, **kwargs: Any) -> An
content = [extract_mcp_content_item(item) for item in
result.content]
+ if result.isError:
Review Comment:
Fixed by moving MCP content/error interpretation outside the session
context. The regression now uses a real `ClientSession` lifecycle and verifies
that the original protocol error content is retained instead of only the outer
`ExceptionGroup` message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
