From 6bc70a6bab79b679a4b18ad146b3450fb9014475 Mon Sep 17 00:00:00 2001 From: Dinesh Thumma <160909147+DineshThumma9@users.noreply.github.com> Date: Mon, 9 Feb 2026 07:48:07 -0800 Subject: [PATCH] fix(mcp): used logger to log instead of print Merge https://github.com/google/adk-python/pull/4324 **Please ensure you have read the [contribution guide](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) before creating a pull request.** ### Link to Issue or Description of Change **1. Link to an existing issue (if applicable):** - Closes: #4320 **2. Or, if no issue exists, describe the change:** **Problem:** Was using print instead of logger for warning message **Solution:* Refactored ### Testing Plan **Unit Tests:** - [x] I have added or updated unit tests for my change. - [x] All unit tests pass locally. _Please include a summary of passed `pytest` results._ **Manual End-to-End (E2E) Tests:** ### Checklist - [x] I have read the [CONTRIBUTING.md](https://github.com/google/adk-python/blob/main/CONTRIBUTING.md) document. - [x] I have performed a self-review of my own code. - [x] I have commented my code, particularly in hard-to-understand areas. - [x] I have added tests that prove my fix is effective or that my feature works. - [x] New and existing unit tests pass locally with my changes. - [x] I have manually tested my changes end-to-end. - [ ] Any dependent changes have been merged and published in downstream modules. ### Additional context _Add any other context or screenshots about the feature request here._ COPYBARA_INTEGRATE_REVIEW=https://github.com/google/adk-python/pull/4324 from DineshThumma9:logger_in_mcp_session_clean 5eaa9bdc8c3c6697e75b0e876f043d496dd8ee95 PiperOrigin-RevId: 867601662 --- .../adk/tools/mcp_tool/mcp_session_manager.py | 7 +++---- .../tools/mcp_tool/test_mcp_session_manager.py | 15 +++++++-------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/google/adk/tools/mcp_tool/mcp_session_manager.py b/src/google/adk/tools/mcp_tool/mcp_session_manager.py index 2571cb6d..a9bfcfbc 100644 --- a/src/google/adk/tools/mcp_tool/mcp_session_manager.py +++ b/src/google/adk/tools/mcp_tool/mcp_session_manager.py @@ -434,10 +434,9 @@ class MCPSessionManager: await exit_stack.aclose() except Exception as e: # Log the error but don't re-raise to avoid blocking shutdown - print( - 'Warning: Error during MCP session cleanup for' - f' {session_key}: {e}', - file=self._errlog, + logger.warning( + f'Error during MCP session cleanup for {session_key}', + exc_info=True, ) finally: del self._sessions[session_key] diff --git a/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py b/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py index 410e5282..cc2cb487 100644 --- a/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py +++ b/tests/unittests/tools/mcp_tool/test_mcp_session_manager.py @@ -388,7 +388,8 @@ class TestMCPSessionManager: assert len(manager._sessions) == 0 @pytest.mark.asyncio - async def test_close_with_errors(self): + @patch("google.adk.tools.mcp_tool.mcp_session_manager.logger") + async def test_close_with_errors(self, mock_logger): """Test cleanup when some sessions fail to close.""" manager = MCPSessionManager(self.mock_stdio_connection_params) @@ -403,9 +404,6 @@ class TestMCPSessionManager: manager._sessions["session1"] = (session1, exit_stack1) manager._sessions["session2"] = (session2, exit_stack2) - custom_errlog = StringIO() - manager._errlog = custom_errlog - # Should not raise exception await manager.close() @@ -413,10 +411,11 @@ class TestMCPSessionManager: exit_stack2.aclose.assert_called_once() assert len(manager._sessions) == 0 - # Error should be logged - error_output = custom_errlog.getvalue() - assert "Warning: Error during MCP session cleanup" in error_output - assert "Close error 1" in error_output + # Error should be logged via logger.warning + mock_logger.warning.assert_called_once() + args, kwargs = mock_logger.warning.call_args + assert "Error during MCP session cleanup for session1" in args[0] + assert kwargs.get("exc_info") @pytest.mark.asyncio @patch("google.adk.tools.mcp_tool.mcp_session_manager.stdio_client")