Part of duplicate code analysis: #2001

Summary

In internal/logger/rpc_logger.go, the logRPCMessageToAll function constructs two nearly identical RPCMessageInfo structs back-to-back. The structs are identical in every field except the payload truncation limit (MaxPayloadPreviewLengthText vs. MaxPayloadPreviewLengthMarkdown). The if err != nil error-assignment block is also duplicated for each struct.

Duplication Details

Pattern: Near-identical RPCMessageInfo struct construction

• Severity: Low

• Occurrences: 2 (within the same function)

• Location: internal/logger/rpc_logger.go, function logRPCMessageToAll (lines ~62–100)

• Duplicated code (~16 lines):

  // First construction (text log)
  infoText := &RPCMessageInfo{
      Direction:   direction,
      MessageType: messageType,
      ServerID:    serverID,
      Method:      method,
      PayloadSize: len(payload),
      Payload:     truncateAndSanitize(string(payload), MaxPayloadPreviewLengthText),
  }
  if err != nil {
      infoText.Error = err.Error()
  }
  
  // Second construction (markdown log) — identical except for Payload length constant
  infoMarkdown := &RPCMessageInfo{
      Direction:   direction,
      MessageType: messageType,
      ServerID:    serverID,
      Method:      method,
      PayloadSize: len(payload),
      Payload:     truncateAndSanitize(string(payload), MaxPayloadPreviewLengthMarkdown),
  }
  if err != nil {
      infoMarkdown.Error = err.Error()
  }

The only difference between the two blocks is the maxLength argument to truncateAndSanitize.

Impact Analysis

• Maintainability: Adding a new field to RPCMessageInfo (e.g., a session ID) requires updating both construction sites. Missing one creates a silent discrepancy between text and markdown logs.
• Bug Risk: Low — the existing code is correct — but any future structural change to RPCMessageInfo initialization must be applied twice.
• Code Bloat: ~8 lines could be eliminated by extracting a helper.

Refactoring Recommendations

1. Extract a newRPCMessageInfo helper function:

   func newRPCMessageInfo(direction RPCMessageDirection, messageType RPCMessageType,
       serverID, method string, payload []byte, err error, maxPayload int) *RPCMessageInfo {
       info := &RPCMessageInfo{
           Direction:   direction,
           MessageType: messageType,
           ServerID:    serverID,
           Method:      method,
           PayloadSize: len(payload),
           Payload:     truncateAndSanitize(string(payload), maxPayload),
       }
       if err != nil {
           info.Error = err.Error()
       }
       return info
   }

   Then logRPCMessageToAll becomes:

   infoText := newRPCMessageInfo(direction, messageType, serverID, method, payload, err, MaxPayloadPreviewLengthText)
   LogDebug("rpc", "%s", formatRPCMessage(infoText))
   
   infoMarkdown := newRPCMessageInfo(direction, messageType, serverID, method, payload, err, MaxPayloadPreviewLengthMarkdown)
   withGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger, func(logger *MarkdownLogger) {
       logger.Log(LogLevelDebug, "rpc", "%s", formatRPCMessageMarkdown(infoMarkdown))
   })

Implementation Checklist

• Add unexported newRPCMessageInfo helper to rpc_logger.go
• Replace the two inline struct constructions in logRPCMessageToAll with calls to the helper
• Verify the existing LogRPCMessage(info *RPCMessageInfo) function still works as before
• Run make test to verify no regressions

Parent Issue

See parent analysis report: #2001
Related to #2001

Generated by Duplicate Code Detector · ◷

• expires on Mar 23, 2026, 3:07 AM UTC