Skip to content

refactoring(cli/mcp): refactoring error msg#576

Open
VascoSch92 wants to merge 1 commit intomainfrom
vasco/refactoring-mcp
Open

refactoring(cli/mcp): refactoring error msg#576
VascoSch92 wants to merge 1 commit intomainfrom
vasco/refactoring-mcp

Conversation

@VascoSch92
Copy link
Contributor

@VascoSch92 VascoSch92 commented Mar 7, 2026

Replaced 6 identical try/except MCPConfigurationError blocks with a single @_mcp_error_handler decorator, and
extracted the repeated "restart session" message into _print_restart_hint().

Why: Duplicated error handling is a maintenance risk — if the error format, exit code, or styling ever needs to change,
you'd have to update 6 places instead of 1. The decorator makes each handler purely about its happy-path logic, which is easier to read and less prone to inconsistencies.


🚀 Try this PR

uvx --python 3.12 git+https://github.com/OpenHands/OpenHands-CLI.git@vasco/refactoring-mcp

@VascoSch92 VascoSch92 requested a review from malhotra5 March 7, 2026 08:56
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands_cli/mcp
   mcp_commands.py103199%160
TOTAL663493085% 

Copy link
Collaborator

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Good taste - This is textbook refactoring done right.

Verdict:Worth merging

Analysis:
Clean DRY refactoring that eliminates 6 identical try/except blocks and 3 identical restart hints. The decorator is simple (no complex nesting), preserves behavior exactly, and makes the code more maintainable. Each handler is now pure happy-path logic, which is easier to read and modify.

Key Insight: This is what refactoring should be - removing duplication with a standard pattern without adding complexity or changing behavior.

No critical issues found. If this module has tests, verify they still pass (though they should, since behavior is preserved).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants