Skip to content

Commit ae720ad

Browse files
committed
Revert "Ensured no traceback at dispatcher boundary."
This reverts commit 8c30d12.
1 parent 8c30d12 commit ae720ad

2 files changed

Lines changed: 14 additions & 21 deletions

File tree

src/mcp/server/mcpserver/server.py

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1303,20 +1303,15 @@ async def get_prompt(
13031303

13041304
except PromptValidationError as e:
13051305
# Expected user-input validation failures, like missing required
1306-
# arguments, don't need a full traceback anywhere in request path.
1307-
1308-
# Raising `MCPError` here — the same pattern
1309-
# `_handle_read_resource` uses for `ResourceNotFoundError` above —
1310-
# lets `handler_exception_to_error_data` at the dispatcher
1311-
# boundary recognize this as expected too, so neither this layer
1312-
# nor the dispatcher's catch-all logs a traceback for it.
1306+
# arguments, don't need a full traceback. Log a concise warning
1307+
# instead of the exc_info dump reserved for unexpected errors.
13131308

13141309
# `Prompt.render` also raises a plain `ValueError` when the prompt
13151310
# function itself throws, so this narrower type is caught here
13161311
# instead of `ValueError` to avoid swallowing that traceback too.
13171312

13181313
logger.warning(f"Error getting prompt {name}: {e}")
1319-
raise MCPError(code=INVALID_PARAMS, message=str(e), data={"name": name}) from e
1314+
raise ValueError(str(e)) from e
13201315

13211316
except Exception as e:
13221317
logger.exception(f"Error getting prompt {name}")

tests/server/mcpserver/test_server.py

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1531,12 +1531,14 @@ def prompt_fn(name: str) -> str: ... # pragma: no branch
15311531
async def test_get_prompt_missing_args_logs_warning_without_traceback(
15321532
self, caplog: pytest.LogCaptureFixture
15331533
) -> None:
1534-
"""Regression for issue #3342: a missing-argument PromptValidationError is
1535-
an expected validation failure. `MCPServer.get_prompt` logs it as a plain
1536-
warning (no exc_info) and raises it as `MCPError`, so the dispatcher's own
1537-
catch-all — which would otherwise log a second, separate traceback for any
1538-
exception it doesn't recognize as expected — treats it as expected too.
1539-
No traceback anywhere in the real request path.
1534+
"""Regression for issue #3342: missing-argument ValueErrors are an expected
1535+
validation failure, so `MCPServer.get_prompt`'s own logger should emit
1536+
a plain warning without exc_info, instead of a full traceback.
1537+
1538+
Note: `jsonrpc_dispatcher` has a separate, intentional catch-all that
1539+
logs a traceback for any handler exception it doesn't recognize as
1540+
`MCPError`/`ValidationError`. However, that generic safety net is out of
1541+
scope here and unaffected by this fix PR #3347.
15401542
"""
15411543
mcp = MCPServer()
15421544

@@ -1545,17 +1547,13 @@ def prompt_fn(name: str) -> str: ... # pragma: no branch.
15451547

15461548
# In Python 3.14, coverage.py undercounts a branch when `caplog.at_level`
15471549
# wraps `async with Client(...): with pytest.raises(...): await ...` as
1548-
# a 4th nesting level around a single `await` statement.
1549-
# 3 levels of nesting is OK. But 4 is not.
1550-
1551-
caplog.set_level(logging.WARNING)
1550+
# a 4th nesting level around a single `await` statement (3 levels of
1551+
# nesting is OK; 4 is not). So `caplog.set_level` avoids extra `with` layer.
1552+
caplog.set_level(logging.WARNING, logger="mcp.server.mcpserver.server")
15521553
async with Client(mcp, mode="legacy") as client:
15531554
with pytest.raises(MCPError, match="Missing required arguments"):
15541555
await client.get_prompt("prompt_fn")
15551556

1556-
# No traceback anywhere in request path. Not just this module's logger.
1557-
assert not any(r.exc_info for r in caplog.records)
1558-
15591557
server_records = [r for r in caplog.records if r.name == "mcp.server.mcpserver.server"]
15601558
assert len(server_records) == 1
15611559

0 commit comments

Comments
 (0)