Skip to content

Commit f256e98

Browse files
fix(server): return 404 when deleting non-existent server (#1592)
* fix(server): ensure server exists before deletion in delete_server function Signed-off-by: Duarte Martinho <dmartinho@ibm.com> * test(server): update delete_server tests for existence check Update test_delete_server_endpoint to mock get_server which is now called before delete_server to verify server existence. Add test_delete_server_not_found to verify that attempting to delete a non-existent server returns HTTP 404 instead of 400. Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: Duarte Martinho <dmartinho@ibm.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Duarte Martinho <dmartinho@ibm.com>
1 parent e997513 commit f256e98

File tree

2 files changed

+14
-1
lines changed

2 files changed

+14
-1
lines changed

mcpgateway/main.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1966,6 +1966,7 @@ async def delete_server(server_id: str, db: Session = Depends(get_db), user=Depe
19661966
try:
19671967
logger.debug(f"User {user} is deleting server with ID {server_id}")
19681968
user_email = user.get("email") if isinstance(user, dict) else str(user)
1969+
await server_service.get_server(db, server_id)
19691970
await server_service.delete_server(db, server_id, user_email=user_email)
19701971
return {
19711972
"status": "success",

tests/unit/mcpgateway/test_main.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,13 +520,25 @@ def test_toggle_server_status(self, mock_toggle, test_client, auth_headers):
520520
mock_toggle.assert_called_once()
521521

522522
@patch("mcpgateway.main.server_service.delete_server")
523-
def test_delete_server_endpoint(self, mock_delete, test_client, auth_headers):
523+
@patch("mcpgateway.main.server_service.get_server")
524+
def test_delete_server_endpoint(self, mock_get, mock_delete, test_client, auth_headers):
524525
"""Test permanently deleting a server."""
526+
mock_get.return_value = ServerRead(**MOCK_SERVER_READ)
525527
mock_delete.return_value = None
526528
response = test_client.delete("/servers/1", headers=auth_headers)
527529
assert response.status_code == 200
528530
assert response.json()["status"] == "success"
529531

532+
@patch("mcpgateway.main.server_service.get_server")
533+
def test_delete_server_not_found(self, mock_get, test_client, auth_headers):
534+
"""Test deleting a non-existent server returns 404."""
535+
from mcpgateway.services.server_service import ServerNotFoundError
536+
537+
mock_get.side_effect = ServerNotFoundError("Server not found: nonexistent-id")
538+
response = test_client.delete("/servers/nonexistent-id", headers=auth_headers)
539+
assert response.status_code == 404
540+
assert "Server not found" in response.json()["detail"]
541+
530542
@patch("mcpgateway.main.tool_service.list_server_tools")
531543
def test_server_get_tools(self, mock_list_tools, test_client, auth_headers):
532544
"""Test listing tools associated with a server."""

0 commit comments

Comments
 (0)