-
Notifications
You must be signed in to change notification settings - Fork 7
Closed as not planned
Closed as not planned
Copy link
Description
🔍 Duplicate Code Pattern: Command Error Detection Logic
Part of duplicate code analysis: #739
Summary
Similar command error detection patterns in internal/mcp/connection.go. While not exact duplication, the error string checking patterns for command failures could benefit from consolidation.
Duplication Details
Pattern: Command/Protocol Error Detection
-
Severity: Low
-
Occurrences: 2 distinct patterns in close proximity
-
Locations:
internal/mcp/connection.go(lines 236-241) - command not found errorsinternal/mcp/connection.go(lines 244-248) - connection/protocol errors
-
Code Sample:
// Check if it's a command not found error (lines 236-241) if strings.Contains(err.Error(), "executable file not found") || strings.Contains(err.Error(), "no such file or directory") { logger.LogErrorMd("backend", "MCP backend command not found, command=%s", command) log.Printf(" ⚠️ Command '%s' not found in PATH", command) log.Printf(" ⚠️ Verify the command is installed and executable") } // Check if it's a connection/protocol error (lines 244-248) if strings.Contains(err.Error(), "EOF") || strings.Contains(err.Error(), "broken pipe") { logger.LogErrorMd("backend", "MCP backend connection/protocol error, command=%s", command) log.Printf(" ⚠️ Process started but terminated unexpectedly") log.Printf(" ⚠️ Check if the command supports MCP protocol over stdio") }
Impact Analysis
- Maintainability: Moderate - These are diagnostic helpers, changes are infrequent
- Bug Risk: Low - These patterns are specific to error diagnostics
- Code Bloat: ~12 lines total, but localized to error handling in one method
Refactoring Recommendations
-
Extract to Error Diagnostic Helper (Optional - Low Priority)
- Extract to:
internal/mcp/connection.goor newinternal/mcp/errors.go - Estimated effort: 20-30 minutes
- Benefits: Centralized error diagnostics, easier to add new error types
// errorCategory represents different types of MCP connection errors type errorCategory int const ( errorCommandNotFound errorCategory = iota errorProtocol errorConnection errorUnknown ) // categorizeError determines the category of an MCP connection error func categorizeError(err error) errorCategory { if err == nil { return errorUnknown } errStr := err.Error() if strings.Contains(errStr, "executable file not found") || strings.Contains(errStr, "no such file or directory") { return errorCommandNotFound } if strings.Contains(errStr, "EOF") || strings.Contains(errStr, "broken pipe") { return errorProtocol } if strings.Contains(errStr, "connection refused") || strings.Contains(errStr, "no such host") || strings.Contains(errStr, "network is unreachable") { return errorConnection } return errorUnknown } // logErrorDiagnostics logs appropriate diagnostic messages based on error category func logErrorDiagnostics(category errorCategory, command string) { switch category { case errorCommandNotFound: logger.LogErrorMd("backend", "MCP backend command not found, command=%s", command) log.Printf(" ⚠️ Command '%s' not found in PATH", command) log.Printf(" ⚠️ Verify the command is installed and executable") case errorProtocol: logger.LogErrorMd("backend", "MCP backend connection/protocol error, command=%s", command) log.Printf(" ⚠️ Process started but terminated unexpectedly") log.Printf(" ⚠️ Check if the command supports MCP protocol over stdio") // ... additional cases } } // Usage: if err != nil { // ... existing error logging ... category := categorizeError(err) logErrorDiagnostics(category, command) return nil, fmt.Errorf("failed to connect: %w", err) }
- Extract to:
-
Alternative: Leave As-Is (Acceptable)
- These patterns are localized error diagnostics
- Only appear in one method (
NewConnection) - Changes are infrequent (diagnostic messages are stable)
- Refactoring may not provide significant value given low change frequency
Recommendation
Priority: Low - This pattern is acceptable as-is. Consider refactoring only if:
- Adding many new error diagnostic types
- Needing to reuse error categorization elsewhere
- Part of broader error handling refactoring effort
Implementation Checklist
- Review duplication findings and decide if refactoring is worthwhile
- If proceeding: Create error categorization helpers in
internal/mcp/errors.go - If proceeding: Replace inline checks with helper calls
- If proceeding: Run
make test-allto verify no functionality broken - If not proceeding: Document decision to leave as-is (acceptable trade-off)
Parent Issue
See parent analysis report: #739
Related to #739
AI generated by Duplicate Code Detector
- expires on Feb 13, 2026, 3:07 AM UTC
Reactions are currently unavailable