Skip to content

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

@github-actions

Description

@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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions