Conversation
Add gather() utility using anyio TaskGroup to run provider queries concurrently. Latency is now O(max provider time) instead of O(sum).
WalkthroughA new asynchronous utility Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fastmcp/server/server.py (1)
840-874: Same silent exception issue in nested gather pattern.The nested gather for resources and templates correctly runs both query types in parallel, but exceptions from individual providers are silently ignored. The
isinstancechecks skip over anyBaseExceptionresults without logging.Consider adding warning logs for provider failures here as well, similar to the list methods.
🧹 Nitpick comments (5)
src/fastmcp/server/providers/base.py (1)
254-289: Consider parallelizingget_tasksfor consistency.The
get_tasksmethod still uses sequentialawaitcalls for eachlist_*operation (lines 270, 274, 278, 285), whileget_componentwas parallelized. For consistency and performance, consider applying the same parallel pattern here.🔎 Proposed parallel implementation
async def get_tasks(self) -> Sequence[FastMCPComponent]: from fastmcp.prompts.prompt import FunctionPrompt from fastmcp.resources.resource import FunctionResource from fastmcp.resources.template import FunctionResourceTemplate from fastmcp.tools.tool import FunctionTool components: list[FastMCPComponent] = [] - for t in await self.list_tools(): + results = await gather( + self.list_tools(), + self.list_resources(), + self.list_resource_templates(), + self.list_prompts(), + ) + tools, resources, templates, prompts = results # type: ignore[assignment] + + for t in tools: if isinstance(t, FunctionTool) and t.task_config.supports_tasks(): components.append(t) - for r in await self.list_resources(): + for r in resources: if isinstance(r, FunctionResource) and r.task_config.supports_tasks(): components.append(r) - for t in await self.list_resource_templates(): + for t in templates: if ( isinstance(t, FunctionResourceTemplate) and t.task_config.supports_tasks() ): components.append(t) - for p in await self.list_prompts(): + for p in prompts: if isinstance(p, FunctionPrompt) and p.task_config.supports_tasks(): components.append(p) return componentssrc/fastmcp/server/server.py (4)
1154-1169: Consider adding provider identification to exception logs.The internal
_list_toolsmethod logs exceptions but doesn't identify which provider failed (line 1162), unlike the publicget_tools()method which tracks provider index and name. This makes debugging harder when multiple providers are in use.🔎 Proposed fix to add provider identification
async def _list_tools( self, context: MiddlewareContext[mcp.types.ListToolsRequest], ) -> list[Tool]: results = await gather( *[p.list_tools() for p in self._providers], return_exceptions=True, ) all_tools: dict[str, Tool] = {} - for result in results: + for i, result in enumerate(results): if isinstance(result, BaseException): - logger.exception("Error listing tools from provider", exc_info=result) + provider = self._providers[i] + provider_name = getattr(provider, "server", provider).__class__.__name__ + logger.exception( + f"Error listing tools from provider {provider_name!r}", + exc_info=result, + ) if fastmcp.settings.mounted_components_raise_on_load_error: raise result continue for tool in result: if self._is_component_enabled(tool) and tool.key not in all_tools: all_tools[tool.key] = tool return list(all_tools.values())
1220-1240: Same missing provider identification issue.
_list_resourcesdoesn't identify which provider failed in the exception log (line 1228-1230).
1292-1312: Same missing provider identification issue.
_list_resource_templatesdoesn't identify which provider failed in the exception log.
1364-1379: Same missing provider identification issue.
_list_promptsdoesn't identify which provider failed in the exception log.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/fastmcp/server/providers/base.pysrc/fastmcp/server/server.pysrc/fastmcp/utilities/async_utils.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/server/providers/base.pysrc/fastmcp/server/server.pysrc/fastmcp/utilities/async_utils.py
🧬 Code graph analysis (2)
src/fastmcp/server/providers/base.py (3)
src/fastmcp/utilities/async_utils.py (3)
gather(12-15)gather(19-22)gather(25-56)src/fastmcp/resources/resource.py (2)
Resource(137-315)key(286-288)src/fastmcp/resources/template.py (2)
ResourceTemplate(97-282)key(251-253)
src/fastmcp/server/server.py (4)
src/fastmcp/utilities/async_utils.py (3)
gather(12-15)gather(19-22)gather(25-56)src/fastmcp/resources/resource.py (3)
register_with_docket(290-294)register_with_docket(400-408)key(286-288)src/fastmcp/utilities/components.py (2)
register_with_docket(156-162)key(94-102)src/fastmcp/exceptions.py (1)
NotFoundError(34-35)
🪛 Ruff (0.14.10)
src/fastmcp/server/server.py
919-919: Avoid specifying long messages outside the exception class
(TRY003)
968-968: 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.10 on windows-latest
- GitHub Check: Run tests: Python 3.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (4)
src/fastmcp/utilities/async_utils.py (1)
1-56: Well-designed utility with proper typed overloads.The implementation correctly preserves result ordering, uses structured concurrency via anyio TaskGroup, and provides proper type narrowing through the overloads. The use of
BaseException(line 46) is appropriate here since we need to capture all exception types includingCancelledErrorandKeyboardInterruptwhenreturn_exceptions=True.src/fastmcp/server/providers/base.py (1)
223-248: Good parallelization of component fetching.The refactor correctly runs all four
list_*operations concurrently. Using the defaultreturn_exceptions=Falsehere is appropriate sinceget_componentis typically called to find a specific component and exceptions should propagate rather than be silently swallowed.The
# type: ignore[assignment]comments are necessary due to TypeVar limitations when unpacking the generic results list.src/fastmcp/server/server.py (2)
477-496: Well-structured parallel task registration with proper error handling.The parallel gather for task registration correctly logs per-provider errors with the provider name and respects the
mounted_components_raise_on_load_errorsetting. Good use of index tracking to correlate results with providers.
796-821: Good parallel implementation with proper error handling.The
get_tools()method demonstrates the correct pattern: parallel gather withreturn_exceptions=True, index tracking to correlate results with providers, per-provider warning logs with provider name, and respecting themounted_components_raise_on_load_errorsetting.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
src/fastmcp/server/server.py (5)
826-835: Missing error logging for provider exceptions.This has the same silent exception handling issue previously flagged - provider exceptions are captured but not logged.
904-913: Missing error logging - same pattern as get_tool.
950-961: Missing error logging - same pattern.
995-1004: Missing error logging - same pattern.
1023-1032: Missing error logging - same pattern.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/fastmcp/server/providers/base.pysrc/fastmcp/server/server.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/server/server.pysrc/fastmcp/server/providers/base.py
🧬 Code graph analysis (2)
src/fastmcp/server/server.py (2)
src/fastmcp/utilities/async_utils.py (3)
gather(12-15)gather(19-22)gather(25-56)src/fastmcp/exceptions.py (1)
NotFoundError(34-35)
src/fastmcp/server/providers/base.py (3)
src/fastmcp/utilities/async_utils.py (3)
gather(12-15)gather(19-22)gather(25-56)src/fastmcp/resources/resource.py (1)
key(286-288)src/fastmcp/resources/template.py (1)
key(251-253)
🪛 Ruff (0.14.10)
src/fastmcp/server/server.py
913-913: Avoid specifying long messages outside the exception class
(TRY003)
961-961: 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). (1)
- GitHub Check: Run tests: Python 3.10 on windows-latest
🔇 Additional comments (10)
src/fastmcp/server/providers/base.py (1)
226-251: Parallel component fetching looks good.The refactored
get_componentcorrectly parallelizes fetching of all component types while preserving the search order semantics. The type ignore comments are necessary due to the heterogeneous return types fromgather. Sincereturn_exceptions=False(default), any provider exception will propagate as expected.src/fastmcp/server/server.py (9)
477-496: Parallel task registration with proper error handling.The parallelized task registration correctly handles exceptions per-provider with logging and respects the
mounted_components_raise_on_load_errorsetting.
802-818: Parallel tool collection with proper error handling.
879-896: Parallel resource collection with proper error handling.
921-942: Parallel template collection with proper error handling.
969-987: Parallel prompt collection with proper error handling.
1144-1159: Parallel tool listing with proper error handling.
1210-1230: Parallel resource listing with proper error handling.
1282-1302: Parallel template listing with proper error handling.
1354-1369: Parallel prompt listing with proper error handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/fastmcp/server/providers/base.py (1)
226-238: Consider usingreturn_exceptions=Truefor consistency with server pattern.The base Provider's
get_componentusesreturn_exceptions=False, causing it to fail fast and cancel all tasks on the first exception. This differs from the server'sget_component(lines 1038-1053 in server.py), which usesreturn_exceptions=Trueto log errors and continue searching other providers.If a provider's
list_tools()fails, this implementation won't even attemptlist_resources(),list_prompts(), etc. For a base class default implementation, the more resilient pattern would allow degraded operation—logging the error and searching the successful results.🔎 Proposed fix for resilient error handling
async def get_component( self, key: str ) -> Tool | Resource | ResourceTemplate | Prompt | None: """Get a component by its prefixed key. Args: key: The prefixed key (e.g., "tool:name", "resource:uri", "template:uri"). Returns: The component if found, or None to continue searching other providers. """ - # Default implementation: fetch all component types in parallel - # Exceptions propagate since return_exceptions=False results = await gather( self.list_tools(), self.list_resources(), self.list_resource_templates(), self.list_prompts(), + return_exceptions=True, ) for components in results: + if isinstance(components, BaseException): + # Log but continue searching other component types + continue for component in components: # type: ignore[union-attr] if component.key == key: return component return None
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/fastmcp/server/providers/base.pysrc/fastmcp/server/server.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python ≥ 3.10 with full type annotations
Never use bareexcept- be specific with exception types
Files:
src/fastmcp/server/server.pysrc/fastmcp/server/providers/base.py
🧬 Code graph analysis (1)
src/fastmcp/server/providers/base.py (4)
src/fastmcp/utilities/async_utils.py (3)
gather(12-15)gather(19-22)gather(25-56)src/fastmcp/resources/resource.py (1)
key(286-288)src/fastmcp/resources/template.py (1)
key(251-253)src/fastmcp/utilities/components.py (1)
key(94-102)
🪛 Ruff (0.14.10)
src/fastmcp/server/server.py
916-916: Avoid specifying long messages outside the exception class
(TRY003)
970-970: 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.10 on ubuntu-latest
- GitHub Check: Run tests: Python 3.13 on ubuntu-latest
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: Run tests with lowest-direct dependencies
🔇 Additional comments (12)
src/fastmcp/server/providers/base.py (2)
64-65: LGTM - Clean__repr__implementation.Simple and effective representation for debugging and logging purposes, as mentioned in the PR objectives.
38-38: LGTM - Import supports parallel operations.src/fastmcp/server/server.py (10)
88-88: LGTM - Import enables parallel provider operations.
477-493: LGTM - Excellent parallel task registration with proper error handling.The implementation correctly:
- Queries all providers concurrently for task-eligible components
- Maps results back to providers using enumerate for accurate error reporting
- Logs warnings for failures while allowing other providers to succeed
- Respects the
mounted_components_raise_on_load_errorsettingThis reduces startup latency from O(sum of provider times) to O(max provider time).
799-815: LGTM - Parallel tool aggregation with proper error handling.The parallel implementation maintains correct semantics:
- First provider wins for duplicate tool names
- Warning-level logging for provider failures (appropriate for list operations)
- Graceful degradation when providers fail
823-838: LGTM - Parallel tool lookup with appropriate logging.The implementation correctly:
- Uses debug-level logging (appropriate for individual lookups)
- Filters out
NotFoundErrorfrom debug logs (line 830), since it's expected when a provider doesn't have the tool- Addresses the error logging concern from past reviews
852-868: LGTM - Clever single-gather optimization for resources and templates.The implementation correctly preserves priority by listing all resource queries before template queries in the gather call. This maintains the "resources have priority over templates" semantics while minimizing latency to O(max provider time) instead of O(max resource time + max template time).
876-893: LGTM - Consistent parallel resource operations.Both methods follow the established pattern with appropriate error handling and logging levels.
Also applies to: 901-916
924-970: LGTM - Consistent parallel operations for templates and prompts.All methods follow the established pattern for parallel provider operations with appropriate error handling. The parallelization provides the same latency benefits across all component types.
Also applies to: 978-1019
1038-1053: LGTM - Parallel component lookup with proper error handling.
1165-1180: LGTM - Internal list methods correctly parallelized.The use of
logger.exceptionin these internal methods provides full stack traces for diagnostics while still allowing graceful degradation. Deduplication by key ensures first-provider-wins semantics are preserved.Also applies to: 1231-1251, 1303-1323, 1375-1390
1579-1591: LGTM - Sequential iteration is correct for execution methods.These methods correctly remain sequential because they need to:
- Execute the first matching component (not just find it)
- Return immediately without querying remaining providers
- Preserve first-provider-wins semantics during execution
The two-pass approach in
_read_resourcealso correctly prioritizes concrete resources over templates within each provider.Also applies to: 1678-1708, 1822-1837
When multiple providers exist, operations like
get_tools()andget_resources()now query all providers concurrently instead of sequentially. Latency is O(max provider time) instead of O(sum).Adds a
gather()utility using anyio TaskGroup with typed overloads:Nested gather enables fully parallel resource + template queries:
Closes #1668