From 87f3f6a64ada30dc184c2e853fab87d2dcf53185 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Wed, 3 Jun 2026 07:45:07 +0000 Subject: [PATCH] Backend: Recover from panics in background tasks and album restore #5637 --- internal/auth/session/cleanup.go | 2 +- internal/event/panic.go | 18 +++++++++ internal/event/panic_test.go | 59 ++++++++++++++++++++++++++++ internal/photoprism/backup/albums.go | 10 +++++ internal/workers/auto/auto.go | 26 ++++++------ internal/workers/workers.go | 8 ++-- 6 files changed, 107 insertions(+), 16 deletions(-) diff --git a/internal/auth/session/cleanup.go b/internal/auth/session/cleanup.go index 2817d30df..cae99b674 100644 --- a/internal/auth/session/cleanup.go +++ b/internal/auth/session/cleanup.go @@ -34,7 +34,7 @@ func Cleanup(interval time.Duration) { ticker.Stop() return case <-ticker.C: - CleanupAction() + event.Safe(CleanupAction) } } }() diff --git a/internal/event/panic.go b/internal/event/panic.go index ab99d4ec2..894d47124 100644 --- a/internal/event/panic.go +++ b/internal/event/panic.go @@ -14,3 +14,21 @@ func LogPanic(r any) { Log.Errorf("panic: %v", r) Log.Errorf("stack trace:\n%s", debug.Stack()) } + +// Recover recovers from a panic in the calling goroutine, if any, and logs it +// with a stack trace via LogPanic. Use it as a deferred call so a crash in a +// background task is reported instead of silently terminating the process: +// defer event.Recover(). +func Recover() { + if r := recover(); r != nil { + LogPanic(r) + } +} + +// Safe runs fn and recovers from any panic it raises, logging it via LogPanic. +// It lets long-running loops continue after a single iteration panics rather +// than taking down the whole process. +func Safe(fn func()) { + defer Recover() + fn() +} diff --git a/internal/event/panic_test.go b/internal/event/panic_test.go index 17c3ad4ae..740dfd3ec 100644 --- a/internal/event/panic_test.go +++ b/internal/event/panic_test.go @@ -45,3 +45,62 @@ func TestLogPanic(t *testing.T) { assert.NotPanics(t, func() { LogPanic("boom") }) }) } + +func TestRecover(t *testing.T) { + t.Run("Panic", func(t *testing.T) { + var buf bytes.Buffer + logger := logrus.New() + logger.SetOutput(&buf) + logger.SetLevel(logrus.ErrorLevel) + orig := Log + Log = logger + t.Cleanup(func() { Log = orig }) + + func() { + defer Recover() + panic("kaboom") + }() + + out := buf.String() + assert.Contains(t, out, "panic: kaboom") + assert.Contains(t, out, "goroutine") + }) + t.Run("NoPanic", func(t *testing.T) { + var buf bytes.Buffer + logger := logrus.New() + logger.SetOutput(&buf) + orig := Log + Log = logger + t.Cleanup(func() { Log = orig }) + + func() { defer Recover() }() + + assert.Empty(t, buf.String()) + }) +} + +func TestSafe(t *testing.T) { + t.Run("RecoversAndContinues", func(t *testing.T) { + var buf bytes.Buffer + logger := logrus.New() + logger.SetOutput(&buf) + logger.SetLevel(logrus.ErrorLevel) + orig := Log + Log = logger + t.Cleanup(func() { Log = orig }) + + ran := 0 + assert.NotPanics(t, func() { + Safe(func() { panic("boom") }) + Safe(func() { ran++ }) + }) + + assert.Equal(t, 1, ran) + assert.Contains(t, buf.String(), "panic: boom") + }) + t.Run("Success", func(t *testing.T) { + called := false + Safe(func() { called = true }) + assert.True(t, called) + }) +} diff --git a/internal/photoprism/backup/albums.go b/internal/photoprism/backup/albums.go index d0b7304ef..90f83d0fa 100644 --- a/internal/photoprism/backup/albums.go +++ b/internal/photoprism/backup/albums.go @@ -1,12 +1,14 @@ package backup import ( + "fmt" "path/filepath" "regexp" "time" "github.com/photoprism/photoprism/internal/entity" "github.com/photoprism/photoprism/internal/entity/query" + "github.com/photoprism/photoprism/internal/event" "github.com/photoprism/photoprism/internal/photoprism/get" "github.com/photoprism/photoprism/pkg/clean" "github.com/photoprism/photoprism/pkg/fs" @@ -85,6 +87,14 @@ func RestoreAlbums(backupPath string, force bool) (count int, result error) { backupAlbumsMutex.Lock() defer backupAlbumsMutex.Unlock() + // Recover from panics so a corrupt backup file cannot abort server startup. + defer func() { + if r := recover(); r != nil { + event.LogPanic(r) + result = fmt.Errorf("recovered from panic while restoring album backups") + } + }() + c := get.Config() if !c.BackupAlbums() && !force { diff --git a/internal/workers/auto/auto.go b/internal/workers/auto/auto.go index 2a401b17c..91109fd24 100644 --- a/internal/workers/auto/auto.go +++ b/internal/workers/auto/auto.go @@ -51,19 +51,21 @@ func Start(conf *config.Config) { ticker.Stop() return case <-ticker.C: - if mustIndex(conf.AutoIndex()) { - log.Debugf("auto-index: starting") - ResetIndex() - if err := Index(); err != nil { - log.Errorf("auto-index: %s", err) + event.Safe(func() { + if mustIndex(conf.AutoIndex()) { + log.Debugf("auto-index: starting") + ResetIndex() + if err := Index(); err != nil { + log.Errorf("auto-index: %s", err) + } + } else if mustImport(conf.AutoImport()) { + log.Debugf("auto-import: starting") + ResetImport() + if err := Import(); err != nil { + log.Errorf("auto-import: %s", err) + } } - } else if mustImport(conf.AutoImport()) { - log.Debugf("auto-import: starting") - ResetImport() - if err := Import(); err != nil { - log.Errorf("auto-import: %s", err) - } - } + }) } } }() diff --git a/internal/workers/workers.go b/internal/workers/workers.go index 5dedfec68..e4461338f 100644 --- a/internal/workers/workers.go +++ b/internal/workers/workers.go @@ -97,9 +97,11 @@ func Start(conf *config.Config) { mutex.SyncWorker.Cancel() return case <-ticker.C: - RunMeta(conf) - RunShare(conf) - RunSync(conf) + event.Safe(func() { + RunMeta(conf) + RunShare(conf) + RunSync(conf) + }) } } }()