feat: handle error from the initialize middleware#2531
feat: handle error from the initialize middleware#2531jlowin merged 4 commits intoPrefectHQ:mainfrom
Conversation
| with responder: | ||
| await responder.respond(e.error) | ||
| else: | ||
| # Don't re-raise: prevents responding to initialize request twice |
There was a problem hiding this comment.
raising here will cause the session to write error directly to write_stream.
WalkthroughServer middleware handling now catches Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/fastmcp/server/low_level.py (1)
107-122: Consider usinglogging.exceptionfor better debugging.The error handling logic correctly prevents duplicate responses by checking
responder._completedbefore attempting to respond with the error. However, when logging the error at line 119, consider usinglogging.exceptioninstead oflogging.errorto automatically include the traceback, which aids debugging.Apply this diff:
else: # Don't re-raise: prevents responding to initialize request twice - logger.error( + logger.exception( "Received McpError but responder is already completed. " "Cannot send error response as response was already sent." )Note: The code accesses the private
_completedattribute (line 114). While this appears intentional based on the past review comment referencing the MCP SDK's session implementation, be aware that relying on private attributes may be fragile if the upstream library changes its internals.Based on static analysis hints from Ruff (TRY400).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/server/middleware/test_initialization_middleware.pyis excluded by none and included by none
📒 Files selected for processing (2)
docs/servers/middleware.mdx(2 hunks)src/fastmcp/server/low_level.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/middleware.mdx
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must be version ≥3.10 with full type annotations
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if they're shorter
Never use bareexceptin code - be specific with exception types
Files:
src/fastmcp/server/low_level.py
🧬 Code graph analysis (1)
src/fastmcp/server/low_level.py (2)
src/fastmcp/server/context.py (1)
fastmcp(152-157)src/fastmcp/server/server.py (1)
_apply_middleware(437-446)
🪛 Ruff (0.14.7)
src/fastmcp/server/low_level.py
119-122: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (2)
docs/servers/middleware.mdx (1)
111-134: Excellent documentation of initialization error handling.The
InitializationMiddlewareexample clearly demonstrates the correct pattern for rejecting clients beforecall_next(), and the warning appropriately explains the constraint on error propagation after the responder completes. This aligns perfectly with the implementation changes and provides users with actionable guidance.As per coding guidelines, the documentation uses clear, direct language with active voice and provides a complete, runnable code example with proper error handling.
src/fastmcp/server/low_level.py (1)
10-10: LGTM: Import necessary for error handling.The
McpErrorimport is required for the new exception handling in the initialization flow.
Test Failure AnalysisSummary: Windows-specific pytest worker crash on Python 3.10 when running Root Cause: The test failure is a Windows-specific infrastructure issue with pytest-xdist workers, not a direct failure of the test logic. The test spawns a subprocess using The PR's new error handling code in Suggested Fix: In the error handling block (lines 117-122), after logging the error, the function should explicitly else:
# Don't re-raise: prevents responding to initialize request twice
logger.error(
"Received McpError but responder is already completed. "
"Cannot send error response as response was already sent."
)
return None # Add explicit returnWhy This Failure Might Be Transient:
Recommendation:
Detailed AnalysisTest Failure Log: What the test does:
Why it might crash on Windows:
Evidence this isn't a logic error: $ uv run pytest tests/server/middleware/test_initialization_middleware.py -xvs
======= 9 passed in 0.12s =======All new error handling tests pass, including:
Related Files
|
0128b2f to
82d9875
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/server/middleware/test_initialization_middleware.pyis excluded by none and included by none
📒 Files selected for processing (2)
docs/servers/middleware.mdx(2 hunks)src/fastmcp/server/low_level.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/middleware.mdx
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must be version ≥3.10 with full type annotations
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if they're shorter
Never use bareexceptin code - be specific with exception types
Files:
src/fastmcp/server/low_level.py
🧬 Code graph analysis (1)
src/fastmcp/server/low_level.py (2)
src/fastmcp/server/context.py (2)
fastmcp(152-157)error(441-455)src/fastmcp/server/server.py (1)
_apply_middleware(437-446)
🪛 Ruff (0.14.7)
src/fastmcp/server/low_level.py
119-122: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (3)
src/fastmcp/server/low_level.py (1)
10-10: LGTM!The
McpErrorimport is correctly placed and necessary for the new error handling logic.docs/servers/middleware.mdx (2)
816-816: LGTM!The closing code fence fix ensures proper Markdown rendering of the
CustomHeaderMiddlewareexample.
111-135: Example is correct per MCP specification.The
context.message.params.get("clientInfo", {})access pattern is valid. Per the MCP protocol specification,InitializeRequest.paramscontains theclientInfoobject with requirednameandversionfields. The example properly accessesclientInfoand defensively provides a fallback value. The code is syntactically correct and the warning about error timing aftercall_next()accurately reflects the implementation behavior.
82d9875 to
a386391
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/fastmcp/server/low_level.py (1)
118-123: Consider using ERROR level for exception logging.The current code uses
logger.warningwithexc_info=e, which does include exception details (addressing the diagnostics concern from the past review). However, a previous review suggested usinglogger.exception()(ERROR level) instead oflogger.error()for better visibility.When an
McpErroris raised but cannot be communicated to the client, this represents a significant issue worthy of ERROR-level logging—even though it's a defensive edge case. While WARNING is defensible (the client received a successful response, and this prevents double-responding), ERROR level would make this exception more visible during debugging and aligns with the past review suggestion.If ERROR level is appropriate, apply this diff:
- logger.warning( + logger.exception( "Received McpError but responder is already completed. " "Cannot send error response as response was already sent.", - exc_info=e, )Note:
logger.exception()automatically includes exception info when called in an except block, so theexc_infoparameter can be removed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/server/middleware/test_initialization_middleware.pyis excluded by none and included by none
📒 Files selected for processing (2)
docs/servers/middleware.mdx(2 hunks)src/fastmcp/server/low_level.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/servers/middleware.mdx
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style
Files:
src/fastmcp/server/low_level.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (2)
src/fastmcp/server/low_level.py (2)
10-10: LGTM! Import is correctly added.The
McpErrorimport is necessary for the exception handling in the middleware application and follows the existing import structure.
107-116: LGTM! Error handling logic is sound.The try/except block correctly catches
McpErrorfrom middleware and propagates it to the client when the responder hasn't completed yet. The defensive check onresponder._completed(though a private attribute) prevents double-response errors, and the context manager ensures proper cleanup.
We need this until PrefectHQ/fastmcp#2531 is accepted.
We need this until PrefectHQ/fastmcp#2531 is accepted.
We need this until PrefectHQ/fastmcp#2531 is accepted.
We need this until PrefectHQ/fastmcp#2531 is accepted.
jlowin
left a comment
There was a problem hiding this comment.
Actually sorry @tonyxwz, when I tried to resolve the merge conflict I found that tests fail with MCP SDK 1.23+ (which is what the main branch / FastMCP 2.14) requires. Perhaps there were some changes to how its internals work with regrad to initialization? If you wouldn't mind reviewing that, we can get this merged once compatible. Thanks!
|
@jlowin thank for reviewing. I will take a look at the failed test. |
In some situation, the initialize middleware can check the status of the server and decide to raise an error. Example use case: in a FastMCPProxy, an initialization middleware overrides the on_initialize method and connect to the underlying proxied client. When client respond with error, I want to pass this error to the client.
a386391 to
5e62a62
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/fastmcp/server/low_level.py (1)
111-127: Fixexc_infoparameter in logger call.The exception handling logic correctly prevents duplicate responses. However, the logging call uses a non-standard pattern:
Line 126: Use
exc_info=Trueinstead ofexc_info=e. The standard Python logging idiom when inside an exception handler isexc_info=True, which automatically captures the current exception context viasys.exc_info(). While passing the exception instance (exc_info=e) is technically valid and supported since Python 3.2+, it deviates from the standard pattern established across Python code.Apply this fix:
logger.warning( "Received McpError but responder is already completed. " "Cannot send error response as response was already sent.", - exc_info=e, + exc_info=True, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/server/middleware/test_initialization_middleware.pyis excluded by none and included by none
📒 Files selected for processing (2)
docs/servers/middleware.mdx(2 hunks)src/fastmcp/server/low_level.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
docs/**/*.mdx
📄 CodeRabbit inference engine (docs/.cursor/rules/mintlify.mdc)
docs/**/*.mdx: Use clear, direct language appropriate for technical audiences
Write in second person ('you') for instructions and procedures in MDX documentation
Use active voice over passive voice in MDX technical documentation
Employ present tense for current states and future tense for outcomes in MDX documentation
Maintain consistent terminology throughout all MDX documentation
Keep sentences concise while providing necessary context in MDX documentation
Use parallel structure in lists, headings, and procedures in MDX documentation
Lead with the most important information using inverted pyramid structure in MDX documentation
Use progressive disclosure in MDX documentation: present basic concepts before advanced ones
Break complex procedures into numbered steps in MDX documentation
Include prerequisites and context before instructions in MDX documentation
Provide expected outcomes for each major step in MDX documentation
End sections with next steps or related information in MDX documentation
Use descriptive, keyword-rich headings for navigation and SEO in MDX documentation
Focus on user goals and outcomes rather than system features in MDX documentation
Anticipate common questions and address them proactively in MDX documentation
Include troubleshooting for likely failure points in MDX documentation
Provide multiple pathways (beginner vs advanced) but offer an opinionated path to avoid overwhelming users in MDX documentation
Always include complete, runnable code examples that users can copy and execute in MDX documentation
Show proper error handling and edge case management in MDX code examples
Use realistic data instead of placeholder values in MDX code examples
Include expected outputs and results for verification in MDX code examples
Test all code examples thoroughly before publishing in MDX documentation
Specify language and include filename when relevant in MDX code examples
Add explanatory comments for complex logic in MDX code examples
Document all API...
Files:
docs/servers/middleware.mdx
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bareexcept- be specific with exception types
Prioritize readable, understandable code - clarity over cleverness; avoid obfuscated or confusing patterns even if shorter
Follow existing patterns and maintain consistency in code organization and style
Files:
src/fastmcp/server/low_level.py
🧬 Code graph analysis (1)
src/fastmcp/server/low_level.py (2)
src/fastmcp/server/context.py (3)
fastmcp(150-155)error(454-468)warning(438-452)src/fastmcp/server/server.py (1)
_apply_middleware(919-928)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (3)
src/fastmcp/server/low_level.py (1)
10-10: LGTM! Import is correctly placed.The
McpErrorimport is necessary for catching initialization errors raised by middleware.docs/servers/middleware.mdx (2)
816-816: LGTM! Code fence properly terminated.This fixes the malformed code fence for the
CustomHeaderMiddlewareexample, ensuring proper markdown rendering.
111-135: Excellent documentation with clear example and warnings.This InitializationMiddleware example effectively demonstrates:
- Pre-validation rejection by raising
McpErrorbeforecall_next()- Accessing client information from the initialize request
- Proper error code usage (-32000 for application errors)
- Post-initialization logging after successful initialization
The warning box correctly explains the critical timing constraint: errors raised after
call_next()won't reach the client because the response has already been sent. This aligns with the implementation pattern in the codebase.The code example follows MDX documentation best practices: uses second person in warnings, includes complete runnable code, provides warnings for likely failure points, and uses clear technical language. The
context.message.params.get("clientInfo", {})pattern follows standard MCP protocol conventions for accessing initialization request parameters.
- Update tests to catch McpError specifically instead of generic Exception - Remove commented-out code in low_level.py
|
@tonyxwz I've made some changes that I think resolve the errors, including some that fixed error handling more broadly |
change merged to upstream: PrefectHQ/fastmcp#2531
change merged to upstream: PrefectHQ/fastmcp#2531
change merged to upstream: PrefectHQ/fastmcp#2531
change merged to upstream: PrefectHQ/fastmcp#2531
* fix: add kwargs to http client factory * chore: upgrade fastmcp to v2.14.1 * feat: remove the fastmcp patch change merged to upstream: PrefectHQ/fastmcp#2531 * fix: upgrade to the latest on_initialize method signature * test: validate the patch merged in fastmcp works as expected * fix pyright issues
Description
In some situation, the initialize middleware can check the status of the server and decide to raise an error.
Example use case: in a FastMCPProxy, an initialization middleware overrides the on_initialize method and connect to the underlying proxied client. If the proxy client respond with error, I want to pass this error to the client.
Contributors Checklist
Review Checklist