[duplicate-code] Duplicate Code Pattern: FileLogger and ServerFileLogger Sync Boilerplate

[github-actions]

Part of duplicate code analysis: #8475

Summary

FileLogger.Log() and ServerFileLogger.Log() share an identical 3-step body: call formatLogLine, call Println, then call file.Sync() with a log.Printf("WARNING: ...") on failure. The sync-with-warning block is copy-pasted verbatim; a future change to the sync failure log format or sync strategy must be applied in both places.

Duplication Details

Pattern: formatLogLine + Println + file.Sync() + WARNING log

• Severity: Medium
• Occurrences: 2 methods
• Locations:
  • internal/logger/file_logger.go (lines 82–109)
  • internal/logger/server_file_logger.go (lines 78–101)

// file_logger.go:82-109 (simplified)
func (fl *FileLogger) Log(level LogLevel, category, format string, args ...interface{}) {
	fl.mu.Lock()
	defer fl.mu.Unlock()
	logLine := formatLogLine(level, category, format, args...)  // ← shared
	fl.logger.Println(logLine)                                  // ← shared
	if fl.logFile != nil {
		if err := fl.logFile.Sync(); err != nil {               // ← shared
			log.Printf("WARNING: Failed to sync log file: %v", err)  // ← shared
		}
	}
}

// server_file_logger.go:78-101 (simplified)
func (sfl *ServerFileLogger) Log(serverID string, level LogLevel, category, format string, args ...interface{}) {
	// ... error handling ...
	logLine := formatLogLine(level, category, format, args...)  // ← shared
	logger.Println(logLine)                                     // ← shared
	sfl.mu.RLock()
	if file, exists := sfl.files[serverID]; exists {
		if err := file.Sync(); err != nil {                     // ← shared
			log.Printf("WARNING: Failed to sync log file for server %s: %v", serverID, err)  // ← shared
		}
	}
	sfl.mu.RUnlock()
}

Impact Analysis

• Maintainability: Any change to sync behavior (e.g., conditional sync, throttled sync, different warning format) must be applied to both methods independently
• Bug Risk: Low for current code; medium if the two implementations drift over time (e.g., one gets a retry added)
• Code Bloat: ~16 lines; small but the file.Sync() + WARNING pattern is non-trivial enough that drift is possible

Refactoring Recommendations

1. Extract a syncFileWithWarning helper to global_helpers.go

   // In global_helpers.go or file_logger.go
   func syncFileWithWarning(file *os.File, context string) {
       if file == nil {
           return
       }
       if err := file.Sync(); err != nil {
           log.Printf("WARNING: Failed to sync log file%s: %v", context, err)
       }
   }

   Both Log() methods call syncFileWithWarning(fl.logFile, "") and syncFileWithWarning(file, " for server "+serverID) respectively.
   Estimated effort: 30–60 minutes.

2. Accept as acceptable structural similarity (lowest effort)

   • The two methods serve different data structures (single file vs per-server map); the shared 3 lines are already small
   • Add a comment cross-referencing both methods to ensure future changes are applied to both
   • Estimated effort: 15 minutes

Implementation Checklist

• Review the two Log() methods
• Decide whether to extract syncFileWithWarning or document-and-accept
• Implement and verify with make test

Parent Issue

See parent analysis report: #8475
Related to #8475

Generated by Duplicate Code Detector · 229.8 AIC · ⊞ 8.5K · ◷

• expires on Jul 9, 2026, 3:44 AM UTC