Part of duplicate code analysis: #3631
Summary
reconnectPlainJSON() and reconnectSDKTransport() in internal/mcp/connection.go contain three identical logger.Log* call sites — one at the start (warn), one on failure (error), one on success (info). This is exact copy-paste duplication across two sibling functions that follow the same reconnect lifecycle.
Duplication Details
Pattern: Identical session reconnect lifecycle log messages
- Severity: High
- Occurrences: 3 unique log lines × 2 functions = 6 duplicated call sites
- Locations:
internal/mcp/connection.go — reconnectPlainJSON() lines 312, 316, 322
internal/mcp/connection.go — reconnectSDKTransport() lines 333, 368, 376
Code Sample — reconnectPlainJSON (lines 307–325):
func (c *Connection) reconnectPlainJSON() error {
c.sessionMu.Lock()
defer c.sessionMu.Unlock()
logConn.Printf("Session expired, reconnecting plain JSON-RPC for serverID=%s", c.serverID)
logger.LogWarn("backend", "MCP session expired for %s, attempting to reconnect...", c.serverID) // ← duplicate
sessionID, err := c.initializeHTTPSession()
if err != nil {
logger.LogError("backend", "Session reconnect failed for %s: %v", c.serverID, err) // ← duplicate
return fmt.Errorf("session reconnect failed: %w", err)
}
...
logger.LogInfo("backend", "Session successfully reconnected for %s", c.serverID) // ← duplicate
return nil
}
Code Sample — reconnectSDKTransport (lines 328–377):
func (c *Connection) reconnectSDKTransport() error {
c.sessionMu.Lock()
defer c.sessionMu.Unlock()
logConn.Printf("Session expired, reconnecting SDK transport for serverID=%s, type=%s", c.serverID, c.httpTransportType)
logger.LogWarn("backend", "MCP session expired for %s, attempting to reconnect...", c.serverID) // ← identical
...
session, err := client.Connect(connectCtx, transport, nil)
if err != nil {
logger.LogError("backend", "Session reconnect failed for %s: %v", c.serverID, err) // ← identical
return fmt.Errorf("session reconnect failed: %w", err)
}
...
logger.LogInfo("backend", "Session successfully reconnected for %s", c.serverID) // ← identical
return nil
}
Impact Analysis
- Maintainability: If the log format for reconnect events ever needs to change (e.g., adding a field), it must be updated in two separate functions. One is easily missed.
- Bug Risk: Divergence between the two copies has already been shown to be feasible — the
logConn.Printf opening line already differs slightly between the two functions, demonstrating that the copies can drift.
- Code Bloat: Minor — 3 extra lines.
Refactoring Recommendations
-
Extract logReconnectStart / logReconnectResult helpers (preferred)
- Add two small private functions to
connection.go:
func (c *Connection) logReconnectStart() {
logger.LogWarn("backend", "MCP session expired for %s, attempting to reconnect...", c.serverID)
}
func (c *Connection) logReconnectResult(err error) {
if err != nil {
logger.LogError("backend", "Session reconnect failed for %s: %v", c.serverID, err)
} else {
logger.LogInfo("backend", "Session successfully reconnected for %s", c.serverID)
}
}
- Replace the three duplicate call sites in each function with
c.logReconnectStart() and c.logReconnectResult(err).
- Estimated effort: 30 minutes
- Benefits: single source of truth for reconnect event log messages; easier to add structured fields later (e.g., reconnect attempt count)
-
Alternatively: introduce a generic withReconnectLogging(serverID string, reconnectFn func() error) error wrapper, but this adds more indirection than is warranted for three log lines.
Implementation Checklist
Parent Issue
See parent analysis report: #3631
Related to #3631
Generated by Duplicate Code Detector · ● 1.3M · ◷
Part of duplicate code analysis: #3631
Summary
reconnectPlainJSON()andreconnectSDKTransport()ininternal/mcp/connection.gocontain three identicallogger.Log*call sites — one at the start (warn), one on failure (error), one on success (info). This is exact copy-paste duplication across two sibling functions that follow the same reconnect lifecycle.Duplication Details
Pattern: Identical session reconnect lifecycle log messages
internal/mcp/connection.go—reconnectPlainJSON()lines 312, 316, 322internal/mcp/connection.go—reconnectSDKTransport()lines 333, 368, 376Code Sample —
reconnectPlainJSON(lines 307–325):Code Sample —
reconnectSDKTransport(lines 328–377):Impact Analysis
logConn.Printfopening line already differs slightly between the two functions, demonstrating that the copies can drift.Refactoring Recommendations
Extract
logReconnectStart/logReconnectResulthelpers (preferred)connection.go:c.logReconnectStart()andc.logReconnectResult(err).Alternatively: introduce a generic
withReconnectLogging(serverID string, reconnectFn func() error) errorwrapper, but this adds more indirection than is warranted for three log lines.Implementation Checklist
logReconnectStart()andlogReconnectResult(err error)helpers toconnection.goreconnectPlainJSONreconnectSDKTransportmake testto verify no regressionsmake lintto verify formattingParent Issue
See parent analysis report: #3631
Related to #3631