diff --git a/internal/scanner/audiobook.go b/internal/scanner/audiobook.go index ade06e8a..6c73a2af 100644 --- a/internal/scanner/audiobook.go +++ b/internal/scanner/audiobook.go @@ -2,6 +2,7 @@ package scanner import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -109,11 +110,19 @@ type parsedAudiobookFile struct { // parsedAudiobookFile with a single synthesized chapter (title = // filename stem); metadata comes from the first file's tags // -// Returns an error wrapping os.ErrNotExist when the folder contains zero -// audio files, so the caller can skip it. +// Returns an error wrapping errFolderHasNoMedia when the folder contains zero +// audio files, so the caller can skip it. Every other error (including an +// ffprobe binary that cannot be executed) is a real failure and must be +// reported by the caller. func parseAudiobookFolder(ctx context.Context, ffprobePath string, folderPath string) (*parsedAudiobook, error) { entries, err := os.ReadDir(folderPath) if err != nil { + if errors.Is(err, os.ErrNotExist) { + // The folder was renamed or deleted between the scan walk and + // this read. It holds no media now, so the caller skips it as it + // would an empty folder rather than counting a scan failure. + return nil, fmt.Errorf("audiobook folder %s: %w", folderPath, errFolderHasNoMedia) + } return nil, fmt.Errorf("read audiobook folder %s: %w", folderPath, err) } @@ -127,7 +136,7 @@ func parseAudiobookFolder(ctx context.Context, ffprobePath string, folderPath st } } if len(audioFiles) == 0 { - return nil, fmt.Errorf("audiobook folder %s: %w", folderPath, os.ErrNotExist) + return nil, fmt.Errorf("audiobook folder %s: %w", folderPath, errFolderHasNoMedia) } sort.Strings(audioFiles) diff --git a/internal/scanner/audiobook_scan.go b/internal/scanner/audiobook_scan.go index 165b0c17..28a70e88 100644 --- a/internal/scanner/audiobook_scan.go +++ b/internal/scanner/audiobook_scan.go @@ -299,7 +299,7 @@ func splitAudiobookReconcileRoots(scans []audiobookRootScan) (roots []string, se // // Each immediate subdirectory of one of folder.Paths is treated as a // single audiobook. Subdirectories that contain zero audio files are -// silently skipped (parseAudiobookFolder returns os.ErrNotExist). +// silently skipped (parseAudiobookFolder returns errFolderHasNoMedia). // // This bypasses the per-file movie/TV pipeline because audiobooks are // inherently folder-scoped (one book = one item, possibly multi-file). @@ -352,8 +352,8 @@ func (s *Scanner) ScanAudiobookFolder(ctx context.Context, folder *models.MediaF processed int64 failed int64 skipped int64 - failMu sync.Mutex - failures []error + cancelMu sync.Mutex + failures scanFailures cancelErr error ) start := time.Now() @@ -367,17 +367,15 @@ func (s *Scanner) ScanAudiobookFolder(ctx context.Context, folder *models.MediaF } if err := s.reconcileAudiobookFolder(ctx, folder, path, &skipped); err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - failMu.Lock() + cancelMu.Lock() if cancelErr == nil { cancelErr = err } - failMu.Unlock() + cancelMu.Unlock() return } atomic.AddInt64(&failed, 1) - failMu.Lock() - failures = append(failures, fmt.Errorf("%s: %w", path, err)) - failMu.Unlock() + failures.addf("%s: %w", path, err) slog.WarnContext(ctx, "audiobook scan: folder failed", "component", "scanner", "folder_id", folder.ID, "path", path, @@ -425,7 +423,7 @@ func (s *Scanner) ScanAudiobookFolder(ctx context.Context, folder *models.MediaF failedCount := atomic.LoadInt64(&failed) skippedCount := atomic.LoadInt64(&skipped) if failedCount > 0 && skippedCount == 0 && failedCount == processedCount { - return fmt.Errorf("audiobook scan failed for every attempted folder_id=%d: %w", folder.ID, errors.Join(failures...)) + return fmt.Errorf("audiobook scan failed for every attempted folder_id=%d: %w", folder.ID, failures.join()) } } @@ -511,7 +509,7 @@ func (s *Scanner) reconcileAudiobookFolder(ctx context.Context, folder *models.M } parsed, err := parseAudiobookFolder(ctx, s.ffprobePath, folderPath) if err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, errFolderHasNoMedia) { return nil } return fmt.Errorf("parse audiobook folder %s: %w", folderPath, err) diff --git a/internal/scanner/audiobook_test.go b/internal/scanner/audiobook_test.go index 5cebd60d..7c7b7928 100644 --- a/internal/scanner/audiobook_test.go +++ b/internal/scanner/audiobook_test.go @@ -248,6 +248,43 @@ func TestParseAudiobookFolderSingleM4B(t *testing.T) { } } +func TestParseAudiobookFolderEmptyFolderSignalsNoMedia(t *testing.T) { + _, err := parseAudiobookFolder(context.Background(), "ffprobe", t.TempDir()) + if !errors.Is(err, errFolderHasNoMedia) { + t.Fatalf("empty folder error = %v, want errFolderHasNoMedia", err) + } +} + +// A missing or misconfigured ffprobe binary must not look like an empty +// folder: exec wraps fs.ErrNotExist when the binary does not exist, so a +// reconcile that skipped on os.ErrNotExist silently indexed nothing while +// reporting processed=N failed=0. +func TestParseAudiobookFolderUnusableFFprobeIsNotSkippable(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "part1.m4b"), []byte("not really audio"), 0o600); err != nil { + t.Fatal(err) + } + + _, err := parseAudiobookFolder(context.Background(), "/nonexistent/bin/ffprobe", dir) + if err == nil { + t.Fatal("parseAudiobookFolder with an unusable ffprobe returned no error") + } + if errors.Is(err, errFolderHasNoMedia) { + t.Fatalf("error = %v, must not be reported as an empty folder", err) + } +} + +// A folder that disappears between the scan walk and the parse is a normal +// mid-scan rename/delete race, not a scan failure: it must stay skippable. +func TestParseAudiobookFolderVanishedFolderSignalsNoMedia(t *testing.T) { + gone := filepath.Join(t.TempDir(), "renamed-away") + + _, err := parseAudiobookFolder(context.Background(), "ffprobe", gone) + if !errors.Is(err, errFolderHasNoMedia) { + t.Fatalf("vanished folder error = %v, want errFolderHasNoMedia", err) + } +} + func TestParseAudiobookFolderMultiFile(t *testing.T) { ffprobePath := FFprobePathFromFFmpeg("ffmpeg") if _, err := exec.LookPath(ffprobePath); err != nil { diff --git a/internal/scanner/ebook_scan.go b/internal/scanner/ebook_scan.go index 4cafe6b9..bbe2a42d 100644 --- a/internal/scanner/ebook_scan.go +++ b/internal/scanner/ebook_scan.go @@ -166,8 +166,8 @@ func (s *Scanner) scanEbookPaths(ctx context.Context, folder *models.MediaFolder processed int64 failed int64 skipped int64 - failMu sync.Mutex - failures []error + cancelMu sync.Mutex + failures scanFailures cancelErr error ) start := time.Now() @@ -181,17 +181,15 @@ func (s *Scanner) scanEbookPaths(ctx context.Context, folder *models.MediaFolder } if err := s.reconcileEbookFile(ctx, folder, path, &skipped, groupLocks); err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - failMu.Lock() + cancelMu.Lock() if cancelErr == nil { cancelErr = err } - failMu.Unlock() + cancelMu.Unlock() return } atomic.AddInt64(&failed, 1) - failMu.Lock() - failures = append(failures, fmt.Errorf("%s: %w", path, err)) - failMu.Unlock() + failures.addf("%s: %w", path, err) slog.WarnContext(ctx, "ebook scan: file failed", "component", "scanner", "folder_id", folder.ID, "path", path, @@ -245,7 +243,7 @@ func (s *Scanner) scanEbookPaths(ctx context.Context, folder *models.MediaFolder failedCount := atomic.LoadInt64(&failed) skippedCount := atomic.LoadInt64(&skipped) if failedCount > 0 && skippedCount == 0 && failedCount == processedCount { - return fmt.Errorf("ebook scan failed for every attempted folder_id=%d: %w", folder.ID, errors.Join(failures...)) + return fmt.Errorf("ebook scan failed for every attempted folder_id=%d: %w", folder.ID, failures.join()) } } diff --git a/internal/scanner/errors.go b/internal/scanner/errors.go new file mode 100644 index 00000000..8fe3d490 --- /dev/null +++ b/internal/scanner/errors.go @@ -0,0 +1,73 @@ +package scanner + +import ( + "errors" + "fmt" + "sync" +) + +// errFolderHasNoMedia signals that a candidate folder contained zero media +// files of the kind its parser looks for. Folder-scoped parsers +// (parseAudiobookFolder, parsePodcastShow) return it so their reconcile +// callers can skip the folder quietly. +// +// It deliberately does NOT wrap os.ErrNotExist. The folder parsers shell out +// to ffprobe, and a missing or misconfigured ffprobe binary surfaces as an +// exec error that wraps fs.ErrNotExist ("fork/exec /path/ffprobe: no such +// file or directory"). Skipping on os.ErrNotExist therefore swallowed a +// server misconfiguration as "this folder has no audio", leaving scans that +// reported processed=N failed=0 while indexing nothing at all. +var errFolderHasNoMedia = errors.New("folder contains no media files") + +// maxRetainedScanFailures caps how many per-item errors a scan keeps for its +// "everything failed" summary. A misconfigured ffprobe fails every candidate, +// so on a library with hundreds of thousands of folders an uncapped slice +// accumulates hundreds of megabytes of near-identical strings for the whole +// scan, and joining them produces a single error that is written verbatim into +// scan_runs.error_message and republished over the events channel and the +// admin SSE stream. The first few failures identify the cause; the rest only +// repeat it. +const maxRetainedScanFailures = 20 + +// scanFailures collects per-item scan errors for the all-failed summary while +// retaining at most maxRetainedScanFailures of them. It is safe for concurrent +// use by the scan worker pools. +type scanFailures struct { + mu sync.Mutex + retained []error + total int +} + +// addf records a formatted failure. The error is only formatted while under +// the cap, so the common runaway case — every candidate failing for the same +// reason — does not pay for building strings it will discard. +func (f *scanFailures) addf(format string, args ...any) { + f.mu.Lock() + defer f.mu.Unlock() + f.total++ + if len(f.retained) < maxRetainedScanFailures { + f.retained = append(f.retained, fmt.Errorf(format, args...)) + } +} + +// len reports how many failures were recorded, including elided ones. +func (f *scanFailures) len() int { + f.mu.Lock() + defer f.mu.Unlock() + return f.total +} + +// join returns the retained failures as one error, with a trailing note when +// failures were elided. Returns nil when nothing was recorded. +func (f *scanFailures) join() error { + f.mu.Lock() + defer f.mu.Unlock() + if f.total == 0 { + return nil + } + if elided := f.total - len(f.retained); elided > 0 { + return errors.Join(append(append([]error(nil), f.retained...), + fmt.Errorf("and %d more failures (elided)", elided))...) + } + return errors.Join(f.retained...) +} diff --git a/internal/scanner/errors_test.go b/internal/scanner/errors_test.go new file mode 100644 index 00000000..4af290e8 --- /dev/null +++ b/internal/scanner/errors_test.go @@ -0,0 +1,98 @@ +package scanner + +import ( + "errors" + "fmt" + "strings" + "sync" + "testing" +) + +func TestScanFailuresJoinsEverythingUnderTheCap(t *testing.T) { + var failures scanFailures + sentinel := errors.New("boom") + failures.addf("/books/a: %w", sentinel) + failures.addf("/books/b: %w", sentinel) + + if got := failures.len(); got != 2 { + t.Fatalf("len() = %d, want 2", got) + } + joined := failures.join() + if !errors.Is(joined, sentinel) { + t.Fatalf("joined error = %v, want it to wrap the sentinel", joined) + } + for _, want := range []string{"/books/a", "/books/b"} { + if !strings.Contains(joined.Error(), want) { + t.Fatalf("joined error %q missing %q", joined, want) + } + } + if strings.Contains(joined.Error(), "elided") { + t.Fatalf("joined error %q reported elisions with nothing elided", joined) + } +} + +// A misconfigured ffprobe fails every candidate in the library, so the +// all-failed summary must stay a bounded diagnostic instead of growing with +// the folder count: it is written verbatim into scan_runs.error_message and +// republished to every admin SSE subscriber. +func TestScanFailuresCapsRetainedErrorsAndReportsTheRemainder(t *testing.T) { + var failures scanFailures + const total = maxRetainedScanFailures + 500 + for i := 0; i < total; i++ { + failures.addf("/books/%d: %w", i, errors.New("fork/exec /usr/lib/jellyfin-ffmpeg/ffprobe: no such file or directory")) + } + + if got := failures.len(); got != total { + t.Fatalf("len() = %d, want %d — the count must include elided failures", got, total) + } + if got := len(failures.retained); got != maxRetainedScanFailures { + t.Fatalf("retained %d errors, want the cap of %d", got, maxRetainedScanFailures) + } + + joined := failures.join() + if !strings.Contains(joined.Error(), fmt.Sprintf("and %d more failures (elided)", total-maxRetainedScanFailures)) { + t.Fatalf("joined error does not report the elided remainder: %q", joined) + } + // The retained sample still names the cause, which is the whole point of + // the summary. + if !strings.Contains(joined.Error(), "/books/0") { + t.Fatalf("joined error dropped the first failure: %q", joined) + } + if strings.Contains(joined.Error(), fmt.Sprintf("/books/%d:", total-1)) { + t.Fatalf("joined error retained a failure past the cap: %q", joined) + } +} + +func TestScanFailuresJoinIsNilWhenNothingFailed(t *testing.T) { + var failures scanFailures + if got := failures.len(); got != 0 { + t.Fatalf("len() = %d, want 0", got) + } + if err := failures.join(); err != nil { + t.Fatalf("join() = %v, want nil", err) + } +} + +// The audiobook, ebook, and manga scans record failures from a worker pool. +func TestScanFailuresIsConcurrencySafe(t *testing.T) { + var failures scanFailures + var wg sync.WaitGroup + const workers, perWorker = 8, 200 + for i := 0; i < workers; i++ { + wg.Add(1) + go func(worker int) { + defer wg.Done() + for j := 0; j < perWorker; j++ { + failures.addf("/books/%d-%d: %w", worker, j, errors.New("probe failed")) + } + }(i) + } + wg.Wait() + + if got := failures.len(); got != workers*perWorker { + t.Fatalf("len() = %d, want %d", got, workers*perWorker) + } + if got := len(failures.retained); got != maxRetainedScanFailures { + t.Fatalf("retained %d errors, want the cap of %d", got, maxRetainedScanFailures) + } +} diff --git a/internal/scanner/manga_scan.go b/internal/scanner/manga_scan.go index b29c56d6..62d2f0ed 100644 --- a/internal/scanner/manga_scan.go +++ b/internal/scanner/manga_scan.go @@ -67,8 +67,8 @@ func (s *Scanner) scanMangaPaths(ctx context.Context, folder *models.MediaFolder processed int64 failed int64 skipped int64 - failMu sync.Mutex - failures []error + cancelMu sync.Mutex + failures scanFailures cancelErr error ) start := time.Now() @@ -82,17 +82,15 @@ func (s *Scanner) scanMangaPaths(ctx context.Context, folder *models.MediaFolder } if err := s.reconcileMangaFile(ctx, folder, path, &skipped, groupLocks); err != nil { if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { - failMu.Lock() + cancelMu.Lock() if cancelErr == nil { cancelErr = err } - failMu.Unlock() + cancelMu.Unlock() return } atomic.AddInt64(&failed, 1) - failMu.Lock() - failures = append(failures, fmt.Errorf("%s: %w", path, err)) - failMu.Unlock() + failures.addf("%s: %w", path, err) slog.WarnContext(ctx, "manga scan: file failed", "component", "scanner", "folder_id", folder.ID, "path", path, @@ -146,7 +144,7 @@ func (s *Scanner) scanMangaPaths(ctx context.Context, folder *models.MediaFolder failedCount := atomic.LoadInt64(&failed) skippedCount := atomic.LoadInt64(&skipped) if failedCount > 0 && skippedCount == 0 && failedCount == processedCount { - return fmt.Errorf("manga scan failed for every attempted folder_id=%d: %w", folder.ID, errors.Join(failures...)) + return fmt.Errorf("manga scan failed for every attempted folder_id=%d: %w", folder.ID, failures.join()) } } diff --git a/internal/scanner/podcast.go b/internal/scanner/podcast.go index c7bc22b0..45174a1c 100644 --- a/internal/scanner/podcast.go +++ b/internal/scanner/podcast.go @@ -2,6 +2,7 @@ package scanner import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -29,8 +30,9 @@ type parsedPodcastEpisode struct { // becomes one episode; tags are read from each file individually so // per-episode titles surface correctly. // -// Returns an os.ErrNotExist-wrapped error if the folder contains zero -// audio files. +// Returns an errFolderHasNoMedia-wrapped error if the folder contains zero +// audio files. Every other error (including an ffprobe binary that cannot be +// executed) is a real failure and must be reported by the caller. func parsePodcastShow(ctx context.Context, ffprobePath string, folderPath string) (*parsedPodcastShow, error) { audioFiles, err := listPodcastShowAudioFiles(folderPath) if err != nil { @@ -73,6 +75,11 @@ func parsePodcastShow(ctx context.Context, ffprobePath string, folderPath string func listPodcastShowAudioFiles(folderPath string) ([]string, error) { entries, err := os.ReadDir(folderPath) if err != nil { + if errors.Is(err, os.ErrNotExist) { + // Renamed or deleted between the scan walk and this read; treat + // it as an empty show so the caller skips instead of failing. + return nil, fmt.Errorf("podcast show %s: %w", folderPath, errFolderHasNoMedia) + } return nil, fmt.Errorf("read podcast folder %s: %w", folderPath, err) } var audioFiles []string @@ -85,7 +92,7 @@ func listPodcastShowAudioFiles(folderPath string) ([]string, error) { } } if len(audioFiles) == 0 { - return nil, fmt.Errorf("podcast show %s: %w", folderPath, os.ErrNotExist) + return nil, fmt.Errorf("podcast show %s: %w", folderPath, errFolderHasNoMedia) } sort.Strings(audioFiles) return audioFiles, nil diff --git a/internal/scanner/podcast_scan.go b/internal/scanner/podcast_scan.go index fa88bdee..946fc4d5 100644 --- a/internal/scanner/podcast_scan.go +++ b/internal/scanner/podcast_scan.go @@ -32,7 +32,7 @@ func (s *Scanner) ScanPodcastFolder(ctx context.Context, folder *models.MediaFol var attempted int var succeeded int - var failures []error + var failures scanFailures reconcileRoots := make([]string, 0, len(folder.Paths)) seenPaths := make(map[string]bool) for _, root := range folder.Paths { @@ -43,7 +43,7 @@ func (s *Scanner) ScanPodcastFolder(ctx context.Context, folder *models.MediaFol if err != nil { slog.WarnContext(ctx, "podcast scan: read root failed", "component", "scanner", "root", root, "error", err) attempted++ - failures = append(failures, fmt.Errorf("read root %s: %w", root, err)) + failures.addf("read root %s: %w", root, err) continue } reconcileRoots = append(reconcileRoots, root) @@ -71,7 +71,7 @@ func (s *Scanner) ScanPodcastFolder(ctx context.Context, folder *models.MediaFol "path", subPath, "error", err, ) - failures = append(failures, fmt.Errorf("%s: %w", subPath, err)) + failures.addf("%s: %w", subPath, err) // Continue with siblings — one bad show should not stop the scan. continue } @@ -81,8 +81,8 @@ func (s *Scanner) ScanPodcastFolder(ctx context.Context, folder *models.MediaFol succeeded++ } } - if attempted > 0 && succeeded == 0 && len(failures) > 0 { - return fmt.Errorf("podcast scan failed for every attempted folder_id=%d: %w", folder.ID, errors.Join(failures...)) + if attempted > 0 && succeeded == 0 && failures.len() > 0 { + return fmt.Errorf("podcast scan failed for every attempted folder_id=%d: %w", folder.ID, failures.join()) } if err := s.reconcilePodcastMissingFiles(ctx, folder, reconcileRoots, seenPaths); err != nil { slog.WarnContext(ctx, "podcast scan: missing-file reconcile failed", "component", "scanner", "folder_id", folder.ID, "error", err) @@ -93,7 +93,7 @@ func (s *Scanner) ScanPodcastFolder(ctx context.Context, folder *models.MediaFol func (s *Scanner) reconcilePodcastShow(ctx context.Context, folder *models.MediaFolder, folderPath string) ([]string, error) { parsed, err := parsePodcastShow(ctx, s.ffprobePath, folderPath) if err != nil { - if errors.Is(err, os.ErrNotExist) { + if errors.Is(err, errFolderHasNoMedia) { return nil, nil } return nil, fmt.Errorf("parse podcast show %s: %w", folderPath, err) diff --git a/internal/scanner/podcast_test.go b/internal/scanner/podcast_test.go index 9865d0cc..58fd1010 100644 --- a/internal/scanner/podcast_test.go +++ b/internal/scanner/podcast_test.go @@ -126,10 +126,24 @@ func TestListPodcastShowAudioFilesReturnsSortedAudioPaths(t *testing.T) { } } -func TestListPodcastShowAudioFilesReturnsNotExistForEmptyShow(t *testing.T) { +func TestListPodcastShowAudioFilesReturnsNoMediaForEmptyShow(t *testing.T) { _, err := listPodcastShowAudioFiles(t.TempDir()) - if !errors.Is(err, os.ErrNotExist) { - t.Fatalf("empty podcast show error = %v, want os.ErrNotExist", err) + if !errors.Is(err, errFolderHasNoMedia) { + t.Fatalf("empty podcast show error = %v, want errFolderHasNoMedia", err) + } + // A missing ffprobe binary also wraps fs.ErrNotExist; the empty-folder + // signal must stay distinguishable from it. + if errors.Is(err, os.ErrNotExist) { + t.Fatalf("empty podcast show error = %v, must not wrap os.ErrNotExist", err) + } +} + +func TestListPodcastShowAudioFilesVanishedShowSignalsNoMedia(t *testing.T) { + gone := filepath.Join(t.TempDir(), "renamed-away") + + _, err := listPodcastShowAudioFiles(gone) + if !errors.Is(err, errFolderHasNoMedia) { + t.Fatalf("vanished show error = %v, want errFolderHasNoMedia", err) } }