Split ValueError from unexpected errors during logs. - #3347
Conversation
ValueError only needs a warning. Unexpected errors need a full traceback.
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
AI-assisted analysis; posting under human responsibility. I think this still leaves the end-to-end traceback reported in #3342. The new regression test intentionally filters to raise ValueError(str(e)) from eAt the dispatcher boundary, logger.exception("handler for %r raised", req.method)So this removes the inner Could the expected-validation signal survive through the dispatcher boundary (or be translated to an exception type that boundary already treats as expected), and could the regression test assert that a missing required prompt argument produces no traceback anywhere in the actual request path, rather than only checking the Unexpected prompt-function exceptions should of course continue reaching the traceback path. Checked against PR head |
Now when PromptValidationError happens, no traceback shows up in request path.
This reverts commit 8c30d12.
|
Hi @keeltrace Thank you for commenting on this. I tried exactly what you suggested in 8c30d12 — raising It works for legacy A single exception raised from So I've reverted to raising |
Fixes #3342.
Motivation and Context
When a prompt is called without a required argument, the request is correctly rejected, but Python-SDK logs validation error with a full traceback.
Validation error doesn't need a full traceback, which is typically reserved for unexpected errors.
Thus, there needs a fix to separate validation error from unexpected errors.
How Has This Been Tested?
Yes. Two tests are added inside
tests/server/mcpserver/test_server.py:test_get_prompt_missing_args_logs_warning_without_traceback— it asserts that missing-argument ValueErrors are an expected validation failure, so logger writes a plain warning withoutexc_info. No full traceback at all.test_get_prompt_unexpected_error_still_logs_traceback— it asserts that a prompt function raising an unexpected exception must still have logs containing a full traceback.Breaking Changes
No breaking changes at all. This PR #3347 is a pure bug fix.
Types of changes
Checklist
help wanted, or I'm a maintainer)Additional context
1. AI disclosure
I used Claude Sonnet 5 during chat sessions as a pair-programming and debugging assistant for:
PromptValidationErrorinsidesrc/mcp/server/mcpserver/prompts/base.py.2. Notes
I left a claim comment on issue #3342 before starting as repo convention per
CONTRIBUTING.md. As of opening this PR, no one else has commented on this issue, and it hasn't been formally assigned or labeled ready for work. Currently PR #3347 is a draft, so bug fix and CI status are verifiable and visible. Waiting on maintainers' buy-in.3. Final
I have reviewed, tested, and can explain every change in my own words.