diff --git a/internal/logger/common.go b/internal/logger/common.go index 16f0b71..ecd7b8b 100644 --- a/internal/logger/common.go +++ b/internal/logger/common.go @@ -72,6 +72,128 @@ import ( // // When adding a new logger type, follow this pattern to ensure consistent behavior. +// Initialization Pattern for Logger Types +// +// The logger package follows a consistent initialization pattern across all logger types, +// with support for customizable fallback behavior. This section documents the pattern +// and explains the different fallback strategies used by each logger type. +// +// Standard Initialization Pattern: +// +// All logger types use the initLogger() generic helper function for initialization: +// +// func Init*Logger(logDir, fileName string) error { +// logger, err := initLogger( +// logDir, fileName, fileFlags, +// setupFunc, // Configure logger after file is opened +// errorHandler, // Handle initialization failures +// ) +// initGlobal*Logger(logger) +// return err +// } +// +// The initLogger() helper: +// 1. Attempts to create the log directory (if needed) +// 2. Opens the log file with specified flags (os.O_APPEND, os.O_TRUNC, etc.) +// 3. Calls setupFunc to configure the logger instance +// 4. On error, calls errorHandler to implement fallback behavior +// 5. Returns the initialized logger and any error +// +// Fallback Behavior Strategies: +// +// Different logger types implement different fallback strategies based on their purpose: +// +// 1. FileLogger - Stdout Fallback: +// - Purpose: Operational logs must always be visible +// - Fallback: Redirects to stdout if log directory/file creation fails +// - Error: Returns nil (never fails, always provides output) +// - Use case: Critical operational messages that must be seen +// +// Example error handler: +// func(err error, logDir, fileName string) (*FileLogger, error) { +// log.Printf("WARNING: Failed to initialize log file: %v", err) +// log.Printf("WARNING: Falling back to stdout for logging") +// return &FileLogger{ +// logDir: logDir, +// fileName: fileName, +// useFallback: true, +// logger: log.New(os.Stdout, "", 0), +// }, nil +// } +// +// 2. MarkdownLogger - Silent Fallback: +// - Purpose: GitHub workflow preview logs (optional enhancement) +// - Fallback: Sets useFallback flag, produces no output +// - Error: Returns nil (never fails, silently disables) +// - Use case: Nice-to-have logs that shouldn't block operations +// +// Example error handler: +// func(err error, logDir, fileName string) (*MarkdownLogger, error) { +// return &MarkdownLogger{ +// logDir: logDir, +// fileName: fileName, +// useFallback: true, +// }, nil +// } +// +// 3. JSONLLogger - Strict Mode: +// - Purpose: Machine-readable RPC message logs for analysis +// - Fallback: None - returns error immediately +// - Error: Returns error to caller +// - Use case: Structured data that requires file storage +// +// Example error handler: +// func(err error, logDir, fileName string) (*JSONLLogger, error) { +// return nil, err +// } +// +// 4. ServerFileLogger - Unified Fallback: +// - Purpose: Per-server log files for troubleshooting +// - Fallback: Sets useFallback flag, logs to unified logger only +// - Error: Returns nil (never fails, falls back to unified logging) +// - Use case: Per-server logs are helpful but not required +// +// Note: ServerFileLogger doesn't use initLogger() because it creates +// files on-demand, but follows the same fallback philosophy. +// +// Global Logger Management: +// +// After initialization, all logger types register themselves as global loggers +// using the generic initGlobal*Logger() helpers from global_helpers.go: +// +// - initGlobalFileLogger() +// - initGlobalJSONLLogger() +// - initGlobalMarkdownLogger() +// - initGlobalServerFileLogger() +// +// These helpers ensure thread-safe initialization with proper cleanup of any +// existing logger instance. +// +// When to Use Each Logger Type: +// +// - FileLogger: Required operational logs (startup, errors, warnings) +// - MarkdownLogger: Optional GitHub workflow preview logs +// - JSONLLogger: Structured RPC message logs for tooling/analysis +// - ServerFileLogger: Per-server troubleshooting logs +// +// Adding a New Logger Type: +// +// When adding a new logger type: +// 1. Implement Close() method following the Close Pattern (above) +// 2. Add type to closableLogger constraint in global_helpers.go +// 3. Use initLogger() for initialization with appropriate fallback strategy +// 4. Add initGlobal*Logger() and closeGlobal*Logger() helpers +// 5. Document the fallback strategy and use case +// +// This consistent pattern ensures: +// - Predictable behavior across all loggers +// - Easy to understand fallback strategies +// - Minimal code duplication +// - Type-safe global logger management +// +// See file_logger.go, jsonl_logger.go, markdown_logger.go, and server_file_logger.go +// for complete implementation examples. + // closeLogFile is a common helper for closing log files with consistent error handling. // It syncs buffered data before closing and handles errors appropriately. // The mutex should already be held by the caller. @@ -148,7 +270,7 @@ type loggerErrorHandlerFunc[T closableLogger] func(err error, logDir, fileName s // - fileName: Name of the log file // - flags: File opening flags (e.g., os.O_APPEND, os.O_TRUNC) // - setup: Function to configure the logger after the file is opened -// - onError: Function to handle initialization errors (can return fallback or error) +// - onError: Function to handle initialization errors (implements fallback strategy) // // Returns: // - T: The initialized logger instance @@ -157,7 +279,13 @@ type loggerErrorHandlerFunc[T closableLogger] func(err error, logDir, fileName s // This function: // 1. Attempts to open the log file with the specified flags // 2. If successful, calls the setup function to configure the logger -// 3. If unsuccessful, calls the error handler to decide on fallback behavior +// 3. If unsuccessful, calls the error handler to implement the logger's fallback strategy +// +// The onError handler determines the fallback behavior. See "Initialization Pattern for Logger Types" +// documentation above for details on fallback strategies: +// - FileLogger: Falls back to stdout +// - MarkdownLogger: Silent fallback (no output) +// - JSONLLogger: Returns error (no fallback) func initLogger[T closableLogger]( logDir, fileName string, flags int, diff --git a/internal/logger/global_helpers.go b/internal/logger/global_helpers.go index 4595daa..f8b2c0e 100644 --- a/internal/logger/global_helpers.go +++ b/internal/logger/global_helpers.go @@ -17,9 +17,9 @@ package logger import "sync" // closableLogger is a constraint for types that have a Close method. -// This is satisfied by *FileLogger, *JSONLLogger, and *MarkdownLogger. +// This is satisfied by *FileLogger, *JSONLLogger, *MarkdownLogger, and *ServerFileLogger. type closableLogger interface { - *FileLogger | *JSONLLogger | *MarkdownLogger + *FileLogger | *JSONLLogger | *MarkdownLogger | *ServerFileLogger Close() error } @@ -113,3 +113,13 @@ func initGlobalMarkdownLogger(logger *MarkdownLogger) { func closeGlobalMarkdownLogger() error { return closeGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger) } + +// initGlobalServerFileLogger initializes the global ServerFileLogger using the generic helper. +func initGlobalServerFileLogger(logger *ServerFileLogger) { + initGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, logger) +} + +// closeGlobalServerFileLogger closes the global ServerFileLogger using the generic helper. +func closeGlobalServerFileLogger() error { + return closeGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger) +} diff --git a/internal/logger/server_file_logger.go b/internal/logger/server_file_logger.go index eff1626..cc8ebb4 100644 --- a/internal/logger/server_file_logger.go +++ b/internal/logger/server_file_logger.go @@ -212,24 +212,5 @@ func LogDebugWithServer(serverID, category, format string, args ...interface{}) // CloseServerFileLogger closes the global server file logger func CloseServerFileLogger() error { - globalServerLoggerMu.Lock() - defer globalServerLoggerMu.Unlock() - - if globalServerFileLogger != nil { - err := globalServerFileLogger.Close() - globalServerFileLogger = nil - return err - } - return nil -} - -// Helper function for initializing the global server file logger -func initGlobalServerFileLogger(logger *ServerFileLogger) { - globalServerLoggerMu.Lock() - defer globalServerLoggerMu.Unlock() - - if globalServerFileLogger != nil { - globalServerFileLogger.Close() - } - globalServerFileLogger = logger + return closeGlobalServerFileLogger() }