From b83a22272ec676183f6350a7e7ac27c09944cf89 Mon Sep 17 00:00:00 2001 From: Jack Yao <105488074+StarsExpress@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:27:37 -0400 Subject: [PATCH 1/7] Split ValueError from unexpected errors during logs. ValueError only needs a warning. Unexpected errors need a full traceback. --- src/mcp/server/mcpserver/server.py | 10 +++++++++ tests/server/mcpserver/test_server.py | 30 +++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index b3a3cb3bcd..fd5cdb8e21 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -1283,6 +1283,7 @@ async def get_prompt( """ if context is None: context = Context(mcp_server=self, subscriptions=self._subscriptions) + try: prompt = self._prompt_manager.get_prompt(name) if not prompt: @@ -1296,8 +1297,17 @@ async def get_prompt( description=prompt.description, messages=pydantic_core.to_jsonable_python(rendered), ) + except MCPError: raise + + except ValueError as e: + # Expected user-input validation failures, like missing required + # arguments, don't need a full traceback for unexpected errors. + # Instead, log a concise warning without exc_info. + logger.warning(f"Error getting prompt {name}: {e}") + raise ValueError(str(e)) from e + except Exception as e: logger.exception(f"Error getting prompt {name}") raise ValueError(str(e)) from e diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index f9055a4ed4..67e232e7c6 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -1,4 +1,5 @@ import base64 +import logging from pathlib import Path from types import SimpleNamespace from typing import Any @@ -1527,6 +1528,35 @@ def prompt_fn(name: str) -> str: ... # pragma: no branch with pytest.raises(MCPError, match="Missing required arguments"): await client.get_prompt("prompt_fn") + async def test_get_prompt_missing_args_logs_warning_without_traceback( + self, caplog: pytest.LogCaptureFixture + ) -> None: + """Regression for issue #3342: missing-argument ValueErrors are an expected + validation failure, so `MCPServer.get_prompt`'s own logger should emit + a plain warning without exc_info, instead of a full traceback. + + Note: `jsonrpc_dispatcher` has a separate, intentional catch-all that + logs a traceback for any handler exception it doesn't recognize as + `MCPError`/`ValidationError`. However, that generic safety net is out of + scope here and unaffected by this fix PR #3347. + """ + mcp = MCPServer() + + @mcp.prompt() + def prompt_fn(name: str) -> str: ... # Pragma: no branch. + + with caplog.at_level(logging.WARNING, logger="mcp.server.mcpserver.server"): + async with Client(mcp, mode="legacy") as client: + with pytest.raises(MCPError, match="Missing required arguments"): + await client.get_prompt("prompt_fn") + + server_records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert len(server_records) == 1 + + # ValueError should have warning log without exc_info. + assert server_records[0].levelno == logging.WARNING + assert not server_records[0].exc_info + async def test_resource_decorator_rfc6570_reserved_expansion(): # Regression: old regex-based param extraction couldn't see `path` From 809dafbd97711458b3709843bf878d3cd54cff4c Mon Sep 17 00:00:00 2001 From: Jack Yao <105488074+StarsExpress@users.noreply.github.com> Date: Thu, 20 Aug 2026 15:50:01 -0400 Subject: [PATCH 2/7] Created PromptValidationError for user-input validation failures. --- src/mcp/server/mcpserver/prompts/__init__.py | 4 ++-- src/mcp/server/mcpserver/prompts/base.py | 16 +++++++++++-- src/mcp/server/mcpserver/server.py | 13 ++++++---- tests/server/mcpserver/test_server.py | 25 +++++++++++++++++++- 4 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/mcp/server/mcpserver/prompts/__init__.py b/src/mcp/server/mcpserver/prompts/__init__.py index 763726964c..5ea52ce2fc 100644 --- a/src/mcp/server/mcpserver/prompts/__init__.py +++ b/src/mcp/server/mcpserver/prompts/__init__.py @@ -1,4 +1,4 @@ -from .base import Prompt +from .base import Prompt, PromptValidationError from .manager import PromptManager -__all__ = ["Prompt", "PromptManager"] +__all__ = ["Prompt", "PromptManager", "PromptValidationError"] diff --git a/src/mcp/server/mcpserver/prompts/base.py b/src/mcp/server/mcpserver/prompts/base.py index d30c0b3c60..2ee0f95354 100644 --- a/src/mcp/server/mcpserver/prompts/base.py +++ b/src/mcp/server/mcpserver/prompts/base.py @@ -22,6 +22,18 @@ from mcp.server.mcpserver.context import Context +class PromptValidationError(ValueError): + """Raised when prompt arguments fail validation (e.g. a required argument is missing). + + A `ValueError` subclass so existing callers that catch `ValueError` are + unaffected. It exists to distinguish this expected, user-input validation + failure from the generic `ValueError` `Prompt.render` also raises when the + prompt function itself raises an unexpected exception - callers that log + the two differently (see `MCPServer.get_prompt`) can `except` this + specifically without downgrading a genuine crash to a quiet warning. + """ + + class Message(BaseModel): """Base class for all prompt messages. @@ -166,7 +178,7 @@ async def render( through unchanged so the multi-round-trip flow reaches the client. Raises: - ValueError: If required arguments are missing, or if rendering fails. + PromptValidationError: If required arguments are missing, or if rendering fails. """ # Validate required arguments if self.arguments: @@ -174,7 +186,7 @@ async def render( provided = set(arguments or {}) missing = required - provided if missing: - raise ValueError(f"Missing required arguments: {missing}") + raise PromptValidationError(f"Missing required arguments: {missing}") try: # Add context to arguments if needed diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index fd5cdb8e21..a3536b1da9 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -72,7 +72,7 @@ from mcp.server.lowlevel.server import lifespan as default_lifespan from mcp.server.mcpserver.context import Context from mcp.server.mcpserver.exceptions import ResourceError, ResourceNotFoundError -from mcp.server.mcpserver.prompts import Prompt, PromptManager +from mcp.server.mcpserver.prompts import Prompt, PromptManager, PromptValidationError from mcp.server.mcpserver.resources import ( DEFAULT_RESOURCE_SECURITY, FunctionResource, @@ -1301,10 +1301,15 @@ async def get_prompt( except MCPError: raise - except ValueError as e: + except PromptValidationError as e: # Expected user-input validation failures, like missing required - # arguments, don't need a full traceback for unexpected errors. - # Instead, log a concise warning without exc_info. + # arguments, don't need a full traceback. Log a concise warning + # instead of the exc_info dump reserved for unexpected errors. + + # `Prompt.render` also raises a plain `ValueError` when the prompt + # function itself throws, so this narrower type is caught here + # instead of `ValueError` to avoid swallowing that traceback too. + logger.warning(f"Error getting prompt {name}: {e}") raise ValueError(str(e)) from e diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index 67e232e7c6..eb84a78e1a 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -1543,7 +1543,7 @@ async def test_get_prompt_missing_args_logs_warning_without_traceback( mcp = MCPServer() @mcp.prompt() - def prompt_fn(name: str) -> str: ... # Pragma: no branch. + def prompt_fn(name: str) -> str: ... # pragma: no branch. with caplog.at_level(logging.WARNING, logger="mcp.server.mcpserver.server"): async with Client(mcp, mode="legacy") as client: @@ -1557,6 +1557,29 @@ def prompt_fn(name: str) -> str: ... # Pragma: no branch. assert server_records[0].levelno == logging.WARNING assert not server_records[0].exc_info + async def test_get_prompt_unexpected_error_still_logs_traceback(self, caplog: pytest.LogCaptureFixture) -> None: + """A prompt function raising an unexpected (non-validation) exception must + still be logged with a full traceback, even though `Prompt.render` also + wraps it as a plain `ValueError` — same wire type as the missing-argument + case, but not a `PromptValidationError`, so it must not be downgraded.""" + mcp = MCPServer() + + @mcp.prompt() + def prompt_fn() -> str: + raise KeyError("boom") + + with caplog.at_level(logging.WARNING, logger="mcp.server.mcpserver.server"): + async with Client(mcp, mode="legacy") as client: + with pytest.raises(MCPError): + await client.get_prompt("prompt_fn") + + server_records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] + assert len(server_records) == 1 + + # Unexpected errors should have error log with exc_info. + assert server_records[0].levelno == logging.ERROR + assert server_records[0].exc_info is not None + async def test_resource_decorator_rfc6570_reserved_expansion(): # Regression: old regex-based param extraction couldn't see `path` From 445e6e3261099eb373c25e780f3fb171d237f8ea Mon Sep 17 00:00:00 2001 From: Jack Yao <105488074+StarsExpress@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:09:53 -0400 Subject: [PATCH 3/7] Re-trigger CI. From 97ee03f93634b6f4cad8ceb004e51acd6f79f88e Mon Sep 17 00:00:00 2001 From: Jack Yao <105488074+StarsExpress@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:28:54 -0400 Subject: [PATCH 4/7] Reduced caplog.at_level layers from 4 to 3. --- tests/server/mcpserver/test_server.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index eb84a78e1a..1e6121e045 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -1545,15 +1545,20 @@ async def test_get_prompt_missing_args_logs_warning_without_traceback( @mcp.prompt() def prompt_fn(name: str) -> str: ... # pragma: no branch. - with caplog.at_level(logging.WARNING, logger="mcp.server.mcpserver.server"): - async with Client(mcp, mode="legacy") as client: - with pytest.raises(MCPError, match="Missing required arguments"): - await client.get_prompt("prompt_fn") + # In Python 3.14, coverage.py undercounts a branch when `caplog.at_level` + # wraps `async with Client(...): with pytest.raises(...): await ...` as + # a 4th nesting level around a single `await` statement (3 levels of + # nesting is OK; 4 is not). So `caplog.set_level` avoids extra `with` layer. + caplog.set_level(logging.WARNING, logger="mcp.server.mcpserver.server") + async with Client(mcp, mode="legacy") as client: + with pytest.raises(MCPError, match="Missing required arguments"): + await client.get_prompt("prompt_fn") server_records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] assert len(server_records) == 1 - # ValueError should have warning log without exc_info. + # Missing-argument PromptValidationError, which is a ValueError subclass, + # logs as a plain warning, with no exc_info/traceback at all. assert server_records[0].levelno == logging.WARNING assert not server_records[0].exc_info @@ -1568,10 +1573,14 @@ async def test_get_prompt_unexpected_error_still_logs_traceback(self, caplog: py def prompt_fn() -> str: raise KeyError("boom") - with caplog.at_level(logging.WARNING, logger="mcp.server.mcpserver.server"): - async with Client(mcp, mode="legacy") as client: - with pytest.raises(MCPError): - await client.get_prompt("prompt_fn") + # In Python 3.14, coverage.py undercounts a branch when `caplog.at_level` + # wraps `async with Client(...): with pytest.raises(...): await ...` as + # a 4th nesting level around a single `await` statement (3 levels of + # nesting is OK; 4 is not). So `caplog.set_level` avoids extra `with` layer. + caplog.set_level(logging.WARNING, logger="mcp.server.mcpserver.server") + async with Client(mcp, mode="legacy") as client: + with pytest.raises(MCPError): + await client.get_prompt("prompt_fn") server_records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] assert len(server_records) == 1 From 98885e2daf48dbfce77afc7d9144eb88cda266a9 Mon Sep 17 00:00:00 2001 From: Jack Yao <105488074+StarsExpress@users.noreply.github.com> Date: Thu, 20 Aug 2026 16:55:55 -0400 Subject: [PATCH 5/7] Clarified docstring and enhanced readability. --- src/mcp/server/mcpserver/prompts/base.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/mcp/server/mcpserver/prompts/base.py b/src/mcp/server/mcpserver/prompts/base.py index 2ee0f95354..b0f4d07ec8 100644 --- a/src/mcp/server/mcpserver/prompts/base.py +++ b/src/mcp/server/mcpserver/prompts/base.py @@ -178,7 +178,8 @@ async def render( through unchanged so the multi-round-trip flow reaches the client. Raises: - PromptValidationError: If required arguments are missing, or if rendering fails. + PromptValidationError: If required arguments are missing. + ValueError: If the prompt function itself raises an unexpected exception. """ # Validate required arguments if self.arguments: @@ -195,6 +196,7 @@ async def render( fn = self.fn if is_async_callable(fn): result = await fn(**call_args) + else: result = await anyio.to_thread.run_sync(functools.partial(self.fn, **call_args)) @@ -210,16 +212,21 @@ async def render( for msg in result: # type: ignore[reportUnknownVariableType] if isinstance(msg, Message): messages.append(msg) + elif isinstance(msg, dict): messages.append(message_validator.validate_python(msg)) + elif isinstance(msg, str | ContentBlock | Image | Audio): # bare content is one user message messages.append(UserMessage(msg)) + else: # pragma: no cover content = pydantic_core.to_json(msg, fallback=str, indent=2).decode() messages.append(Message(role="user", content=content)) return messages + except MCPError: raise + except Exception as e: raise ValueError(f"Error rendering prompt {self.name}: {e}") From 8c30d12ec1016b72d31a71d4a7b05ad3ab6e46e3 Mon Sep 17 00:00:00 2001 From: Jack Yao <105488074+StarsExpress@users.noreply.github.com> Date: Thu, 20 Aug 2026 21:04:50 -0400 Subject: [PATCH 6/7] Ensured no traceback at dispatcher boundary. Now when PromptValidationError happens, no traceback shows up in request path. --- src/mcp/server/mcpserver/server.py | 11 ++++++++--- tests/server/mcpserver/test_server.py | 24 +++++++++++++----------- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index a3536b1da9..7b04ca5765 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -1303,15 +1303,20 @@ async def get_prompt( except PromptValidationError as e: # Expected user-input validation failures, like missing required - # arguments, don't need a full traceback. Log a concise warning - # instead of the exc_info dump reserved for unexpected errors. + # arguments, don't need a full traceback anywhere in request path. + + # Raising `MCPError` here — the same pattern + # `_handle_read_resource` uses for `ResourceNotFoundError` above — + # lets `handler_exception_to_error_data` at the dispatcher + # boundary recognize this as expected too, so neither this layer + # nor the dispatcher's catch-all logs a traceback for it. # `Prompt.render` also raises a plain `ValueError` when the prompt # function itself throws, so this narrower type is caught here # instead of `ValueError` to avoid swallowing that traceback too. logger.warning(f"Error getting prompt {name}: {e}") - raise ValueError(str(e)) from e + raise MCPError(code=INVALID_PARAMS, message=str(e), data={"name": name}) from e except Exception as e: logger.exception(f"Error getting prompt {name}") diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index 1e6121e045..42c46ea7ad 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -1531,14 +1531,12 @@ def prompt_fn(name: str) -> str: ... # pragma: no branch async def test_get_prompt_missing_args_logs_warning_without_traceback( self, caplog: pytest.LogCaptureFixture ) -> None: - """Regression for issue #3342: missing-argument ValueErrors are an expected - validation failure, so `MCPServer.get_prompt`'s own logger should emit - a plain warning without exc_info, instead of a full traceback. - - Note: `jsonrpc_dispatcher` has a separate, intentional catch-all that - logs a traceback for any handler exception it doesn't recognize as - `MCPError`/`ValidationError`. However, that generic safety net is out of - scope here and unaffected by this fix PR #3347. + """Regression for issue #3342: a missing-argument PromptValidationError is + an expected validation failure. `MCPServer.get_prompt` logs it as a plain + warning (no exc_info) and raises it as `MCPError`, so the dispatcher's own + catch-all — which would otherwise log a second, separate traceback for any + exception it doesn't recognize as expected — treats it as expected too. + No traceback anywhere in the real request path. """ mcp = MCPServer() @@ -1547,13 +1545,17 @@ def prompt_fn(name: str) -> str: ... # pragma: no branch. # In Python 3.14, coverage.py undercounts a branch when `caplog.at_level` # wraps `async with Client(...): with pytest.raises(...): await ...` as - # a 4th nesting level around a single `await` statement (3 levels of - # nesting is OK; 4 is not). So `caplog.set_level` avoids extra `with` layer. - caplog.set_level(logging.WARNING, logger="mcp.server.mcpserver.server") + # a 4th nesting level around a single `await` statement. + # 3 levels of nesting is OK. But 4 is not. + + caplog.set_level(logging.WARNING) async with Client(mcp, mode="legacy") as client: with pytest.raises(MCPError, match="Missing required arguments"): await client.get_prompt("prompt_fn") + # No traceback anywhere in request path. Not just this module's logger. + assert not any(r.exc_info for r in caplog.records) + server_records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] assert len(server_records) == 1 From ae720adaf49cfbf8d5b8f5fad228e5ba1b4205fd Mon Sep 17 00:00:00 2001 From: Jack Yao <105488074+StarsExpress@users.noreply.github.com> Date: Thu, 20 Aug 2026 21:14:07 -0400 Subject: [PATCH 7/7] Revert "Ensured no traceback at dispatcher boundary." This reverts commit 8c30d12ec1016b72d31a71d4a7b05ad3ab6e46e3. --- src/mcp/server/mcpserver/server.py | 11 +++-------- tests/server/mcpserver/test_server.py | 24 +++++++++++------------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/mcp/server/mcpserver/server.py b/src/mcp/server/mcpserver/server.py index 7b04ca5765..a3536b1da9 100644 --- a/src/mcp/server/mcpserver/server.py +++ b/src/mcp/server/mcpserver/server.py @@ -1303,20 +1303,15 @@ async def get_prompt( except PromptValidationError as e: # Expected user-input validation failures, like missing required - # arguments, don't need a full traceback anywhere in request path. - - # Raising `MCPError` here — the same pattern - # `_handle_read_resource` uses for `ResourceNotFoundError` above — - # lets `handler_exception_to_error_data` at the dispatcher - # boundary recognize this as expected too, so neither this layer - # nor the dispatcher's catch-all logs a traceback for it. + # arguments, don't need a full traceback. Log a concise warning + # instead of the exc_info dump reserved for unexpected errors. # `Prompt.render` also raises a plain `ValueError` when the prompt # function itself throws, so this narrower type is caught here # instead of `ValueError` to avoid swallowing that traceback too. logger.warning(f"Error getting prompt {name}: {e}") - raise MCPError(code=INVALID_PARAMS, message=str(e), data={"name": name}) from e + raise ValueError(str(e)) from e except Exception as e: logger.exception(f"Error getting prompt {name}") diff --git a/tests/server/mcpserver/test_server.py b/tests/server/mcpserver/test_server.py index 42c46ea7ad..1e6121e045 100644 --- a/tests/server/mcpserver/test_server.py +++ b/tests/server/mcpserver/test_server.py @@ -1531,12 +1531,14 @@ def prompt_fn(name: str) -> str: ... # pragma: no branch async def test_get_prompt_missing_args_logs_warning_without_traceback( self, caplog: pytest.LogCaptureFixture ) -> None: - """Regression for issue #3342: a missing-argument PromptValidationError is - an expected validation failure. `MCPServer.get_prompt` logs it as a plain - warning (no exc_info) and raises it as `MCPError`, so the dispatcher's own - catch-all — which would otherwise log a second, separate traceback for any - exception it doesn't recognize as expected — treats it as expected too. - No traceback anywhere in the real request path. + """Regression for issue #3342: missing-argument ValueErrors are an expected + validation failure, so `MCPServer.get_prompt`'s own logger should emit + a plain warning without exc_info, instead of a full traceback. + + Note: `jsonrpc_dispatcher` has a separate, intentional catch-all that + logs a traceback for any handler exception it doesn't recognize as + `MCPError`/`ValidationError`. However, that generic safety net is out of + scope here and unaffected by this fix PR #3347. """ mcp = MCPServer() @@ -1545,17 +1547,13 @@ def prompt_fn(name: str) -> str: ... # pragma: no branch. # In Python 3.14, coverage.py undercounts a branch when `caplog.at_level` # wraps `async with Client(...): with pytest.raises(...): await ...` as - # a 4th nesting level around a single `await` statement. - # 3 levels of nesting is OK. But 4 is not. - - caplog.set_level(logging.WARNING) + # a 4th nesting level around a single `await` statement (3 levels of + # nesting is OK; 4 is not). So `caplog.set_level` avoids extra `with` layer. + caplog.set_level(logging.WARNING, logger="mcp.server.mcpserver.server") async with Client(mcp, mode="legacy") as client: with pytest.raises(MCPError, match="Missing required arguments"): await client.get_prompt("prompt_fn") - # No traceback anywhere in request path. Not just this module's logger. - assert not any(r.exc_info for r in caplog.records) - server_records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"] assert len(server_records) == 1