Part of duplicate code analysis: #1765

Summary

Across 5 logger files in internal/logger/, an identical RWMutex nil-check pattern for accessing the global logger instance appears 8+ times. This results in ~40 lines of exact structural duplication that could be consolidated into a single helper function.

Duplication Details

Pattern: Global Logger RWMutex Nil-Check Access

• Severity: Medium-High

• Occurrences: 8+ instances

• Locations:

  • internal/logger/file_logger.go (lines ~117–122): In logWithLevel()
  • internal/logger/markdown_logger.go (lines ~178–183): In logWithMarkdown()
  • internal/logger/jsonl_logger.go (lines ~112–116): In LogRPCMessageJSONLWithTags()
  • internal/logger/server_file_logger.go (lines ~166–171): In logWithLevelAndServer()
  • internal/logger/rpc_logger.go (lines ~96–101): In logRPCMessageToAll()

• Code Sample (repeated nearly identically in all locations):

globalLoggerMu.RLock()
defer globalLoggerMu.RUnlock()

if globalLogger != nil {
    globalLogger.LogSomething(args...)
}

Some variants add additional checks or call multiple methods on the global logger, but the mutex + nil-check pattern is identical.

Impact Analysis

• Maintainability: If the locking strategy changes (e.g., switching from sync.RWMutex to sync/atomic, or adding a context timeout), all 8+ locations must be updated.
• Bug Risk: High for consistency — Any missed update when changing the nil-check or mutex pattern introduces a race condition or nil-pointer dereference in one logger but not others.
• Code Bloat: ~40 lines of exact duplication; simplest pattern to abstract.

Refactoring Recommendations

1. Extract withGlobalLogger() Helper Function

   // withGlobalLogger executes fn with the global logger if it is initialized.
   // It acquires a read lock for the duration of fn.
   func withGlobalLogger(fn func(*FileLogger)) {
       globalLoggerMu.RLock()
       defer globalLoggerMu.RUnlock()
       if globalLogger != nil {
           fn(globalLogger)
       }
   }

   Then all call sites become:

   withGlobalLogger(func(l *FileLogger) {
       l.LogSomething(args...)
   })

   • Estimated effort: Very Low (30 minutes)
   • Benefits: Single locking point; easy to change strategy globally

2. Per-Logger Type Helpers

   • Each logger file defines its own withGlobal*Logger() helper for its specific global type
   • Reduces cross-file dependencies while still eliminating duplication within each file
   • Estimated effort: Low (1 hour for all 5 files)

Implementation Checklist

• Review duplication findings
• Create withGlobal*Logger() helper(s)
• Update all 8+ call sites to use helpers
• Verify thread-safety of refactored code
• Run tests to confirm no regressions

Parent Issue

See parent analysis report: #1765
Related to #1765

Generated by Duplicate Code Detector · ◷

• expires on Mar 18, 2026, 2:56 AM UTC