diff --git a/internal/server/auth.go b/internal/server/auth.go index 92bd05f9..03e3f083 100644 --- a/internal/server/auth.go +++ b/internal/server/auth.go @@ -1,35 +1,13 @@ package server import ( - "log" "net/http" - "time" "github.com/github/gh-aw-mcpg/internal/logger" ) var logAuth = logger.New("server:auth") -// logRuntimeError logs runtime errors to stdout per spec section 9.2 -func logRuntimeError(errorType, detail string, r *http.Request, serverName *string) { - logAuth.Printf("Logging runtime error: type=%s, detail=%s", errorType, detail) - - timestamp := time.Now().UTC().Format(time.RFC3339) - requestID := r.Header.Get("X-Request-ID") - if requestID == "" { - requestID = "unknown" - } - - server := "gateway" - if serverName != nil { - server = *serverName - } - - // Spec 9.2: Log to stdout with timestamp, server name, request ID, error details - log.Printf("[ERROR] timestamp=%s server=%s request_id=%s error_type=%s detail=%s path=%s method=%s", - timestamp, server, requestID, errorType, detail, r.URL.Path, r.Method) -} - // isMalformedAuthHeader returns true if the header value contains characters // that are not valid in HTTP header values per RFC 7230: null bytes, control // characters below 0x20 (except horizontal tab 0x09), or DEL (0x7F). diff --git a/internal/server/auth_test.go b/internal/server/auth_test.go index acf5cf2f..a24a3350 100644 --- a/internal/server/auth_test.go +++ b/internal/server/auth_test.go @@ -292,68 +292,6 @@ func TestAuthMiddleware_ConcurrentRequests(t *testing.T) { } } -// TestLogRuntimeError tests the logRuntimeError function -func TestLogRuntimeError(t *testing.T) { - tests := []struct { - name string - errorType string - detail string - requestID string - serverName *string - path string - method string - }{ - { - name: "BasicError", - errorType: "authentication_failed", - detail: "missing_auth_header", - requestID: "req-123", - serverName: nil, - path: "/api/test", - method: "GET", - }, - { - name: "ErrorWithServerName", - errorType: "backend_error", - detail: "connection_failed", - requestID: "req-456", - serverName: stringPtr("test-server"), - path: "/mcp/test", - method: "POST", - }, - { - name: "ErrorWithoutRequestID", - errorType: "validation_error", - detail: "invalid_input", - requestID: "", - serverName: nil, - path: "/health", - method: "GET", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Create a test request - req := httptest.NewRequest(tt.method, tt.path, nil) - if tt.requestID != "" { - req.Header.Set("X-Request-ID", tt.requestID) - } - - // Call logRuntimeError (it logs to stdout via log.Printf) - // We can't easily capture the log output in tests, but we can verify it doesn't panic - assert.NotPanics(t, func() { - logRuntimeError(tt.errorType, tt.detail, req, tt.serverName) - }, "logRuntimeError should not panic") - }) - } -} - -// Helper function to create string pointer -func stringPtr(s string) *string { - return &s -} - // TestIsMalformedAuthHeader tests the isMalformedAuthHeader helper. func TestIsMalformedAuthHeader(t *testing.T) { tests := []struct { diff --git a/internal/server/http_helpers.go b/internal/server/http_helpers.go index b77875b2..b919520b 100644 --- a/internal/server/http_helpers.go +++ b/internal/server/http_helpers.go @@ -6,6 +6,7 @@ import ( "io" "log" "net/http" + "time" "github.com/github/gh-aw-mcpg/internal/auth" "github.com/github/gh-aw-mcpg/internal/guard" @@ -17,6 +18,26 @@ import ( var logHelpers = logger.New("server:helpers") +// logRuntimeError logs runtime errors to stdout per spec section 9.2 +func logRuntimeError(errorType, detail string, r *http.Request, serverName *string) { + logHelpers.Printf("Logging runtime error: type=%s, detail=%s", errorType, detail) + + timestamp := time.Now().UTC().Format(time.RFC3339) + requestID := r.Header.Get("X-Request-ID") + if requestID == "" { + requestID = "unknown" + } + + server := "gateway" + if serverName != nil { + server = *serverName + } + + // Spec 9.2: Log to stdout with timestamp, server name, request ID, error details + log.Printf("[ERROR] timestamp=%s server=%s request_id=%s error_type=%s detail=%s path=%s method=%s", + timestamp, server, requestID, errorType, detail, r.URL.Path, r.Method) +} + // writeErrorResponse writes a JSON error response with a consistent shape. // All HTTP error paths in the server package should use this helper to ensure // clients always receive application/json rather than text/plain. diff --git a/internal/server/http_helpers_test.go b/internal/server/http_helpers_test.go index 03711155..ceb561bd 100644 --- a/internal/server/http_helpers_test.go +++ b/internal/server/http_helpers_test.go @@ -672,3 +672,61 @@ func TestWrapWithMiddleware_LogTagVariations(t *testing.T) { }) } } + +// TestLogRuntimeError tests the logRuntimeError function. +func TestLogRuntimeError(t *testing.T) { + tests := []struct { + name string + errorType string + detail string + requestID string + serverName *string + path string + method string + }{ + { + name: "BasicError", + errorType: "authentication_failed", + detail: "missing_auth_header", + requestID: "req-123", + serverName: nil, + path: "/api/test", + method: "GET", + }, + { + name: "ErrorWithServerName", + errorType: "backend_error", + detail: "connection_failed", + requestID: "req-456", + serverName: stringPtr("test-server"), + path: "/mcp/test", + method: "POST", + }, + { + name: "ErrorWithoutRequestID", + errorType: "validation_error", + detail: "invalid_input", + requestID: "", + serverName: nil, + path: "/health", + method: "GET", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := httptest.NewRequest(tt.method, tt.path, nil) + if tt.requestID != "" { + req.Header.Set("X-Request-ID", tt.requestID) + } + assert.NotPanics(t, func() { + logRuntimeError(tt.errorType, tt.detail, req, tt.serverName) + }, "logRuntimeError should not panic") + }) + } +} + +// stringPtr returns a pointer to s. Used as a helper in tests. +func stringPtr(s string) *string { + return &s +}