Don't fail when mounted proxy servers are unreachable#2637
Don't fail when mounted proxy servers are unreachable#2637FaranIdo wants to merge 5 commits intoPrefectHQ:mainfrom
Conversation
Test Failure AnalysisSummary: The Windows tests are failing because the new test file Root Cause: The new test file contains 6 tests that intentionally connect to an unreachable server at The existing codebase already has this workaround in @pytest.mark.skipif(
sys.platform == "win32", reason="Windows asyncio networking timeouts."
)
async def test_mount_with_unreachable_proxy_servers(self, caplog):Suggested Solution: Add the same Windows skip marker to all tests in the new import sysThen add this decorator to each test method in the @pytest.mark.skipif(
sys.platform == "win32", reason="Windows asyncio networking timeouts."
)This should be applied to all 6 test methods:
Background: Why Windows FailsWindows has a well-documented issue with asyncio when handling connection timeouts to unreachable servers. The problem is related to how Windows' proactor event loop handles socket operations and timeouts differently than Unix systems. This is why the FastMCP test suite already:
See the CI workflow configuration:
Related issues: Related Files
Updated to reflect latest workflow run failure analysis |
WalkthroughThe change adds RuntimeError handlers across provider resolution and execution paths in the server module. Updated methods include get_tool, _get_resource_or_template_or_none (resource and template checks), get_resource, get_resource_template, get_prompt, _call_tool, _read_resource (resource and template reads), and _get_prompt. Each new except RuntimeError logs a warning when a provider is unavailable and continues searching other mounted providers or components. Existing NotFoundError handling and fallback behavior are preserved. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)src/fastmcp/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧬 Code graph analysis (1)src/fastmcp/server/server.py (4)
⏰ 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). (4)
🔇 Additional comments (9)
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 (3)
src/fastmcp/server/proxy.py (3)
111-122: Usefrom einstead offrom Noneto preserve error context for debugging.Using
from Nonesuppresses the exception chain, which makes it harder to debug why the proxy call failed. The original exception context (connection timeout, refused connection, etc.) would be valuable for troubleshooting.except (McpError, RuntimeError) as e: logger.warning(f"Failed to call tool {key!r} from proxy: {e}") - raise NotFoundError(f"Tool {key!r} not found") from None + raise NotFoundError(f"Tool {key!r} not found") from e
189-215: Consider usingfrom efor exception chaining consistency.Similar to the tool call path, the exception chain is suppressed with
from None. For consistency and debuggability, consider preserving the chain.except (McpError, RuntimeError) as e: logger.warning(f"Failed to read resource {uri!r} from proxy: {e}") - raise NotFoundError(f"Resource {uri!r} not found") from None + raise NotFoundError(f"Resource {uri!r} not found") from e
261-273: Consider usingfrom efor exception chaining.Same recommendation as other paths - preserving the exception chain aids debugging.
except (McpError, RuntimeError) as e: logger.warning(f"Failed to get prompt {name!r} from proxy: {e}") - raise NotFoundError(f"Prompt {name!r} not found") from None + raise NotFoundError(f"Prompt {name!r} not found") from e
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/server/test_mount.pyis excluded by none and included by nonetests/test_proxy_dead_server.pyis excluded by none and included by none
📒 Files selected for processing (2)
src/fastmcp/server/proxy.py(7 hunks)src/fastmcp/server/server.py(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/fastmcp/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/fastmcp/**/*.py: Write Python code with ≥3.10 type annotations required throughout
Never use bare except - be specific with exception types
Files:
src/fastmcp/server/server.pysrc/fastmcp/server/proxy.py
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T03:06:14.522Z
Learning: Applies to src/fastmcp/**/*.py : Never use bare except - be specific with exception types
📚 Learning: 2025-12-17T03:06:14.522Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T03:06:14.522Z
Learning: Applies to src/fastmcp/**/*.py : Never use bare except - be specific with exception types
Applied to files:
src/fastmcp/server/server.py
📚 Learning: 2025-12-17T03:06:14.522Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-17T03:06:14.522Z
Learning: Applies to src/fastmcp/**/__init__.py : Be intentional about re-exports; core types that define a module's purpose should be exported; specialized features can live in submodules; only re-export to fastmcp.* for fundamental types
Applied to files:
src/fastmcp/server/proxy.py
🧬 Code graph analysis (2)
src/fastmcp/server/server.py (1)
src/fastmcp/exceptions.py (1)
NotFoundError(34-35)
src/fastmcp/server/proxy.py (3)
src/fastmcp/resources/resource.py (2)
key(276-283)ResourceContent(35-130)src/fastmcp/exceptions.py (2)
NotFoundError(34-35)ResourceError(14-15)src/fastmcp/resources/resource_manager.py (1)
read_resource(289-330)
🪛 Ruff (0.14.8)
src/fastmcp/server/server.py
1873-1873: Consider moving this statement to an else block
(TRY300)
src/fastmcp/server/proxy.py
122-122: Avoid specifying long messages outside the exception class
(TRY003)
194-196: Avoid specifying long messages outside the exception class
(TRY003)
210-212: Avoid specifying long messages outside the exception class
(TRY003)
215-215: Avoid specifying long messages outside the exception class
(TRY003)
273-273: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (7)
src/fastmcp/server/server.py (4)
970-971: Appropriate exception broadening for proxy resilience.The addition of
McpErrorandRuntimeErrorto the catch clause allows the tool lookup to gracefully returnNonewhen a mounted proxy is unreachable. This aligns with the method's purpose of returningNoneif not found.
1743-1744: Correct exception handling for mounted server resilience.Catching
(NotFoundError, McpError, RuntimeError)enables the tool call to continue searching other mounted servers when one fails. This is essential for the PR objective of not failing when mounted proxies are unreachable.
1866-1875: Resource resolution correctly handles proxy failures.The restructured code now catches
(NotFoundError, McpError, RuntimeError)after attempting to read from mounted servers. The static analysis hint about moving to anelseblock (TRY300) can be safely ignored here since the flow is clear and thereturn resulton line 1873 correctly exits on success.
2043-2044: Prompt resolution correctly handles proxy failures.Consistent with the tool and resource resolution changes, this allows prompt lookup to continue searching other mounted servers when one fails due to connection issues.
src/fastmcp/server/proxy.py (3)
89-91: LGTM! Graceful degradation for tool listing.Catching
(McpError, RuntimeError)and logging a warning allows the proxy to continue serving local/mounted tools when the remote backend is unreachable.
147-148: Consistent warning logging for resource and template listing failures.The pattern of catching
(McpError, RuntimeError)and logging warnings is applied consistently across resource and template listing operations.Also applies to: 167-168
240-241: Consistent warning logging for prompt listing failures.Follows the same pattern as other listing operations.
Catch RuntimeError (connection failures) in provider iteration loops and continue to next provider instead of failing. This allows working servers to handle requests even when a dead proxy with shared/no prefix is mounted first. Fixed locations: - get_tool, get_resource, get_resource_template, get_prompt - _call_tool, _read_resource (concrete and templates), _get_prompt_content - _get_resource_or_template_or_none 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
d637048 to
d9c3a50
Compare
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/server/test_proxy_shared_prefix.pyis excluded by none and included by none
📒 Files selected for processing (1)
src/fastmcp/server/server.py(10 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/fastmcp/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/fastmcp/**/*.py: Write Python code with ≥3.10 type annotations required throughout
Never use bare except - be specific with exception types
Files:
src/fastmcp/server/server.py
🧠 Learnings (1)
📚 Learning: 2025-11-26T21:51:44.174Z
Learnt from: CR
Repo: jlowin/fastmcp PR: 0
File: .cursor/rules/core-mcp-objects.mdc:0-0
Timestamp: 2025-11-26T21:51:44.174Z
Learning: Review and update related Manager classes (ToolManager, ResourceManager, PromptManager) when modifying MCP object definitions
Applied to files:
src/fastmcp/server/server.py
🧬 Code graph analysis (1)
src/fastmcp/server/server.py (4)
src/fastmcp/server/context.py (1)
warning(457-471)src/fastmcp/resources/resource.py (1)
key(276-283)src/fastmcp/resources/template.py (1)
key(221-228)src/fastmcp/utilities/components.py (1)
key(66-73)
🪛 GitHub Actions: Run static analysis
src/fastmcp/server/server.py
[error] 1-1: Process completed with exit code 1 during ruff-format hook.
⏰ 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). (4)
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
🔇 Additional comments (2)
src/fastmcp/server/server.py (2)
1003-1006: LGTM! Consistent error handling across provider operations.These RuntimeError handlers follow a consistent pattern:
- Catch the exception with proper type annotation
- Log a descriptive warning with context
- Continue to the next provider gracefully
This allows the server to remain operational when mounted proxies are unreachable, which addresses the PR objective.
Also applies to: 1047-1052, 1093-1096, 1616-1619, 1732-1735, 1753-1758, 1866-1869
904-906: RuntimeError handlers should be consistent with logging across all providers.The RuntimeError exception is the correct choice here—it's what the MCP client library intentionally raises for server connection/availability failures. However, most handlers log warnings (e.g., lines 904–906, 1003–1006) while some don't (lines 948–950, 960–962). Add logging to the handlers in
_get_resource_or_template_or_nonefor consistency:except RuntimeError: # Connection failures (e.g., dead proxy) - continue to next provider logger.warning(f"Provider unavailable when getting resource/template {uri!r}") continue
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
This fix is still needed after the Provider refactoring in #2635. That PR handles exceptions for listing operations ( The issue shows up when a dead proxy is mounted before a working server with the same prefix (or no prefix): main_app.mount(dead_proxy, "shared") # first, has no tools
main_app.mount(working_server, "shared") # second, has "my_tool"
await client.list_tools() # works - returns ["shared_my_tool"]
await client.call_tool("shared_my_tool") # fails!Even though my_tool only exists in working_server, the call fails because both providers share the same prefix. dead_proxy gets checked first, MountedProvider.get_tool() tries to The fix catches RuntimeError in provider iteration loops and continues to the next provider, same as listing operations already do. |
|
@jlowin, any chance you can please review this? |
|
@FaranIdo we are in the middle of a comprehensive backend refactor that affects how both mounts and proxies work. please be patient. |
|
I believe this is already handled automatically during provider iteration now; the results are gathered with exceptions suppressed so they can be logged without interrupting execution. |
When a FastMCP server has mounted proxy servers (via
FastMCP.as_proxy()), connection failures to any remote server would prevent local tools, resources, and prompts from being listed or called - even those on the main server or working remotes. This happened because the proxy managers only caughtMETHOD_NOT_FOUNDerrors, not connection failures which surface asRuntimeErrorfrom the Client.The fix catches both
McpErrorandRuntimeErrorin the proxy managers and server iteration loops, allowing the server to gracefully skip unavailable remotes while continuing to serve local and reachable components.