From 055c37ef109480b64e9e10b7b0ab1f2b6c726bab Mon Sep 17 00:00:00 2001
From: basilalshukaili <basilalshukaili@gmail.com>
Date: Tue, 2 Jun 2026 20:06:27 +0000
Subject: [PATCH] fix(time): re-raise McpError in call_tool so INVALID_PARAMS
 error code is preserved

The call_tool handler had a broad `except Exception as e` block that caught
`McpError` (raised by `get_zoneinfo` for unknown timezone names) and
re-raised it as a plain `ValueError`. This silently discarded the structured
INVALID_PARAMS error code that the MCP framework uses to distinguish client
errors from server errors.

Fix: add an explicit `except McpError: raise` guard before the broad handler
so McpError propagates to the MCP transport layer with its code intact.

Also fix a minor description typo: 'in a specific timezones' -> 'in a specific timezone'.

Tests: add three focused assertions verifying that McpError carries
INVALID_PARAMS when an invalid timezone is supplied to both tools.
---
 src/time/src/mcp_server_time/server.py |  4 +++-
 src/time/test/time_server_test.py      | 31 ++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/src/time/src/mcp_server_time/server.py b/src/time/src/mcp_server_time/server.py
index 83e97af333..2fe861dbd3 100644
--- a/src/time/src/mcp_server_time/server.py
+++ b/src/time/src/mcp_server_time/server.py
@@ -131,7 +131,7 @@ async def list_tools() -> list[Tool]:
         return [
             Tool(
                 name=TimeTools.GET_CURRENT_TIME.value,
-                description="Get current time in a specific timezones",
+                description="Get current time in a specific timezone",
                 inputSchema={
                     "type": "object",
                     "properties": {
@@ -212,6 +212,8 @@ async def call_tool(
                 TextContent(type="text", text=json.dumps(result.model_dump(), indent=2))
             ]
 
+        except McpError:
+            raise
         except Exception as e:
             raise ValueError(f"Error processing mcp-server-time query: {str(e)}")
 
diff --git a/src/time/test/time_server_test.py b/src/time/test/time_server_test.py
index 8d963508d7..3a506505f9 100644
--- a/src/time/test/time_server_test.py
+++ b/src/time/test/time_server_test.py
@@ -526,3 +526,34 @@ def test_get_local_tz_various_timezones(mock_get_localzone, timezone_name):
     result = get_local_tz()
     assert str(result) == timezone_name
     assert isinstance(result, ZoneInfo)
+
+
+def test_get_current_time_invalid_timezone_raises_invalid_params():
+    """McpError for an unknown timezone must carry the INVALID_PARAMS error code.
+
+    This guards against the call_tool handler accidentally catching McpError and
+    re-raising it as a plain ValueError (which loses the structured error code).
+    """
+    from mcp.types import INVALID_PARAMS as MCP_INVALID_PARAMS
+    time_server = TimeServer()
+    with pytest.raises(McpError) as exc_info:
+        time_server.get_current_time("Bad/Timezone")
+    assert exc_info.value.error.code == MCP_INVALID_PARAMS
+
+
+def test_convert_time_invalid_source_timezone_raises_invalid_params():
+    """McpError for an invalid source timezone must carry the INVALID_PARAMS error code."""
+    from mcp.types import INVALID_PARAMS as MCP_INVALID_PARAMS
+    time_server = TimeServer()
+    with pytest.raises(McpError) as exc_info:
+        time_server.convert_time("Bad/Source", "12:00", "Europe/London")
+    assert exc_info.value.error.code == MCP_INVALID_PARAMS
+
+
+def test_convert_time_invalid_target_timezone_raises_invalid_params():
+    """McpError for an invalid target timezone must carry the INVALID_PARAMS error code."""
+    from mcp.types import INVALID_PARAMS as MCP_INVALID_PARAMS
+    time_server = TimeServer()
+    with pytest.raises(McpError) as exc_info:
+        time_server.convert_time("Europe/London", "12:00", "Bad/Target")
+    assert exc_info.value.error.code == MCP_INVALID_PARAMS