Skip to content

Commit d8a5268

Browse files
committed
♻️ refactor mcp app/service/db/test case
1 parent 8e67dce commit d8a5268

File tree

5 files changed

+43
-6
lines changed

5 files changed

+43
-6
lines changed

backend/apps/remote_mcp_app.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,10 @@ async def check_mcp_health(mcp_url: str, service_name: str, authorization: Optio
130130
status_code=HTTPStatus.OK,
131131
content={"status": "success"}
132132
)
133+
except MCPConnectionError as e:
134+
logger.error(f"MCP connection failed: {e}")
135+
raise HTTPException(status_code=HTTPStatus.SERVICE_UNAVAILABLE,
136+
detail="MCP connection failed")
133137
except Exception as e:
134138
logger.error(f"Failed to check the health of the MCP server: {e}")
135139
raise HTTPException(status_code=HTTPStatus.INTERNAL_SERVER_ERROR,

backend/services/remote_mcp_service.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,5 @@ async def check_mcp_health_and_update_db(mcp_url, service_name, tenant_id, user_
7979
tenant_id=tenant_id,
8080
user_id=user_id,
8181
status=status)
82+
if not status:
83+
raise MCPConnectionError("MCP connection failed")

frontend/services/mcpService.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,10 +318,14 @@ export const checkMcpServerHealth = async (mcpUrl: string, serviceName: string)
318318
message: data.message || t('mcpService.message.healthCheckSuccess')
319319
};
320320
} else {
321+
let errorMessage = data.message || t('mcpService.message.healthCheckFailed');
322+
if (response.status === 503) {
323+
errorMessage = t('mcpService.message.cannotConnectToServer');
324+
}
321325
return {
322326
success: false,
323327
data: null,
324-
message: data.message || t('mcpService.message.healthCheckFailed')
328+
message: errorMessage
325329
};
326330
}
327331
} catch (error) {

test/backend/app/test_remote_mcp_app.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,29 @@ def test_check_mcp_health_success(self, mock_health_check, mock_get_user_id):
325325
"http://test.com", "test_service", "tenant456", "user123"
326326
)
327327

328+
@patch('apps.remote_mcp_app.get_current_user_id')
329+
@patch('apps.remote_mcp_app.check_mcp_health_and_update_db')
330+
def test_check_mcp_health_connection_error(self, mock_health_check, mock_get_user_id):
331+
"""Test MCP connection error during health check"""
332+
mock_get_user_id.return_value = ("user123", "tenant456")
333+
mock_health_check.side_effect = MCPConnectionError("MCP connection failed")
334+
335+
response = client.get(
336+
"/mcp/healthcheck",
337+
params={"mcp_url": "http://unreachable.com",
338+
"service_name": "test_service"},
339+
headers={"Authorization": "Bearer test_token"}
340+
)
341+
342+
assert response.status_code == HTTPStatus.SERVICE_UNAVAILABLE
343+
data = response.json()
344+
assert "MCP connection failed" in data["detail"]
345+
346+
mock_get_user_id.assert_called_once_with("Bearer test_token")
347+
mock_health_check.assert_called_once_with(
348+
"http://unreachable.com", "test_service", "tenant456", "user123"
349+
)
350+
328351
@patch('apps.remote_mcp_app.get_current_user_id')
329352
@patch('apps.remote_mcp_app.check_mcp_health_and_update_db')
330353
def test_check_mcp_health_database_error(self, mock_health_check, mock_get_user_id):

test/backend/services/test_remote_mcp_service.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -280,11 +280,13 @@ async def test_check_health_success(self, mock_health, mock_update):
280280
@patch('backend.services.remote_mcp_service.update_mcp_status_by_name_and_url')
281281
@patch('backend.services.remote_mcp_service.mcp_server_health')
282282
async def test_check_health_false(self, mock_health, mock_update):
283-
"""Test health check failure"""
283+
"""Test health check failure - should raise MCPConnectionError when status is False"""
284284
mock_health.return_value = False
285285

286-
await check_mcp_health_and_update_db('http://srv', 'name', 'tid', 'uid')
286+
with self.assertRaises(MCPConnectionError) as context:
287+
await check_mcp_health_and_update_db('http://srv', 'name', 'tid', 'uid')
287288

289+
self.assertEqual(str(context.exception), "MCP connection failed")
288290
mock_update.assert_called_once_with(
289291
mcp_name='name',
290292
mcp_server='http://srv',
@@ -308,12 +310,14 @@ async def test_update_db_fail(self, mock_health, mock_update):
308310
@patch('backend.services.remote_mcp_service.update_mcp_status_by_name_and_url')
309311
@patch('backend.services.remote_mcp_service.mcp_server_health')
310312
async def test_health_check_exception(self, mock_health, mock_update):
311-
"""Test health check exception - should catch exception and set status to False"""
313+
"""Test health check exception - should catch exception, set status to False, and raise MCPConnectionError"""
312314
mock_health.side_effect = MCPConnectionError("Connection failed")
313315

314-
# Should not raise exception, but should set status to False
315-
await check_mcp_health_and_update_db('http://srv', 'name', 'tid', 'uid')
316+
# Should catch the exception from mcp_server_health, set status to False, and then raise MCPConnectionError
317+
with self.assertRaises(MCPConnectionError) as context:
318+
await check_mcp_health_and_update_db('http://srv', 'name', 'tid', 'uid')
316319

320+
self.assertEqual(str(context.exception), "MCP connection failed")
317321
mock_health.assert_called_once_with(remote_mcp_server='http://srv')
318322
mock_update.assert_called_once_with(
319323
mcp_name='name',

0 commit comments

Comments
 (0)