Fix log handler to accept any JSON-serializable data type (#2102)

This commit is contained in:
Jeremiah Lowin 2025-10-15 10:20:04 -07:00 committed by GitHub
commit 5988873601
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 65 additions and 17 deletions

View file

@ -16,8 +16,8 @@ LogHandler: TypeAlias = Callable[[LogMessage], Awaitable[None]]
async def default_log_handler(message: LogMessage) -> None:
"""Default handler that properly routes server log messages to appropriate log levels."""
msg = message.data.get("msg", str(message))
extra = message.data.get("extra", {})
# data can be any JSON-serializable type, not just a dict
data = message.data
# Map MCP log levels to Python logging levels
level_map = {
@ -40,8 +40,8 @@ async def default_log_handler(message: LogMessage) -> None:
if message.logger:
msg_prefix += f" ({message.logger})"
# Log with appropriate level and extra data
log_fn(msg=f"{msg_prefix}: {msg}", extra=extra)
# Log with appropriate level and data
log_fn(msg=f"{msg_prefix}: {data}")
def create_log_callback(handler: LogHandler | None = None) -> LoggingFnT:

View file

@ -91,7 +91,7 @@ class TestClientLogs:
class TestDefaultLogHandler:
"""Tests for default_log_handler bug fix (issue #1394)."""
"""Tests for default_log_handler with data as any JSON-serializable type."""
async def test_default_handler_routes_to_correct_levels(self):
"""Test that default_log_handler routes server logs to appropriate Python log levels."""
@ -129,11 +129,11 @@ class TestDefaultLogHandler:
# Reset mocks
mock_logger.reset_mock()
# Create log message
# Create log message with data as a string
log_msg = LoggingMessageNotificationParams(
level=level, # type: ignore[arg-type]
logger="test.logger",
data={"msg": msg, "extra": {"test_key": "test_value"}},
data=msg,
)
# Call handler
@ -141,8 +141,7 @@ class TestDefaultLogHandler:
# Verify correct method was called
expected_method.assert_called_once_with(
msg=f"Received {level.upper()} from server (test.logger): {msg}",
extra={"test_key": "test_value"},
msg=f"Received {level.upper()} from server (test.logger): {msg}"
)
async def test_default_handler_without_logger_name(self):
@ -159,17 +158,17 @@ class TestDefaultLogHandler:
log_msg = LoggingMessageNotificationParams(
level="info",
logger=None,
data={"msg": "Message without logger", "extra": {}},
data="Message without logger",
)
await default_log_handler(log_msg)
mock_logger.info.assert_called_once_with(
msg="Received INFO from server: Message without logger", extra={}
msg="Received INFO from server: Message without logger"
)
async def test_default_handler_with_missing_msg(self):
"""Test that default_log_handler handles missing 'msg' gracefully."""
async def test_default_handler_with_dict_data(self):
"""Test that default_log_handler handles dict data correctly."""
from unittest.mock import MagicMock, patch
from mcp.types import LoggingMessageNotificationParams
@ -182,13 +181,62 @@ class TestDefaultLogHandler:
log_msg = LoggingMessageNotificationParams(
level="info",
logger="test.logger",
data={"extra": {"key": "value"}}, # Missing 'msg' key
data={"key": "value", "count": 42},
)
await default_log_handler(log_msg)
# Should use str(message) as fallback
# Should log the entire dict as a string
mock_logger.info.assert_called_once()
call_args = mock_logger.info.call_args
assert "Received INFO from server" in call_args[1]["msg"]
assert call_args[1]["extra"] == {"key": "value"}
assert "Received INFO from server (test.logger):" in call_args[1]["msg"]
assert "key" in call_args[1]["msg"]
assert "value" in call_args[1]["msg"]
async def test_default_handler_with_list_data(self):
"""Test that default_log_handler handles list data correctly."""
from unittest.mock import MagicMock, patch
from mcp.types import LoggingMessageNotificationParams
from fastmcp.client.logging import default_log_handler
with patch("fastmcp.client.logging.from_server_logger") as mock_logger:
mock_logger.warning = MagicMock()
log_msg = LoggingMessageNotificationParams(
level="warning",
logger="test.logger",
data=["item1", "item2", "item3"],
)
await default_log_handler(log_msg)
# Should log the entire list as a string
mock_logger.warning.assert_called_once()
call_args = mock_logger.warning.call_args
assert "Received WARNING from server (test.logger):" in call_args[1]["msg"]
assert "item1" in call_args[1]["msg"]
async def test_default_handler_with_number_data(self):
"""Test that default_log_handler handles numeric data correctly."""
from unittest.mock import MagicMock, patch
from mcp.types import LoggingMessageNotificationParams
from fastmcp.client.logging import default_log_handler
with patch("fastmcp.client.logging.from_server_logger") as mock_logger:
mock_logger.error = MagicMock()
log_msg = LoggingMessageNotificationParams(
level="error",
logger=None,
data=404,
)
await default_log_handler(log_msg)
mock_logger.error.assert_called_once_with(
msg="Received ERROR from server: 404"
)