fix(scanner): stop reporting an unusable ffprobe as an empty folder (#468)

* fix(scanner): stop reporting an unusable ffprobe as an empty folder

parseAudiobookFolder and parsePodcastShow signalled "this folder holds no
audio files" by wrapping os.ErrNotExist, and their reconcile callers skipped
on that. exec also wraps fs.ErrNotExist when the configured ffprobe binary
cannot be run, so a wrong playback.ffmpeg_path made every candidate folder
look empty: the scan logged processed=N failed=0, indexed nothing, and gave
the operator no clue why the library stayed empty.

Introduce an errFolderHasNoMedia sentinel that deliberately does not wrap
os.ErrNotExist, and skip on that instead. A folder that disappears between
the scan walk and the parse still maps to the sentinel, so a mid-scan rename
or delete stays a quiet skip rather than a scan failure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(scanner): bound the all-failed scan summary instead of joining every failure

The sentinel change in this PR makes a previously-unreachable path reachable.
Before it, a misconfigured ffprobe made every candidate folder look empty, so
the scan skipped everything and `failed` stayed 0 — the `failedCount ==
processedCount` branch never fired. Now that an unusable ffprobe propagates as
a real failure, that branch is the expected outcome of a first scan with a bad
`playback.ffmpeg_path`, and it joins one wrapped error per failed folder.

On the 240k-folder library the scan code is written for, `errors.Join` over
that slice produces a ~64 MB error string (measured) that is written verbatim
into `scan_runs.error_message` and republished over the Redis events channel
and the admin SSE stream. The `failures` slice itself also grew unbounded for
the whole scan even when the all-failed guard could not fire (any rescan with
`skipped > 0`), holding hundreds of megabytes across a multi-hour scan before
discarding it.

Add a `scanFailures` collector that retains the first 20 failures and counts
the rest, joining them with a trailing "and N more failures (elided)". The
retained sample still names the cause, which is the entire purpose of the
summary. The same 64 MB case now produces 5.4 KB.

The ebook and manga scans have the identical shape and the same exposure via
their own probe failures, so all four call sites share the collector rather
than fixing the two audio paths alone. `failMu` now guards only `cancelErr`
and is renamed `cancelMu` to match.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Quick <31828688+Quick104@users.noreply.github.com>
This commit is contained in:
Suspense
2026-07-26 23:35:36 -04:00
committed by GitHub
co-authored by Claude Opus 5 Quick
parent 172beb99ef
commit baa33768d6
10 changed files with 273 additions and 41 deletions
+12 -3
View File
@@ -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)
+8 -10
View File
@@ -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)
+37
View File
@@ -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 {
+6 -8
View File
@@ -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())
}
}
+73
View File
@@ -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...)
}
+98
View File
@@ -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)
}
}
+6 -8
View File
@@ -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())
}
}
+10 -3
View File
@@ -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
+6 -6
View File
@@ -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)
+17 -3
View File
@@ -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)
}
}