Skip to content

Commit e89ade7

Browse files
iskakaushikjovezhong
authored andcommitted
fix: prevent BrokenResourceError by returning structured responses for query errors (ClickHouse#26)
fixes ClickHouse#25
1 parent 47bbd47 commit e89ade7

File tree

2 files changed

+28
-10
lines changed

2 files changed

+28
-10
lines changed

mcp_timeplus/mcp_server.py

+24-8
Original file line numberDiff line numberDiff line change
@@ -119,15 +119,29 @@ def get_table_info(table):
119119

120120
@mcp.tool()
121121
def run_sql(query: str):
122+
"""Run a query in a Timeplus database"""
122123
logger.info(f"Executing query: {query}")
123-
future = QUERY_EXECUTOR.submit(execute_query, query)
124124
try:
125-
result = future.result(timeout=SELECT_QUERY_TIMEOUT_SECS)
126-
return result
127-
except concurrent.futures.TimeoutError:
128-
logger.warning(f"Query timed out after {SELECT_QUERY_TIMEOUT_SECS} seconds: {query}")
129-
future.cancel()
130-
return f"Queries taking longer than {SELECT_QUERY_TIMEOUT_SECS} seconds are currently not supported."
125+
future = QUERY_EXECUTOR.submit(execute_query, query)
126+
try:
127+
result = future.result(timeout=SELECT_QUERY_TIMEOUT_SECS)
128+
# Check if we received an error structure from execute_query
129+
if isinstance(result, dict) and "error" in result:
130+
logger.warning(f"Query failed: {result['error']}")
131+
# MCP requires structured responses; string error messages can cause
132+
# serialization issues leading to BrokenResourceError
133+
return {"status": "error", "message": f"Query failed: {result['error']}"}
134+
return result
135+
except concurrent.futures.TimeoutError:
136+
logger.warning(f"Query timed out after {SELECT_QUERY_TIMEOUT_SECS} seconds: {query}")
137+
future.cancel()
138+
# Return a properly structured response for timeout errors
139+
return {"status": "error", "message": f"Query timed out after {SELECT_QUERY_TIMEOUT_SECS} seconds"}
140+
except Exception as e:
141+
logger.error(f"Unexpected error in run_select_query: {str(e)}")
142+
# Catch all other exceptions and return them in a structured format
143+
# to prevent MCP serialization failures
144+
return {"status": "error", "message": f"Unexpected error: {str(e)}"}
131145

132146
client = create_timeplus_client()
133147
try:
@@ -144,7 +158,9 @@ def run_sql(query: str):
144158
return rows
145159
except Exception as err:
146160
logger.error(f"Error executing query: {err}")
147-
return f"error running query: {err}"
161+
# Return a structured dictionary rather than a string to ensure proper serialization
162+
# by the MCP protocol. String responses for errors can cause BrokenResourceError.
163+
return {"error": str(err)}
148164

149165
@mcp.prompt()
150166
def generate_sql(requirements: str) -> str:

tests/test_tool.py

+4-2
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,10 @@ def test_run_select_query_failure(self):
7070
"""Test running a SELECT query with an error."""
7171
query = f"SELECT * FROM {self.test_db}.non_existent_table"
7272
result = run_sql(query)
73-
self.assertIsInstance(result, str)
74-
self.assertIn("error running query", result)
73+
self.assertIsInstance(result, dict)
74+
self.assertEqual(result["status"], "error")
75+
self.assertIn("Query failed", result["message"])
76+
7577

7678
def test_table_and_column_comments(self):
7779
"""Test that table and column comments are correctly retrieved."""

0 commit comments

Comments
 (0)