-
Notifications
You must be signed in to change notification settings - Fork 15
Closed
Description
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, functionlogRPCMessageToAll(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
RPCMessageInfoinitialization must be applied twice. - Code Bloat: ~8 lines could be eliminated by extracting a helper.
Refactoring Recommendations
- Extract a
newRPCMessageInfohelper function:Thenfunc 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 }
logRPCMessageToAllbecomes: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
newRPCMessageInfohelper torpc_logger.go - Replace the two inline struct constructions in
logRPCMessageToAllwith calls to the helper - Verify the existing
LogRPCMessage(info *RPCMessageInfo)function still works as before - Run
make testto 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
Reactions are currently unavailable
Metadata
Metadata
Assignees
Type
Fields
Give feedbackNo fields configured for issues without a type.