Fix MCP initialization error handling and enable graceful degradation #6
Conversation
Co-authored-by: Vivek V. <34197929+awsdataarchitect@users.noreply.github.com>
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR successfully implements graceful degradation for MCP client initialization failures, which addresses the ImportError issue described in the PR description. The changes allow the module to be imported and function with limited capabilities when MCP clients fail to initialize, rather than crashing the application.
Key Improvements ✅
- Graceful Error Handling: Replaces exception re-raising with graceful degradation
- Conditional Tool Loading: Safely handles tool initialization when MCP clients are unavailable
- Safe Cleanup: Prevents errors during cleanup when clients weren't initialized
- Import Compatibility: Fixes the
mcp_initializedImportError issue
Minor Optimization Opportunities
I've identified two areas where the conditional logic can be simplified by removing redundant checks, since the client variables are guaranteed to be None when mcp_initialized is False.
The overall approach is solid and production-ready. The changes maintain backward compatibility while significantly improving error resilience.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| docs_tools = aws_docs_mcp_client.list_tools_sync() if mcp_initialized and aws_docs_mcp_client else [] | ||
| diagram_tools = aws_diagram_mcp_client.list_tools_sync() if mcp_initialized and aws_diagram_mcp_client else [] |
There was a problem hiding this comment.
The conditional check mcp_initialized and aws_docs_mcp_client is redundant since aws_docs_mcp_client is already set to None when mcp_initialized is False. The check can be simplified to just mcp_initialized.
| docs_tools = aws_docs_mcp_client.list_tools_sync() if mcp_initialized and aws_docs_mcp_client else [] | |
| diagram_tools = aws_diagram_mcp_client.list_tools_sync() if mcp_initialized and aws_diagram_mcp_client else [] | |
| # Get tools from MCP clients (if initialized) | |
| docs_tools = aws_docs_mcp_client.list_tools_sync() if mcp_initialized else [] | |
| diagram_tools = aws_diagram_mcp_client.list_tools_sync() if mcp_initialized else [] |
cloud_engineer_agent.py
Outdated
| if mcp_initialized and aws_docs_mcp_client: | ||
| try: | ||
| aws_docs_mcp_client.stop() | ||
| print("AWS Documentation MCP client stopped") | ||
| except Exception as e: | ||
| print(f"Error stopping AWS Documentation MCP client: {e}") | ||
|
|
||
| try: | ||
| aws_diagram_mcp_client.stop() | ||
| print("AWS Diagram MCP client stopped") | ||
| except Exception as e: | ||
| print(f"Error stopping AWS Diagram MCP client: {e}") | ||
| if mcp_initialized and aws_diagram_mcp_client: | ||
| try: | ||
| aws_diagram_mcp_client.stop() | ||
| print("AWS Diagram MCP client stopped") | ||
| except Exception as e: | ||
| print(f"Error stopping AWS Diagram MCP client: {e}") |
There was a problem hiding this comment.
Similar to the tool loading logic, the conditional check mcp_initialized and aws_docs_mcp_client is redundant. When mcp_initialized is False, aws_docs_mcp_client is guaranteed to be None. Simplify to just check mcp_initialized.
| if mcp_initialized and aws_docs_mcp_client: | |
| try: | |
| aws_docs_mcp_client.stop() | |
| print("AWS Documentation MCP client stopped") | |
| except Exception as e: | |
| print(f"Error stopping AWS Documentation MCP client: {e}") | |
| try: | |
| aws_diagram_mcp_client.stop() | |
| print("AWS Diagram MCP client stopped") | |
| except Exception as e: | |
| print(f"Error stopping AWS Diagram MCP client: {e}") | |
| if mcp_initialized and aws_diagram_mcp_client: | |
| try: | |
| aws_diagram_mcp_client.stop() | |
| print("AWS Diagram MCP client stopped") | |
| except Exception as e: | |
| print(f"Error stopping AWS Diagram MCP client: {e}") | |
| def cleanup(): | |
| if mcp_initialized: | |
| try: | |
| aws_docs_mcp_client.stop() | |
| print("AWS Documentation MCP client stopped") | |
| except Exception as e: | |
| print(f"Error stopping AWS Documentation MCP client: {e}") | |
| if mcp_initialized: | |
| try: | |
| aws_diagram_mcp_client.stop() | |
| print("AWS Diagram MCP client stopped") | |
| except Exception as e: | |
| print(f"Error stopping AWS Diagram MCP client: {e}") |
|
/q make changes as per the suggestions |
|
Taking a look! |
|
I will implement the following changes based on the review suggestions: • Simplify conditional check in tool loading logic at cloud_engineer_agent.py:116-117 - remove redundant • Simplify conditional checks in cleanup function at cloud_engineer_agent.py:158-170 - remove redundant |
|
✅ Changes implemented successfully! I've simplified the redundant conditional checks as suggested in the review comments. The code is now cleaner and more readable while maintaining the same functionality. |
- Remove redundant aws_docs_mcp_client and aws_diagram_mcp_client checks in tool loading logic - Remove redundant client checks in cleanup function since clients are guaranteed to be None when mcp_initialized is False - Improves code readability and removes unnecessary complexity
|
I've started simplifying the redundant conditional checks in the MCP client initialization logic. |
|
I'm performing final verifications and then I'll raise this for review. |
|
I've created the following pull request for you to review. |
|
I've started adding defensive null checks to the MCP client tool listing to prevent potential crashes. |
|
I'm performing final verifications and then I'll raise this for review. |
|
I've created a pull request for you to review: |
|
Here's my updated plan. Let me know if you have any feedback on my plan or steps as I go. STEP 8
STEP 9
|
|
I'm performing final verifications and then I'll raise this for review. |
|
I've created a pull request for you to review:
STEP 8 (R008 - Already delegated)
STEP 9 (R009 - Already delegated)
These requirements are currently in progress. No updates to the requirements are needed at this time. |
Overview
This PR fixes the ImportError for
mcp_initializedvariable and implements comprehensive error handling for MCP client initialization, along with environment validation and health check capabilities.Requirements Implemented
✅ Fix ImportError for
mcp_initializedmcp_initializedfromcloud_engineer_agent.py/app/app.py, line 2 (or similar line)ImportError: cannot import name 'mcp_initialized' from 'cloud_engineer_agent'mcp_initializedis properly defined at module level (line 44) incloud_engineer_agent.py✅ Enhanced MCP Initialization Error Handling
docs_toolsanddiagram_toolsreturn empty lists when MCP is not availablemcp_initializedflag before accessing tools✅ Environment Variable Validation
AWS_REGION- AWS region for Bedrock and other servicesAWS_ACCESS_KEY_ID- AWS access key ID for authenticationAWS_SECRET_ACCESS_KEY- AWS secret access key for authenticationenv_validator.pymodule imported in bothcloud_engineer_agent.pyandapp.py✅ Health Check Endpoint
health_check.py✅ Docker Validation
cloudeng-strands-agent/DockerfileChanges Made
Modified Files
cloud_engineer_agent.py:mcp_initializedflagKey Code Changes
Testing Recommendations
mcp_initializedcan be successfully imported fromcloud_engineer_agentBenefits
Related Issues
Fixes the ImportError that was preventing application startup when importing
mcp_initializedfromcloud_engineer_agent.py.