diff --git a/internal/logging/request_logger.go b/internal/logging/request_logger.go index 8a8b6fbde..e1c7a9cc4 100644 --- a/internal/logging/request_logger.go +++ b/internal/logging/request_logger.go @@ -110,6 +110,9 @@ func (s *FileBodySource) CreatePart(prefix string) (*os.File, error) { return nil, fmt.Errorf("file body source has been cleaned") } prefix = sanitizeTempPrefix(prefix) + if errMkdir := os.MkdirAll(s.dir, 0755); errMkdir != nil { + return nil, errMkdir + } file, errCreate := os.CreateTemp(s.dir, prefix+"-*.tmp") if errCreate != nil { return nil, errCreate @@ -165,16 +168,23 @@ func (s *FileBodySource) WriteTo(w io.Writer) error { return nil } paths := s.Paths() - for i, path := range paths { - if i > 0 { - if _, errWrite := io.WriteString(w, "\n"); errWrite != nil { - return errWrite - } - } + wrote := false + for _, path := range paths { file, errOpen := os.Open(path) if errOpen != nil { + if os.IsNotExist(errOpen) { + continue + } return errOpen } + if wrote { + if _, errWrite := io.WriteString(w, "\n"); errWrite != nil { + if errClose := file.Close(); errClose != nil { + log.WithError(errClose).Warn("failed to close log part file") + } + return errWrite + } + } _, errCopy := io.Copy(w, file) if errClose := file.Close(); errClose != nil { log.WithError(errClose).Warn("failed to close log part file") @@ -185,6 +195,7 @@ func (s *FileBodySource) WriteTo(w io.Writer) error { if errCopy != nil { return errCopy } + wrote = true } return nil } @@ -222,7 +233,7 @@ func (s *FileBodySource) Cleanup() error { } } if dir != "" { - if errRemove := os.Remove(dir); errRemove != nil && !os.IsNotExist(errRemove) && firstErr == nil { + if errRemove := os.RemoveAll(dir); errRemove != nil && firstErr == nil { firstErr = errRemove } } diff --git a/internal/logging/request_logger_home_test.go b/internal/logging/request_logger_home_test.go index 2d974f31d..451eab41a 100644 --- a/internal/logging/request_logger_home_test.go +++ b/internal/logging/request_logger_home_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "net/http" "os" + "path/filepath" "strings" "testing" "time" @@ -23,6 +24,57 @@ func (c *stubHomeRequestLogClient) RPushRequestLog(_ context.Context, payload [] return nil } +func assertFileBodySourceCleaned(t *testing.T, partPaths []string) { + t.Helper() + + dirs := make(map[string]struct{}, len(partPaths)) + for _, path := range partPaths { + if _, errStat := os.Stat(path); !os.IsNotExist(errStat) { + t.Fatalf("expected part %s to be removed, stat err=%v", path, errStat) + } + dirs[filepath.Dir(path)] = struct{}{} + } + for dir := range dirs { + if _, errStat := os.Stat(dir); !os.IsNotExist(errStat) { + t.Fatalf("expected part dir %s to be removed, stat err=%v", dir, errStat) + } + } +} + +func TestFileBodySource_RecreatesPartDirAfterManualCleanup(t *testing.T) { + logsDir := t.TempDir() + source, errSource := NewFileBodySourceInDir(logsDir, "websocket-timeline-test") + if errSource != nil { + t.Fatalf("NewFileBodySourceInDir: %v", errSource) + } + if errAppend := source.AppendPart([]byte("before manual cleanup")); errAppend != nil { + t.Fatalf("AppendPart before cleanup: %v", errAppend) + } + if errRemove := os.RemoveAll(logsDir); errRemove != nil { + t.Fatalf("RemoveAll logs dir: %v", errRemove) + } + if errAppend := source.AppendPart([]byte("after manual cleanup")); errAppend != nil { + t.Fatalf("AppendPart after cleanup: %v", errAppend) + } + + raw, errBytes := source.Bytes() + if errBytes != nil { + t.Fatalf("Bytes after cleanup: %v", errBytes) + } + if bytes.Contains(raw, []byte("before manual cleanup")) { + t.Fatalf("expected manually removed part to be skipped, got %q", string(raw)) + } + if !bytes.Contains(raw, []byte("after manual cleanup")) { + t.Fatalf("expected recreated part content, got %q", string(raw)) + } + + partPaths := source.Paths() + if errCleanup := source.Cleanup(); errCleanup != nil { + t.Fatalf("Cleanup: %v", errCleanup) + } + assertFileBodySourceCleaned(t, partPaths) +} + func TestFileRequestLogger_HomeEnabled_ForwardsWhenRequestLogEnabled(t *testing.T) { original := currentHomeRequestLogClient defer func() { @@ -143,11 +195,7 @@ func TestFileRequestLogger_LogRequestWithSourcesWritesLocalLogAndCleansParts(t * t.Fatalf("LogRequestWithOptionsAndSources error: %v", errLog) } - for _, path := range partPaths { - if _, errStat := os.Stat(path); !os.IsNotExist(errStat) { - t.Fatalf("expected part %s to be removed, stat err=%v", path, errStat) - } - } + assertFileBodySourceCleaned(t, partPaths) entries, errRead := os.ReadDir(logsDir) if errRead != nil { @@ -245,11 +293,7 @@ func TestFileRequestLogger_HomeEnabled_ForwardsSourceLogAndCleansParts(t *testin if !strings.Contains(got.RequestLog, "Event: websocket.request") { t.Fatalf("forwarded request_log missing websocket request: %s", got.RequestLog) } - for _, path := range partPaths { - if _, errStat := os.Stat(path); !os.IsNotExist(errStat) { - t.Fatalf("expected part %s to be removed, stat err=%v", path, errStat) - } - } + assertFileBodySourceCleaned(t, partPaths) } func TestFileRequestLogger_HomeEnabled_ForwardsStreamingRequestID(t *testing.T) {