diff --git a/internal/logger/file_logger.go b/internal/logger/file_logger.go index da2c6079..10ef3e1f 100644 --- a/internal/logger/file_logger.go +++ b/internal/logger/file_logger.go @@ -52,7 +52,7 @@ func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, err // If the log directory doesn't exist and can't be created, falls back to stdout func InitFileLogger(logDir, fileName string) error { logger, err := initLogger(logDir, fileName, os.O_APPEND, setupFileLogger, handleFileLoggerError) - initGlobalFileLogger(logger) + initGlobalLogger(&globalLoggerMu, &globalFileLogger, logger) return err } @@ -153,5 +153,5 @@ var logFuncs = map[LogLevel]func(string, string, ...interface{}){ // CloseGlobalLogger closes the global file logger func CloseGlobalLogger() error { - return closeGlobalFileLogger() + return closeGlobalLogger(&globalLoggerMu, &globalFileLogger) } diff --git a/internal/logger/global_helpers.go b/internal/logger/global_helpers.go index d702f94a..002e0d93 100644 --- a/internal/logger/global_helpers.go +++ b/internal/logger/global_helpers.go @@ -122,56 +122,3 @@ func closeGlobalLogger[T closableLogger](mu *sync.RWMutex, logger *T) error { } return nil } - -// Type-specific helper functions that use the generic implementations above. -// These maintain backward compatibility with existing code. - -// initGlobalFileLogger initializes the global FileLogger using the generic helper. -func initGlobalFileLogger(logger *FileLogger) { - initGlobalLogger(&globalLoggerMu, &globalFileLogger, logger) -} - -// closeGlobalFileLogger closes the global FileLogger using the generic helper. -func closeGlobalFileLogger() error { - return closeGlobalLogger(&globalLoggerMu, &globalFileLogger) -} - -// initGlobalJSONLLogger initializes the global JSONLLogger using the generic helper. -func initGlobalJSONLLogger(logger *JSONLLogger) { - initGlobalLogger(&globalJSONLMu, &globalJSONLLogger, logger) -} - -// closeGlobalJSONLLogger closes the global JSONLLogger using the generic helper. -func closeGlobalJSONLLogger() error { - return closeGlobalLogger(&globalJSONLMu, &globalJSONLLogger) -} - -// initGlobalMarkdownLogger initializes the global MarkdownLogger using the generic helper. -func initGlobalMarkdownLogger(logger *MarkdownLogger) { - initGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger, logger) -} - -// closeGlobalMarkdownLogger closes the global MarkdownLogger using the generic helper. -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) -} - -// initGlobalToolsLogger initializes the global ToolsLogger using the generic helper. -func initGlobalToolsLogger(logger *ToolsLogger) { - initGlobalLogger(&globalToolsMu, &globalToolsLogger, logger) -} - -// closeGlobalToolsLogger closes the global ToolsLogger using the generic helper. -func closeGlobalToolsLogger() error { - return closeGlobalLogger(&globalToolsMu, &globalToolsLogger) -} diff --git a/internal/logger/jsonl_logger.go b/internal/logger/jsonl_logger.go index ff14a148..0a240bf5 100644 --- a/internal/logger/jsonl_logger.go +++ b/internal/logger/jsonl_logger.go @@ -62,7 +62,7 @@ func InitJSONLLogger(logDir, fileName string) error { // JSONLLogger has no fallback mode, so we should not initialize // the global logger when initialization fails if err == nil { - initGlobalJSONLLogger(logger) + initGlobalLogger(&globalJSONLMu, &globalJSONLLogger, logger) } return err } @@ -107,7 +107,7 @@ func (jl *JSONLLogger) logEntry(entry interface{}) error { // CloseJSONLLogger closes the global JSONL logger func CloseJSONLLogger() error { - return closeGlobalJSONLLogger() + return closeGlobalLogger(&globalJSONLMu, &globalJSONLLogger) } // LogRPCMessageJSONL logs an RPC message to the global JSONL logger diff --git a/internal/logger/markdown_logger.go b/internal/logger/markdown_logger.go index 7a9d4ffa..a695d6c4 100644 --- a/internal/logger/markdown_logger.go +++ b/internal/logger/markdown_logger.go @@ -48,7 +48,7 @@ func handleMarkdownLoggerError(_ error, logDir, fileName string) (*MarkdownLogge // InitMarkdownLogger initializes the global markdown logger func InitMarkdownLogger(logDir, fileName string) error { logger, err := initLogger(logDir, fileName, os.O_TRUNC, setupMarkdownLogger, handleMarkdownLoggerError) - initGlobalMarkdownLogger(logger) + initGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger, logger) return err } @@ -215,5 +215,5 @@ func LogDebugMd(category, format string, args ...interface{}) { // CloseMarkdownLogger closes the global markdown logger func CloseMarkdownLogger() error { - return closeGlobalMarkdownLogger() + return closeGlobalLogger(&globalMarkdownMu, &globalMarkdownLogger) } diff --git a/internal/logger/server_file_logger.go b/internal/logger/server_file_logger.go index 581bdf56..990e3f47 100644 --- a/internal/logger/server_file_logger.go +++ b/internal/logger/server_file_logger.go @@ -37,7 +37,7 @@ func InitServerFileLogger(logDir string) error { files: make(map[string]*os.File), useFallback: true, } - initGlobalServerFileLogger(sfl) + initGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, sfl) return nil } @@ -49,7 +49,7 @@ func InitServerFileLogger(logDir string) error { } log.Printf("Initialized per-serverID logging in directory: %s", logDir) - initGlobalServerFileLogger(sfl) + initGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger, sfl) return nil } @@ -177,5 +177,5 @@ func LogDebugWithServer(serverID, category, format string, args ...interface{}) // CloseServerFileLogger closes the global server file logger func CloseServerFileLogger() error { - return closeGlobalServerFileLogger() + return closeGlobalLogger(&globalServerLoggerMu, &globalServerFileLogger) } diff --git a/internal/logger/tools_logger.go b/internal/logger/tools_logger.go index afd5b1b4..074f9ae7 100644 --- a/internal/logger/tools_logger.go +++ b/internal/logger/tools_logger.go @@ -77,7 +77,7 @@ func handleToolsLoggerError(err error, logDir, fileName string) (*ToolsLogger, e // If the log directory doesn't exist and can't be created, falls back to no-op func InitToolsLogger(logDir, fileName string) error { logger, err := initLogger(logDir, fileName, os.O_TRUNC, setupToolsLogger, handleToolsLoggerError) - initGlobalToolsLogger(logger) + initGlobalLogger(&globalToolsMu, &globalToolsLogger, logger) return err } @@ -149,5 +149,5 @@ func LogToolsForServer(serverID string, tools []ToolInfo) { // CloseToolsLogger closes the global tools logger func CloseToolsLogger() error { - return closeGlobalToolsLogger() + return closeGlobalLogger(&globalToolsMu, &globalToolsLogger) } diff --git a/internal/server/auth.go b/internal/server/auth.go index 49a723df..96eae71d 100644 --- a/internal/server/auth.go +++ b/internal/server/auth.go @@ -46,17 +46,13 @@ func authMiddleware(apiKey string, next http.HandlerFunc) http.HandlerFunc { if authHeader == "" { // Spec 7.1: Missing token returns 401 - logger.LogErrorMd("auth", "Authentication failed: missing Authorization header, remote=%s, path=%s", r.RemoteAddr, r.URL.Path) - logRuntimeError("authentication_failed", "missing_auth_header", r, nil) - writeErrorResponse(w, http.StatusUnauthorized, "unauthorized", "missing Authorization header") + rejectRequest(w, r, http.StatusUnauthorized, "unauthorized", "missing Authorization header", "auth", "authentication_failed", "missing_auth_header") return } // Spec 7.1: Authorization header must contain API key directly (not Bearer scheme) if authHeader != apiKey { - logger.LogErrorMd("auth", "Authentication failed: invalid API key, remote=%s, path=%s", r.RemoteAddr, r.URL.Path) - logRuntimeError("authentication_failed", "invalid_api_key", r, nil) - writeErrorResponse(w, http.StatusUnauthorized, "unauthorized", "invalid API key") + rejectRequest(w, r, http.StatusUnauthorized, "unauthorized", "invalid API key", "auth", "authentication_failed", "invalid_api_key") return } diff --git a/internal/server/http_helpers.go b/internal/server/http_helpers.go index 2ff2d637..d79ebb1b 100644 --- a/internal/server/http_helpers.go +++ b/internal/server/http_helpers.go @@ -33,6 +33,21 @@ func writeErrorResponse(w http.ResponseWriter, statusCode int, code, message str }) } +// rejectRequest logs a structured error, records a runtime error, and writes an +// HTTP error response. This consolidates the 3-step rejection pattern that was +// previously duplicated across auth and handler code paths. +// +// Parameters: +// - logCategory: category for the structured log (e.g. "auth") +// - runtimeErrType: error_type field for runtime error log (e.g. "authentication_failed") +// - runtimeDetail: detail field for runtime error log (e.g. "missing_auth_header") +// - msg: human-readable message sent back in the HTTP response +func rejectRequest(w http.ResponseWriter, r *http.Request, status int, code, msg, logCategory, runtimeErrType, runtimeDetail string) { + logger.LogErrorMd(logCategory, "Request rejected: %s, remote=%s, path=%s", msg, r.RemoteAddr, r.URL.Path) + logRuntimeError(runtimeErrType, runtimeDetail, r, nil) + writeErrorResponse(w, status, code, msg) +} + // withResponseLogging wraps an http.Handler to log response bodies func withResponseLogging(handler http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {