-
Notifications
You must be signed in to change notification settings - Fork 9
Description
🔍 Duplicate Code Pattern: SDK CallToolResult Error Returns
Part of duplicate code analysis: #739
Summary
Highly repetitive error return pattern in internal/server/unified.go. The pattern return &sdk.CallToolResult{IsError: true}, nil, err appears 16 times throughout the file, creating significant duplication in error handling logic.
Duplication Details
Pattern: CallToolResult Error Construction
-
Severity: High
-
Occurrences: 16 instances (lines 313, 324, 388, 401, 429, 479, 489, 650, 681, 689, 694, 700, 707, 728, 734, 760)
-
Locations: All in
internal/server/unified.go -
Code Sample:
// Pattern repeated 16 times: if err != nil { logger.LogError("client", "Some error occurred: %v", err) return &sdk.CallToolResult{IsError: true}, nil, err } // Sometimes with fmt.Errorf wrapping: if err != nil { return &sdk.CallToolResult{IsError: true}, nil, fmt.Errorf("failed to connect: %w", err) }
Specific Examples
Example 1 - Tool argument parsing (line 313):
if err != nil {
logger.LogError("client", "Failed to unmarshal tool arguments, tool=%s, error=%v", toolNameCopy, err)
return &sdk.CallToolResult{IsError: true}, nil, err
}Example 2 - Session validation (line 324):
if err := us.requireSession(ctx); err != nil {
logger.LogError("client", "MCP tool call failed: session not initialized, session=%s, tool=%s", sessionID, toolNameCopy)
return &sdk.CallToolResult{IsError: true}, nil, err
}Example 3 - Backend connection (line 681):
conn, err := launcher.GetOrLaunchForSession(us.launcher, serverID, sessionID)
if err != nil {
return &sdk.CallToolResult{IsError: true}, nil, fmt.Errorf("failed to connect: %w", err)
}Impact Analysis
- Maintainability: Changing error return format requires updating 16+ locations
- Bug Risk: Inconsistent error handling across different tool operations
- Code Bloat: ~16 lines of repeated boilerplate
- Testing Complexity: Harder to ensure consistent error behavior
Refactoring Recommendations
-
Extract Error Helper Functions
- Create in:
internal/server/unified.goorinternal/server/error_helpers.go - Estimated effort: 30-45 minutes
- Benefits: Consistent error handling, easier to add error context (e.g., error codes, structured logging)
// newToolError creates a CallToolResult error response with consistent formatting func newToolError(err error) (*sdk.CallToolResult, interface{}, error) { return &sdk.CallToolResult{IsError: true}, nil, err } // newToolErrorf creates a CallToolResult error response with formatted error message func newToolErrorf(format string, args ...interface{}) (*sdk.CallToolResult, interface{}, error) { err := fmt.Errorf(format, args...) return &sdk.CallToolResult{IsError: true}, nil, err } // Usage examples: if err != nil { logger.LogError("client", "Failed to unmarshal tool arguments, tool=%s, error=%v", toolNameCopy, err) return newToolError(err) } if err := us.requireSession(ctx); err != nil { logger.LogError("client", "MCP tool call failed: session not initialized, session=%s, tool=%s", sessionID, toolNameCopy) return newToolError(err) } if err != nil { return newToolErrorf("failed to connect: %w", err) }
- Create in:
-
Advanced: Error Result Builder Pattern (Optional, for future enhancement)
- Consider a builder pattern for more complex error responses
- Allows attaching error codes, structured metadata, etc.
- Estimated effort: 1-2 hours
type ToolErrorBuilder struct { err error context map[string]interface{} } func NewToolError(err error) *ToolErrorBuilder { return &ToolErrorBuilder{err: err, context: make(map[string]interface{})} } func (b *ToolErrorBuilder) WithContext(key string, value interface{}) *ToolErrorBuilder { b.context[key] = value return b } func (b *ToolErrorBuilder) Build() (*sdk.CallToolResult, interface{}, error) { // Could add structured error info to CallToolResult.Content return &sdk.CallToolResult{IsError: true}, nil, b.err }
Implementation Checklist
- Review duplication findings and decide on approach (simple helpers vs builder)
- Create error helper functions in
internal/server/unified.goor newerror_helpers.go - Replace all 16 instances with helper calls
- Update tests to verify error handling behavior
- Run
make test-allto verify no functionality broken - Consider adding unit tests for new helper functions
- Run
make agent-finishedbefore completion
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