Spaces:
Running
Running
Fix client-side logging bug #1394 (#1397)
Browse files- src/fastmcp/client/logging.py +25 -1
- tests/client/test_logs.py +103 -0
src/fastmcp/client/logging.py
CHANGED
|
@@ -13,7 +13,31 @@ LogHandler: TypeAlias = Callable[[LogMessage], Awaitable[None]]
|
|
| 13 |
|
| 14 |
|
| 15 |
async def default_log_handler(message: LogMessage) -> None:
|
| 16 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 17 |
|
| 18 |
|
| 19 |
def create_log_callback(handler: LogHandler | None = None) -> LoggingFnT:
|
|
|
|
| 13 |
|
| 14 |
|
| 15 |
async def default_log_handler(message: LogMessage) -> None:
|
| 16 |
+
"""Default handler that properly routes server log messages to appropriate log levels."""
|
| 17 |
+
msg = message.data.get("msg", str(message))
|
| 18 |
+
extra = message.data.get("extra", {})
|
| 19 |
+
|
| 20 |
+
# Map MCP log levels to Python logging levels
|
| 21 |
+
level_map = {
|
| 22 |
+
"debug": logger.debug,
|
| 23 |
+
"info": logger.info,
|
| 24 |
+
"notice": logger.info, # Python doesn't have 'notice', map to info
|
| 25 |
+
"warning": logger.warning,
|
| 26 |
+
"error": logger.error,
|
| 27 |
+
"critical": logger.critical,
|
| 28 |
+
"alert": logger.critical, # Map alert to critical
|
| 29 |
+
"emergency": logger.critical, # Map emergency to critical
|
| 30 |
+
}
|
| 31 |
+
|
| 32 |
+
# Get the appropriate logging function based on the message level
|
| 33 |
+
log_fn = level_map.get(message.level.lower(), logger.info)
|
| 34 |
+
|
| 35 |
+
# Include logger name if available
|
| 36 |
+
if message.logger:
|
| 37 |
+
msg = f"[{message.logger}] {msg}"
|
| 38 |
+
|
| 39 |
+
# Log with appropriate level and extra data
|
| 40 |
+
log_fn(f"Server log: {msg}", extra=extra)
|
| 41 |
|
| 42 |
|
| 43 |
def create_log_callback(handler: LogHandler | None = None) -> LoggingFnT:
|
tests/client/test_logs.py
CHANGED
|
@@ -88,3 +88,106 @@ class TestClientLogs:
|
|
| 88 |
assert caplog.records[0].levelname == "INFO"
|
| 89 |
assert caplog.records[1].msg == "this is a warning log"
|
| 90 |
assert caplog.records[1].levelname == "WARNING"
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 88 |
assert caplog.records[0].levelname == "INFO"
|
| 89 |
assert caplog.records[1].msg == "this is a warning log"
|
| 90 |
assert caplog.records[1].levelname == "WARNING"
|
| 91 |
+
|
| 92 |
+
|
| 93 |
+
class TestDefaultLogHandler:
|
| 94 |
+
"""Tests for default_log_handler bug fix (issue #1394)."""
|
| 95 |
+
|
| 96 |
+
async def test_default_handler_routes_to_correct_levels(self):
|
| 97 |
+
"""Test that default_log_handler routes server logs to appropriate Python log levels."""
|
| 98 |
+
from unittest.mock import MagicMock, patch
|
| 99 |
+
|
| 100 |
+
from mcp.types import LoggingMessageNotificationParams
|
| 101 |
+
|
| 102 |
+
from fastmcp.client.logging import default_log_handler
|
| 103 |
+
|
| 104 |
+
with patch("fastmcp.client.logging.logger") as mock_logger:
|
| 105 |
+
# Set up mock methods
|
| 106 |
+
mock_logger.debug = MagicMock()
|
| 107 |
+
mock_logger.info = MagicMock()
|
| 108 |
+
mock_logger.warning = MagicMock()
|
| 109 |
+
mock_logger.error = MagicMock()
|
| 110 |
+
mock_logger.critical = MagicMock()
|
| 111 |
+
|
| 112 |
+
# Test each log level
|
| 113 |
+
test_cases = [
|
| 114 |
+
("debug", mock_logger.debug, "Debug message"),
|
| 115 |
+
("info", mock_logger.info, "Info message"),
|
| 116 |
+
("notice", mock_logger.info, "Notice message"), # notice -> info
|
| 117 |
+
("warning", mock_logger.warning, "Warning message"),
|
| 118 |
+
("error", mock_logger.error, "Error message"),
|
| 119 |
+
("critical", mock_logger.critical, "Critical message"),
|
| 120 |
+
("alert", mock_logger.critical, "Alert message"), # alert -> critical
|
| 121 |
+
(
|
| 122 |
+
"emergency",
|
| 123 |
+
mock_logger.critical,
|
| 124 |
+
"Emergency message",
|
| 125 |
+
), # emergency -> critical
|
| 126 |
+
]
|
| 127 |
+
|
| 128 |
+
for level, expected_method, msg in test_cases:
|
| 129 |
+
# Reset mocks
|
| 130 |
+
mock_logger.reset_mock()
|
| 131 |
+
|
| 132 |
+
# Create log message
|
| 133 |
+
log_msg = LoggingMessageNotificationParams(
|
| 134 |
+
level=level, # type: ignore[arg-type]
|
| 135 |
+
logger="test.logger",
|
| 136 |
+
data={"msg": msg, "extra": {"test_key": "test_value"}},
|
| 137 |
+
)
|
| 138 |
+
|
| 139 |
+
# Call handler
|
| 140 |
+
await default_log_handler(log_msg)
|
| 141 |
+
|
| 142 |
+
# Verify correct method was called
|
| 143 |
+
expected_method.assert_called_once_with(
|
| 144 |
+
f"Server log: [test.logger] {msg}", extra={"test_key": "test_value"}
|
| 145 |
+
)
|
| 146 |
+
|
| 147 |
+
async def test_default_handler_without_logger_name(self):
|
| 148 |
+
"""Test that default_log_handler works when logger name is None."""
|
| 149 |
+
from unittest.mock import MagicMock, patch
|
| 150 |
+
|
| 151 |
+
from mcp.types import LoggingMessageNotificationParams
|
| 152 |
+
|
| 153 |
+
from fastmcp.client.logging import default_log_handler
|
| 154 |
+
|
| 155 |
+
with patch("fastmcp.client.logging.logger") as mock_logger:
|
| 156 |
+
mock_logger.info = MagicMock()
|
| 157 |
+
|
| 158 |
+
log_msg = LoggingMessageNotificationParams(
|
| 159 |
+
level="info",
|
| 160 |
+
logger=None,
|
| 161 |
+
data={"msg": "Message without logger", "extra": {}},
|
| 162 |
+
)
|
| 163 |
+
|
| 164 |
+
await default_log_handler(log_msg)
|
| 165 |
+
|
| 166 |
+
mock_logger.info.assert_called_once_with(
|
| 167 |
+
"Server log: Message without logger", extra={}
|
| 168 |
+
)
|
| 169 |
+
|
| 170 |
+
async def test_default_handler_with_missing_msg(self):
|
| 171 |
+
"""Test that default_log_handler handles missing 'msg' gracefully."""
|
| 172 |
+
from unittest.mock import MagicMock, patch
|
| 173 |
+
|
| 174 |
+
from mcp.types import LoggingMessageNotificationParams
|
| 175 |
+
|
| 176 |
+
from fastmcp.client.logging import default_log_handler
|
| 177 |
+
|
| 178 |
+
with patch("fastmcp.client.logging.logger") as mock_logger:
|
| 179 |
+
mock_logger.info = MagicMock()
|
| 180 |
+
|
| 181 |
+
log_msg = LoggingMessageNotificationParams(
|
| 182 |
+
level="info",
|
| 183 |
+
logger="test.logger",
|
| 184 |
+
data={"extra": {"key": "value"}}, # Missing 'msg' key
|
| 185 |
+
)
|
| 186 |
+
|
| 187 |
+
await default_log_handler(log_msg)
|
| 188 |
+
|
| 189 |
+
# Should use str(message) as fallback
|
| 190 |
+
mock_logger.info.assert_called_once()
|
| 191 |
+
call_args = mock_logger.info.call_args
|
| 192 |
+
assert "Server log:" in call_args[0][0]
|
| 193 |
+
assert call_args[1]["extra"] == {"key": "value"}
|