From db760a7ce95503669aca2b4f350c224ef69ba922 Mon Sep 17 00:00:00 2001 From: Zamil Majdy Date: Wed, 11 Feb 2026 07:23:52 +0400 Subject: [PATCH] fix(mcp): Remove silent exception swallowing in credential lookup Let credential lookup errors propagate in discover_tools endpoint instead of silently catching all exceptions. Upgrade block.py auto-lookup logging from debug to warning. Update tests to mock creds_manager so they don't hit the database. --- .../backend/api/features/mcp/routes.py | 47 +++++++++---------- .../backend/api/features/mcp/test_routes.py | 30 ++++++++++-- .../backend/backend/blocks/mcp/block.py | 2 +- 3 files changed, 48 insertions(+), 31 deletions(-) diff --git a/autogpt_platform/backend/backend/api/features/mcp/routes.py b/autogpt_platform/backend/backend/api/features/mcp/routes.py index f78619463a..6b567db1c2 100644 --- a/autogpt_platform/backend/backend/api/features/mcp/routes.py +++ b/autogpt_platform/backend/backend/api/features/mcp/routes.py @@ -78,32 +78,29 @@ async def discover_tools( # Auto-use stored MCP credential when no explicit token is provided. if not auth_token: - try: - mcp_creds = await creds_manager.store.get_creds_by_provider( - user_id, ProviderName.MCP.value - ) - # Find the freshest credential for this server URL - best_cred: OAuth2Credentials | None = None - for cred in mcp_creds: - if ( - isinstance(cred, OAuth2Credentials) - and cred.metadata.get("mcp_server_url") == request.server_url + mcp_creds = await creds_manager.store.get_creds_by_provider( + user_id, ProviderName.MCP.value + ) + # Find the freshest credential for this server URL + best_cred: OAuth2Credentials | None = None + for cred in mcp_creds: + if ( + isinstance(cred, OAuth2Credentials) + and cred.metadata.get("mcp_server_url") == request.server_url + ): + if best_cred is None or ( + (cred.access_token_expires_at or 0) + > (best_cred.access_token_expires_at or 0) ): - if best_cred is None or ( - (cred.access_token_expires_at or 0) - > (best_cred.access_token_expires_at or 0) - ): - best_cred = cred - if best_cred: - # Refresh the token if expired before using it - best_cred = await creds_manager.refresh_if_needed(user_id, best_cred) - logger.info( - f"Using MCP credential {best_cred.id} for {request.server_url}, " - f"expires_at={best_cred.access_token_expires_at}" - ) - auth_token = best_cred.access_token.get_secret_value() - except Exception: - logger.debug("Could not look up stored MCP credentials", exc_info=True) + best_cred = cred + if best_cred: + # Refresh the token if expired before using it + best_cred = await creds_manager.refresh_if_needed(user_id, best_cred) + logger.info( + f"Using MCP credential {best_cred.id} for {request.server_url}, " + f"expires_at={best_cred.access_token_expires_at}" + ) + auth_token = best_cred.access_token.get_secret_value() client = MCPClient(request.server_url, auth_token=auth_token) diff --git a/autogpt_platform/backend/backend/api/features/mcp/test_routes.py b/autogpt_platform/backend/backend/api/features/mcp/test_routes.py index 0f65ea350b..e86b9f4865 100644 --- a/autogpt_platform/backend/backend/api/features/mcp/test_routes.py +++ b/autogpt_platform/backend/backend/api/features/mcp/test_routes.py @@ -54,7 +54,11 @@ class TestDiscoverTools: ), ] - with (patch("backend.api.features.mcp.routes.MCPClient") as MockClient,): + with ( + patch("backend.api.features.mcp.routes.MCPClient") as MockClient, + patch("backend.api.features.mcp.routes.creds_manager") as mock_cm, + ): + mock_cm.store.get_creds_by_provider = AsyncMock(return_value=[]) instance = MockClient.return_value instance.initialize = AsyncMock( return_value={ @@ -143,7 +147,11 @@ class TestDiscoverTools: @pytest.mark.asyncio(loop_scope="session") async def test_discover_tools_mcp_error(self, client): - with patch("backend.api.features.mcp.routes.MCPClient") as MockClient: + with ( + patch("backend.api.features.mcp.routes.MCPClient") as MockClient, + patch("backend.api.features.mcp.routes.creds_manager") as mock_cm, + ): + mock_cm.store.get_creds_by_provider = AsyncMock(return_value=[]) instance = MockClient.return_value instance.initialize = AsyncMock( side_effect=MCPClientError("Connection refused") @@ -159,7 +167,11 @@ class TestDiscoverTools: @pytest.mark.asyncio(loop_scope="session") async def test_discover_tools_generic_error(self, client): - with patch("backend.api.features.mcp.routes.MCPClient") as MockClient: + with ( + patch("backend.api.features.mcp.routes.MCPClient") as MockClient, + patch("backend.api.features.mcp.routes.creds_manager") as mock_cm, + ): + mock_cm.store.get_creds_by_provider = AsyncMock(return_value=[]) instance = MockClient.return_value instance.initialize = AsyncMock(side_effect=Exception("Network timeout")) @@ -173,7 +185,11 @@ class TestDiscoverTools: @pytest.mark.asyncio(loop_scope="session") async def test_discover_tools_auth_required(self, client): - with patch("backend.api.features.mcp.routes.MCPClient") as MockClient: + with ( + patch("backend.api.features.mcp.routes.MCPClient") as MockClient, + patch("backend.api.features.mcp.routes.creds_manager") as mock_cm, + ): + mock_cm.store.get_creds_by_provider = AsyncMock(return_value=[]) instance = MockClient.return_value instance.initialize = AsyncMock( side_effect=HTTPClientError("HTTP 401 Error: Unauthorized", 401) @@ -189,7 +205,11 @@ class TestDiscoverTools: @pytest.mark.asyncio(loop_scope="session") async def test_discover_tools_forbidden(self, client): - with patch("backend.api.features.mcp.routes.MCPClient") as MockClient: + with ( + patch("backend.api.features.mcp.routes.MCPClient") as MockClient, + patch("backend.api.features.mcp.routes.creds_manager") as mock_cm, + ): + mock_cm.store.get_creds_by_provider = AsyncMock(return_value=[]) instance = MockClient.return_value instance.initialize = AsyncMock( side_effect=HTTPClientError("HTTP 403 Error: Forbidden", 403) diff --git a/autogpt_platform/backend/backend/blocks/mcp/block.py b/autogpt_platform/backend/backend/blocks/mcp/block.py index 7554af9b6a..89bbdf749b 100644 --- a/autogpt_platform/backend/backend/blocks/mcp/block.py +++ b/autogpt_platform/backend/backend/blocks/mcp/block.py @@ -241,7 +241,7 @@ class MCPToolBlock(Block): ) return best except Exception: - logger.debug("Auto-lookup MCP credential failed", exc_info=True) + logger.warning("Auto-lookup MCP credential failed", exc_info=True) return None async def run(