[duplicate-code] Duplicate Code Pattern: HTTP Transport Connection Boilerplate

[github-actions]

🔍 Duplicate Code Pattern: HTTP Transport Connection Boilerplate

Part of duplicate code analysis: #351

Summary

HTTP transport connection functions contain significant duplication with similar boilerplate code repeated across tryStreamableHTTPTransport and trySSETransport functions in internal/mcp/connection.go. Both functions follow the exact same pattern with only minor variations in transport type.

Duplication Details

Pattern: Transport Connection Boilerplate

• Severity: Medium
• Occurrences: 2 instances (could expand to 3 with plain JSON transport)
• Locations:
  • internal/mcp/connection.go - tryStreamableHTTPTransport() (lines 310-337)
  • internal/mcp/connection.go - trySSETransport() (lines 339-365)

Code Sample - Streamable HTTP:

func tryStreamableHTTPTransport(ctx context.Context, cancel context.CancelFunc, url string, headers map[string]string, httpClient *http.Client) (*Connection, error) {
    // Create MCP client
    client := newMCPClient()
    
    // Create streamable HTTP transport
    transport := &sdk.StreamableClientTransport{
        Endpoint:   url,
        HTTPClient: httpClient,
        MaxRetries: 0, // Don't retry on failure - we'll try other transports
    }
    
    // Try to connect with a timeout
    connectCtx, connectCancel := context.WithTimeout(ctx, 5*time.Second)
    defer connectCancel()
    
    session, err := client.Connect(connectCtx, transport, nil)
    if err != nil {
        return nil, fmt.Errorf("streamable HTTP transport connect failed: %w", err)
    }
    
    conn := newHTTPConnection(ctx, cancel, client, session, url, headers, httpClient, HTTPTransportStreamable)
    
    logger.LogInfo("backend", "Streamable HTTP transport connected successfully")
    logConn.Printf("Connected with streamable HTTP transport")
    return conn, nil
}

Code Sample - SSE:

func trySSETransport(ctx context.Context, cancel context.CancelFunc, url string, headers map[string]string, httpClient *http.Client) (*Connection, error) {
    // Create MCP client
    client := newMCPClient()
    
    // Create SSE transport
    transport := &sdk.SSEClientTransport{
        Endpoint:   url,
        HTTPClient: httpClient,
    }
    
    // Try to connect with a timeout
    connectCtx, connectCancel := context.WithTimeout(ctx, 5*time.Second)
    defer connectCancel()
    
    session, err := client.Connect(connectCtx, transport, nil)
    if err != nil {
        return nil, fmt.Errorf("SSE transport connect failed: %w", err)
    }
    
    conn := newHTTPConnection(ctx, cancel, client, session, url, headers, httpClient, HTTPTransportSSE)
    
    logger.LogInfo("backend", "SSE transport connected successfully")
    logConn.Printf("Connected with SSE transport")
    return conn, nil
}

Both functions have ~90% identical structure - only the transport type and error messages differ.

Impact Analysis

• Maintainability: Adding a new transport type requires copying the entire pattern
• Bug Risk: Medium - changes to connection logic must be synchronized across both functions
• Code Bloat: ~25 lines × 2 = 50 lines of similar connection boilerplate
• Testing: Requires separate tests for essentially the same logic

Refactoring Recommendations

1. Generic Transport Connection Function (Recommended)

• Extract common logic to parameterized function
• Location: internal/mcp/connection.go
• Estimated effort: 2-3 hours
• Benefits:
  • Single place to update connection logic
  • Easier to add new transport types
  • Reduced code duplication (~40 lines saved)

Example Implementation:

// transportConnector defines the interface for creating SDK transports
type transportConnector interface {
    createTransport(url string, httpClient *http.Client) sdk.Transport
    transportType() HTTPTransportType
    transportName() string
}

// tryTransport is a generic function to attempt connection with any transport type
func tryTransport(ctx context.Context, cancel context.CancelFunc, url string, headers map[string]string, httpClient *http.Client, connector transportConnector) (*Connection, error) {
    // Create MCP client
    client := newMCPClient()
    
    // Create transport using the connector
    transport := connector.createTransport(url, httpClient)
    
    // Try to connect with a timeout
    connectCtx, connectCancel := context.WithTimeout(ctx, 5*time.Second)
    defer connectCancel()
    
    session, err := client.Connect(connectCtx, transport, nil)
    if err != nil {
        return nil, fmt.Errorf("%s transport connect failed: %w", connector.transportName(), err)
    }
    
    conn := newHTTPConnection(ctx, cancel, client, session, url, headers, httpClient, connector.transportType())
    
    logger.LogInfo("backend", "%s transport connected successfully", connector.transportName())
    logConn.Printf("Connected with %s transport", connector.transportName())
    return conn, nil
}

Usage:

// Streamable HTTP connector
type streamableConnector struct{}
func (s streamableConnector) createTransport(url string, httpClient *http.Client) sdk.Transport {
    return &sdk.StreamableClientTransport{
        Endpoint:   url,
        HTTPClient: httpClient,
        MaxRetries: 0,
    }
}
func (s streamableConnector) transportType() HTTPTransportType { return HTTPTransportStreamable }
func (s streamableConnector) transportName() string { return "streamable HTTP" }

// Then in NewHTTPConnection:
conn, err := tryTransport(ctx, cancel, url, headers, httpClient, streamableConnector{})

2. Alternative: Function with Transport Factory

• Use a factory function pattern to create transports
• Pass transport creation as a parameter
• Slightly simpler but less type-safe

Implementation Checklist

• Design generic transport connection interface
• Implement tryTransport generic function
• Create connector implementations for each transport type
• Refactor tryStreamableHTTPTransport to use generic function
• Refactor trySSETransport to use generic function
• Consider refactoring tryPlainJSONTransport for consistency
• Add unit tests for generic connection logic
• Run integration tests to verify no functionality broken

Parent Issue

See parent analysis report: #351
Related to #351

AI generated by Duplicate Code Detector