From 87709fd018897d1f66c1dea3a4ea2bc0cc599ff1 Mon Sep 17 00:00:00 2001 From: SmallCoccinelle <89733524+SmallCoccinelle@users.noreply.github.com> Date: Tue, 21 Sep 2021 01:34:25 +0200 Subject: [PATCH] Errcheck phase 1 (#1715) * Avoid redundant logging in migrations Return the error and let the caller handle the logging of the error if needed. While here, defer m.Close() to the function boundary. * Treat errors as values Use %v rather than %s and pass the errors directly. * Generate a wrapped error on stat-failure * Log 3 unchecked errors Rather than ignore errors, log them at the WARNING log level. The server has been functioning without these, so assume they are not at the ERROR level. * Propagate errors upward Failure in path generation was ignored. Propagate the errors upward the call stack, so it can be handled at the level of orchestration. * Warn on errors Log errors rather than quenching them. Errors are logged at the Warn-level for now. * Check error when creating test databases Use the builtin log package and stop the program fatally on error. * Add warnings to uncheck task errors Focus on the task system in a single commit, logging unchecked errors as warnings. * Warn-on-error in API routes Look through the API routes, and make sure errors are being logged if they occur. Prefer the Warn-log-level because none of these has proven to be fatal in the system up until now. * Propagate error when adding Util API * Propagate error on adding util API * Return unhandled error * JS log API: propagate and log errors * JS Plugins: log GQL addition failures. * Warn on failure to write to stdin * Warn on failure to stop task * Wrap viper.BindEnv The current viper code only errors if no name is provided, so it should never fail. Rewrite the code flow to factor through a panic-function. This removes error warnings from this part of the code. * Log errors in concurrency test If we can't initialize the configuration, treat the test as a failure. * Warn on errors in configuration code * Plug an unchecked error in gallery zip walking * Warn on screenshot serving failure * Warn on encoder screenshot failure * Warn on errors in path-handling code * Undo the errcheck on configurations for now. * Use one-line initializers where applicable rather than using err := f() if err!= nil { .. prefer the shorter if err := f(); err != nil { .. If f() isn't too long of a name, or wraps a function with a body. --- pkg/api/resolver_mutation_configure.go | 4 ++- pkg/api/resolver_mutation_plugin.go | 2 +- pkg/api/routes_image.go | 6 +++- pkg/api/routes_movie.go | 19 ++++++++--- pkg/api/routes_performer.go | 10 ++++-- pkg/api/routes_scene.go | 30 ++++++++++++----- pkg/api/routes_studio.go | 10 ++++-- pkg/api/routes_tag.go | 10 ++++-- pkg/database/database.go | 7 ++-- pkg/dlna/service.go | 4 ++- pkg/ffmpeg/ffprobe.go | 5 +-- pkg/ffmpeg/stream.go | 6 +++- pkg/manager/config/config.go | 10 ++++-- pkg/manager/config/config_concurrency_test.go | 6 ++-- pkg/manager/config/init.go | 22 ++++++++----- pkg/manager/image.go | 5 ++- pkg/manager/manager_tasks.go | 24 ++++++++++---- pkg/manager/paths/paths_generated.go | 21 +++++++----- pkg/manager/paths/paths_json.go | 33 ++++++++++++++----- pkg/manager/running_streams.go | 10 ++++-- pkg/manager/screenshot.go | 6 +++- pkg/manager/task_export.go | 26 ++++++++++----- pkg/manager/task_import.go | 4 ++- pkg/manager/task_scan.go | 20 ++++++++--- pkg/manager/task_stash_box_tag.go | 5 ++- pkg/plugin/js.go | 20 ++++++++--- pkg/plugin/js/gql.go | 12 +++++-- pkg/plugin/js/log.go | 32 +++++++++++++----- pkg/plugin/js/util.go | 13 ++++++-- pkg/plugin/plugins.go | 4 ++- pkg/plugin/raw.go | 4 ++- pkg/utils/crypto.go | 6 +++- scripts/test_db_generator/makeTestDB.go | 7 ++-- 33 files changed, 296 insertions(+), 107 deletions(-) diff --git a/pkg/api/resolver_mutation_configure.go b/pkg/api/resolver_mutation_configure.go index 4eb477731..83f4c3bd3 100644 --- a/pkg/api/resolver_mutation_configure.go +++ b/pkg/api/resolver_mutation_configure.go @@ -289,7 +289,9 @@ func (r *mutationResolver) ConfigureDlna(ctx context.Context, input models.Confi if !*input.Enabled && dlnaService.IsRunning() { dlnaService.Stop(nil) } else if *input.Enabled && !dlnaService.IsRunning() { - dlnaService.Start(nil) + if err := dlnaService.Start(nil); err != nil { + logger.Warnf("error starting DLNA service: %v", err) + } } } diff --git a/pkg/api/resolver_mutation_plugin.go b/pkg/api/resolver_mutation_plugin.go index 832f21371..656356ff7 100644 --- a/pkg/api/resolver_mutation_plugin.go +++ b/pkg/api/resolver_mutation_plugin.go @@ -17,7 +17,7 @@ func (r *mutationResolver) RunPluginTask(ctx context.Context, pluginID string, t func (r *mutationResolver) ReloadPlugins(ctx context.Context) (bool, error) { err := manager.GetInstance().PluginCache.LoadPlugins() if err != nil { - logger.Errorf("Error reading plugin configs: %s", err.Error()) + logger.Errorf("Error reading plugin configs: %v", err) } return true, nil diff --git a/pkg/api/routes_image.go b/pkg/api/routes_image.go index 3e64887a8..59b7382b6 100644 --- a/pkg/api/routes_image.go +++ b/pkg/api/routes_image.go @@ -7,6 +7,7 @@ import ( "github.com/go-chi/chi" "github.com/stashapp/stash/pkg/image" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/manager" "github.com/stashapp/stash/pkg/models" "github.com/stashapp/stash/pkg/utils" @@ -59,7 +60,7 @@ func ImageCtx(next http.Handler) http.Handler { imageID, _ := strconv.Atoi(imageIdentifierQueryParam) var image *models.Image - manager.GetInstance().TxnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { + readTxnErr := manager.GetInstance().TxnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { qb := repo.Image() if imageID == 0 { image, _ = qb.FindByChecksum(imageIdentifierQueryParam) @@ -69,6 +70,9 @@ func ImageCtx(next http.Handler) http.Handler { return nil }) + if readTxnErr != nil { + logger.Warnf("read transaction failure while trying to read image by id: %v", readTxnErr) + } if image == nil { http.Error(w, http.StatusText(404), 404) diff --git a/pkg/api/routes_movie.go b/pkg/api/routes_movie.go index 988d4ddf9..4018d39ed 100644 --- a/pkg/api/routes_movie.go +++ b/pkg/api/routes_movie.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/go-chi/chi" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/manager" "github.com/stashapp/stash/pkg/models" "github.com/stashapp/stash/pkg/utils" @@ -32,17 +33,22 @@ func (rs movieRoutes) FrontImage(w http.ResponseWriter, r *http.Request) { defaultParam := r.URL.Query().Get("default") var image []byte if defaultParam != "true" { - rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { + err := rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { image, _ = repo.Movie().GetFrontImage(movie.ID) return nil }) + if err != nil { + logger.Warnf("read transaction error while getting front image: %v", err) + } } if len(image) == 0 { _, image, _ = utils.ProcessBase64Image(models.DefaultMovieImage) } - utils.ServeImage(image, w, r) + if err := utils.ServeImage(image, w, r); err != nil { + logger.Warnf("error serving front image: %v", err) + } } func (rs movieRoutes) BackImage(w http.ResponseWriter, r *http.Request) { @@ -50,17 +56,22 @@ func (rs movieRoutes) BackImage(w http.ResponseWriter, r *http.Request) { defaultParam := r.URL.Query().Get("default") var image []byte if defaultParam != "true" { - rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { + err := rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { image, _ = repo.Movie().GetBackImage(movie.ID) return nil }) + if err != nil { + logger.Warnf("read transaction error on fetch back image: %v", err) + } } if len(image) == 0 { _, image, _ = utils.ProcessBase64Image(models.DefaultMovieImage) } - utils.ServeImage(image, w, r) + if err := utils.ServeImage(image, w, r); err != nil { + logger.Warnf("error while serving image: %v", err) + } } func MovieCtx(next http.Handler) http.Handler { diff --git a/pkg/api/routes_performer.go b/pkg/api/routes_performer.go index 1940e0dfe..101fb2932 100644 --- a/pkg/api/routes_performer.go +++ b/pkg/api/routes_performer.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/go-chi/chi" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/manager" "github.com/stashapp/stash/pkg/manager/config" "github.com/stashapp/stash/pkg/models" @@ -33,17 +34,22 @@ func (rs performerRoutes) Image(w http.ResponseWriter, r *http.Request) { var image []byte if defaultParam != "true" { - rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { + readTxnErr := rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { image, _ = repo.Performer().GetImage(performer.ID) return nil }) + if readTxnErr != nil { + logger.Warnf("couldn't execute getting a performer image from read transaction: %v", readTxnErr) + } } if len(image) == 0 || defaultParam == "true" { image, _ = getRandomPerformerImageUsingName(performer.Name.String, performer.Gender.String, config.GetInstance().GetCustomPerformerImageLocation()) } - utils.ServeImage(image, w, r) + if err := utils.ServeImage(image, w, r); err != nil { + logger.Warnf("error serving image: %v", err) + } } func PerformerCtx(next http.Handler) http.Handler { diff --git a/pkg/api/routes_scene.go b/pkg/api/routes_scene.go index fb038ce29..a67d0fda4 100644 --- a/pkg/api/routes_scene.go +++ b/pkg/api/routes_scene.go @@ -59,7 +59,7 @@ func getSceneFileContainer(scene *models.Scene) ffmpeg.Container { // shouldn't happen, fallback to ffprobe tmpVideoFile, err := ffmpeg.NewVideoFile(manager.GetInstance().FFProbePath, scene.Path, false) if err != nil { - logger.Errorf("[transcode] error reading video file: %s", err.Error()) + logger.Errorf("[transcode] error reading video file: %v", err) return ffmpeg.Container("") } @@ -85,7 +85,9 @@ func (rs sceneRoutes) StreamMKV(w http.ResponseWriter, r *http.Request) { container := getSceneFileContainer(scene) if container != ffmpeg.Matroska { w.WriteHeader(http.StatusBadRequest) - w.Write([]byte("not an mkv file")) + if _, err := w.Write([]byte("not an mkv file")); err != nil { + logger.Warnf("[stream] error writing to stream: %v", err) + } return } @@ -105,7 +107,7 @@ func (rs sceneRoutes) StreamHLS(w http.ResponseWriter, r *http.Request) { videoFile, err := ffmpeg.NewVideoFile(manager.GetInstance().FFProbePath, scene.Path, false) if err != nil { - logger.Errorf("[stream] error reading video file: %s", err.Error()) + logger.Errorf("[stream] error reading video file: %v", err) return } @@ -126,7 +128,9 @@ func (rs sceneRoutes) StreamHLS(w http.ResponseWriter, r *http.Request) { rangeStr := requestByteRange.ToHeaderValue(int64(str.Len())) w.Header().Set("Content-Range", rangeStr) - w.Write(ret) + if n, err := w.Write(ret); err != nil { + logger.Warnf("[stream] error writing stream (wrote %v bytes): %v", n, err) + } } func (rs sceneRoutes) StreamTS(w http.ResponseWriter, r *http.Request) { @@ -141,12 +145,15 @@ func (rs sceneRoutes) streamTranscode(w http.ResponseWriter, r *http.Request, vi videoFile, err := ffmpeg.NewVideoFile(manager.GetInstance().FFProbePath, scene.Path, false) if err != nil { - logger.Errorf("[stream] error reading video file: %s", err.Error()) + logger.Errorf("[stream] error reading video file: %v", err) return } // start stream based on query param, if provided - r.ParseForm() + if err = r.ParseForm(); err != nil { + logger.Warnf("[stream] error parsing query form: %v", err) + } + startTime := r.Form.Get("start") requestedSize := r.Form.Get("resolution") @@ -168,9 +175,11 @@ func (rs sceneRoutes) streamTranscode(w http.ResponseWriter, r *http.Request, vi stream, err = encoder.GetTranscodeStream(options) if err != nil { - logger.Errorf("[stream] error transcoding video file: %s", err.Error()) + logger.Errorf("[stream] error transcoding video file: %v", err) w.WriteHeader(http.StatusBadRequest) - w.Write([]byte(err.Error())) + if _, err := w.Write([]byte(err.Error())); err != nil { + logger.Warnf("[stream] error writing response: %v", err) + } return } @@ -355,7 +364,7 @@ func SceneCtx(next http.Handler) http.Handler { sceneID, _ := strconv.Atoi(sceneIdentifierQueryParam) var scene *models.Scene - manager.GetInstance().TxnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { + readTxnErr := manager.GetInstance().TxnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { qb := repo.Scene() if sceneID == 0 { // determine checksum/os by the length of the query param @@ -370,6 +379,9 @@ func SceneCtx(next http.Handler) http.Handler { return nil }) + if readTxnErr != nil { + logger.Warnf("error executing SceneCtx transaction: %v", readTxnErr) + } if scene == nil { http.Error(w, http.StatusText(404), 404) diff --git a/pkg/api/routes_studio.go b/pkg/api/routes_studio.go index 840e060ad..67cb862dc 100644 --- a/pkg/api/routes_studio.go +++ b/pkg/api/routes_studio.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/go-chi/chi" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/manager" "github.com/stashapp/stash/pkg/models" "github.com/stashapp/stash/pkg/utils" @@ -32,17 +33,22 @@ func (rs studioRoutes) Image(w http.ResponseWriter, r *http.Request) { var image []byte if defaultParam != "true" { - rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { + err := rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { image, _ = repo.Studio().GetImage(studio.ID) return nil }) + if err != nil { + logger.Warnf("read transaction error while fetching studio image: %v", err) + } } if len(image) == 0 { _, image, _ = utils.ProcessBase64Image(models.DefaultStudioImage) } - utils.ServeImage(image, w, r) + if err := utils.ServeImage(image, w, r); err != nil { + logger.Warnf("error serving studio image: %v", err) + } } func StudioCtx(next http.Handler) http.Handler { diff --git a/pkg/api/routes_tag.go b/pkg/api/routes_tag.go index 42d64271c..cebb628cc 100644 --- a/pkg/api/routes_tag.go +++ b/pkg/api/routes_tag.go @@ -6,6 +6,7 @@ import ( "strconv" "github.com/go-chi/chi" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/manager" "github.com/stashapp/stash/pkg/models" "github.com/stashapp/stash/pkg/utils" @@ -32,17 +33,22 @@ func (rs tagRoutes) Image(w http.ResponseWriter, r *http.Request) { var image []byte if defaultParam != "true" { - rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { + err := rs.txnManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { image, _ = repo.Tag().GetImage(tag.ID) return nil }) + if err != nil { + logger.Warnf("read transaction error while getting tag image: %v", err) + } } if len(image) == 0 { image = models.DefaultTagImage } - utils.ServeImage(image, w, r) + if err := utils.ServeImage(image, w, r); err != nil { + logger.Warnf("error serving tag image: %v", err) + } } func TagCtx(next http.Handler) http.Handler { diff --git a/pkg/database/database.go b/pkg/database/database.go index 49fe94536..b1658e00f 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -232,6 +232,7 @@ func RunMigrations() error { if err != nil { panic(err.Error()) } + defer m.Close() databaseSchemaVersion, _, _ = m.Version() stepNumber := appSchemaVersion - databaseSchemaVersion @@ -240,14 +241,10 @@ func RunMigrations() error { err = m.Steps(int(stepNumber)) if err != nil { // migration failed - logger.Errorf("Error migrating database: %s", err.Error()) - m.Close() return err } } - m.Close() - // re-initialise the database Initialize(dbPath) @@ -255,7 +252,7 @@ func RunMigrations() error { logger.Info("Performing vacuum on database") _, err = DB.Exec("VACUUM") if err != nil { - logger.Warnf("error while performing post-migration vacuum: %s", err.Error()) + logger.Warnf("error while performing post-migration vacuum: %v", err) } return nil diff --git a/pkg/dlna/service.go b/pkg/dlna/service.go index f0fb759e4..d0b3ba196 100644 --- a/pkg/dlna/service.go +++ b/pkg/dlna/service.go @@ -251,7 +251,9 @@ func (s *Service) Stop(duration *time.Duration) { if s.startTimer == nil { s.startTimer = time.AfterFunc(*duration, func() { - s.Start(nil) + if err := s.Start(nil); err != nil { + logger.Warnf("error restarting DLNA server: %v", err) + } }) t := time.Now().Add(*duration) s.startTime = &t diff --git a/pkg/ffmpeg/ffprobe.go b/pkg/ffmpeg/ffprobe.go index cb526fcdf..ed276f14a 100644 --- a/pkg/ffmpeg/ffprobe.go +++ b/pkg/ffmpeg/ffprobe.go @@ -273,8 +273,9 @@ func parse(filePath string, probeJSON *FFProbeJSON, stripExt bool) (*VideoFile, result.Duration = math.Round(duration*100) / 100 fileStat, err := os.Stat(filePath) if err != nil { - logger.Errorf("Error statting file <%s>: %s", filePath, err.Error()) - return nil, err + statErr := fmt.Errorf("error statting file <%s>: %w", filePath, err) + logger.Errorf("%v", statErr) + return nil, statErr } result.Size = fileStat.Size() result.StartTime, _ = strconv.ParseFloat(probeJSON.Format.StartTime, 64) diff --git a/pkg/ffmpeg/stream.go b/pkg/ffmpeg/stream.go index 420667680..276f8c847 100644 --- a/pkg/ffmpeg/stream.go +++ b/pkg/ffmpeg/stream.go @@ -224,7 +224,11 @@ func (e *Encoder) stream(probeResult VideoFile, options TranscodeStreamOptions) } registerRunningEncoder(probeResult.Path, cmd.Process) - go waitAndDeregister(probeResult.Path, cmd) + go func() { + if err := waitAndDeregister(probeResult.Path, cmd); err != nil { + logger.Warnf("Error while deregistering ffmpeg stream: %v", err) + } + }() // stderr must be consumed or the process deadlocks go func() { diff --git a/pkg/manager/config/config.go b/pkg/manager/config/config.go index 21d5e5c58..d17daf8b7 100644 --- a/pkg/manager/config/config.go +++ b/pkg/manager/config/config.go @@ -16,6 +16,7 @@ import ( "github.com/spf13/viper" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/manager/paths" "github.com/stashapp/stash/pkg/models" "github.com/stashapp/stash/pkg/utils" @@ -453,7 +454,10 @@ func (i *Instance) GetStashBoxes() []*models.StashBox { i.RLock() defer i.RUnlock() var boxes []*models.StashBox - viper.UnmarshalKey(StashBoxes, &boxes) + if err := viper.UnmarshalKey(StashBoxes, &boxes); err != nil { + logger.Warnf("error in unmarshalkey: %v", err) + } + return boxes } @@ -801,7 +805,9 @@ func (i *Instance) SetCSS(css string) { buf := []byte(css) - ioutil.WriteFile(fn, buf, 0777) + if err := ioutil.WriteFile(fn, buf, 0777); err != nil { + logger.Warnf("error while writing %v bytes to %v: %v", len(buf), fn, err) + } } func (i *Instance) GetCSSEnabled() bool { diff --git a/pkg/manager/config/config_concurrency_test.go b/pkg/manager/config/config_concurrency_test.go index 2f86f332e..fbfadd81c 100644 --- a/pkg/manager/config/config_concurrency_test.go +++ b/pkg/manager/config/config_concurrency_test.go @@ -13,9 +13,9 @@ func TestConcurrentConfigAccess(t *testing.T) { //const loops = 1000 const loops = 200 var wg sync.WaitGroup - for t := 0; t < workers; t++ { + for k := 0; k < workers; k++ { wg.Add(1) - go func() { + go func(wk int) { for l := 0; l < loops; l++ { i.SetInitialConfig() @@ -93,7 +93,7 @@ func TestConcurrentConfigAccess(t *testing.T) { i.Set(MaxUploadSize, i.GetMaxUploadSize()) } wg.Done() - }() + }(k) } wg.Wait() diff --git a/pkg/manager/config/init.go b/pkg/manager/config/init.go index 49f168b66..7117a18af 100644 --- a/pkg/manager/config/init.go +++ b/pkg/manager/config/init.go @@ -112,16 +112,22 @@ func initFlags() flagStruct { } func initEnvs() { - viper.SetEnvPrefix("stash") // will be uppercased automatically - viper.BindEnv("host") // STASH_HOST - viper.BindEnv("port") // STASH_PORT - viper.BindEnv("external_host") // STASH_EXTERNAL_HOST - viper.BindEnv("generated") // STASH_GENERATED - viper.BindEnv("metadata") // STASH_METADATA - viper.BindEnv("cache") // STASH_CACHE + viper.SetEnvPrefix("stash") // will be uppercased automatically + bindEnv("host") // STASH_HOST + bindEnv("port") // STASH_PORT + bindEnv("external_host") // STASH_EXTERNAL_HOST + bindEnv("generated") // STASH_GENERATED + bindEnv("metadata") // STASH_METADATA + bindEnv("cache") // STASH_CACHE // only set stash config flag if not already set if instance.GetStashPaths() == nil { - viper.BindEnv("stash") // STASH_STASH + bindEnv("stash") // STASH_STASH + } +} + +func bindEnv(key string) { + if err := viper.BindEnv(key); err != nil { + panic(fmt.Sprintf("unable to set environment key (%v): %v", key, err)) } } diff --git a/pkg/manager/image.go b/pkg/manager/image.go index 8af987233..b0962f554 100644 --- a/pkg/manager/image.go +++ b/pkg/manager/image.go @@ -61,10 +61,13 @@ func walkGalleryZip(path string, walkFunc func(file *zip.File) error) error { func countImagesInZip(path string) int { ret := 0 - walkGalleryZip(path, func(file *zip.File) error { + err := walkGalleryZip(path, func(file *zip.File) error { ret++ return nil }) + if err != nil { + logger.Warnf("Error while walking gallery zip: %v", err) + } return ret } diff --git a/pkg/manager/manager_tasks.go b/pkg/manager/manager_tasks.go index 101d3aa9f..56c16a352 100644 --- a/pkg/manager/manager_tasks.go +++ b/pkg/manager/manager_tasks.go @@ -163,7 +163,9 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI if err := s.validateFFMPEG(); err != nil { return 0, err } - instance.Paths.Generated.EnsureTmpDir() + if err := instance.Paths.Generated.EnsureTmpDir(); err != nil { + logger.Warnf("could not generate temporary directory: %v", err) + } sceneIDs, err := utils.StringSliceToIntSlice(input.SceneIDs) if err != nil { @@ -250,14 +252,18 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI // Start measuring how long the generate has taken. (consider moving this up) start := time.Now() - instance.Paths.Generated.EnsureTmpDir() + if err = instance.Paths.Generated.EnsureTmpDir(); err != nil { + logger.Warnf("could not create temprary directory: %v", err) + } for _, scene := range scenes { progress.Increment() if job.IsCancelled(ctx) { logger.Info("Stopping due to user request") wg.Wait() - instance.Paths.Generated.EmptyTmpDir() + if err := instance.Paths.Generated.EmptyTmpDir(); err != nil { + logger.Warnf("failure emptying temporary directory: %v", err) + } return } @@ -340,7 +346,9 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI if job.IsCancelled(ctx) { logger.Info("Stopping due to user request") wg.Wait() - instance.Paths.Generated.EmptyTmpDir() + if err := instance.Paths.Generated.EmptyTmpDir(); err != nil { + logger.Warnf("failure emptying temporary directory: %v", err) + } elapsed := time.Since(start) logger.Info(fmt.Sprintf("Generate finished (%s)", elapsed)) return @@ -365,7 +373,9 @@ func (s *singleton) Generate(ctx context.Context, input models.GenerateMetadataI wg.Wait() - instance.Paths.Generated.EmptyTmpDir() + if err = instance.Paths.Generated.EmptyTmpDir(); err != nil { + logger.Warnf("failure emptying temporary directory: %v", err) + } elapsed := time.Since(start) logger.Info(fmt.Sprintf("Generate finished (%s)", elapsed)) }) @@ -383,7 +393,9 @@ func (s *singleton) GenerateScreenshot(ctx context.Context, sceneId string, at f // generate default screenshot if at is nil func (s *singleton) generateScreenshot(ctx context.Context, sceneId string, at *float64) int { - instance.Paths.Generated.EnsureTmpDir() + if err := instance.Paths.Generated.EnsureTmpDir(); err != nil { + logger.Warnf("failure generating screenshot: %v", err) + } j := job.MakeJobExec(func(ctx context.Context, progress *job.Progress) { sceneIdInt, err := strconv.Atoi(sceneId) diff --git a/pkg/manager/paths/paths_generated.go b/pkg/manager/paths/paths_generated.go index 234f3918b..32e79a1ff 100644 --- a/pkg/manager/paths/paths_generated.go +++ b/pkg/manager/paths/paths_generated.go @@ -5,6 +5,7 @@ import ( "io/ioutil" "path/filepath" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/utils" ) @@ -37,26 +38,30 @@ func (gp *generatedPaths) GetTmpPath(fileName string) string { return filepath.Join(gp.Tmp, fileName) } -func (gp *generatedPaths) EnsureTmpDir() { - utils.EnsureDir(gp.Tmp) +func (gp *generatedPaths) EnsureTmpDir() error { + return utils.EnsureDir(gp.Tmp) } -func (gp *generatedPaths) EmptyTmpDir() { - utils.EmptyDir(gp.Tmp) +func (gp *generatedPaths) EmptyTmpDir() error { + return utils.EmptyDir(gp.Tmp) } -func (gp *generatedPaths) RemoveTmpDir() { - utils.RemoveDir(gp.Tmp) +func (gp *generatedPaths) RemoveTmpDir() error { + return utils.RemoveDir(gp.Tmp) } func (gp *generatedPaths) TempDir(pattern string) (string, error) { - gp.EnsureTmpDir() + if err := gp.EnsureTmpDir(); err != nil { + logger.Warnf("Could not ensure existence of a temporary directory: %v", err) + } ret, err := ioutil.TempDir(gp.Tmp, pattern) if err != nil { return "", err } - utils.EmptyDir(ret) + if err = utils.EmptyDir(ret); err != nil { + logger.Warnf("could not recursively empty dir: %v", err) + } return ret, nil } diff --git a/pkg/manager/paths/paths_json.go b/pkg/manager/paths/paths_json.go index c7f3b7490..1bd6a5e59 100644 --- a/pkg/manager/paths/paths_json.go +++ b/pkg/manager/paths/paths_json.go @@ -3,6 +3,7 @@ package paths import ( "path/filepath" + "github.com/stashapp/stash/pkg/logger" "github.com/stashapp/stash/pkg/utils" ) @@ -43,14 +44,30 @@ func GetJSONPaths(baseDir string) *JSONPaths { func EnsureJSONDirs(baseDir string) { jsonPaths := GetJSONPaths(baseDir) - utils.EnsureDir(jsonPaths.Metadata) - utils.EnsureDir(jsonPaths.Scenes) - utils.EnsureDir(jsonPaths.Images) - utils.EnsureDir(jsonPaths.Galleries) - utils.EnsureDir(jsonPaths.Performers) - utils.EnsureDir(jsonPaths.Studios) - utils.EnsureDir(jsonPaths.Movies) - utils.EnsureDir(jsonPaths.Tags) + if err := utils.EnsureDir(jsonPaths.Metadata); err != nil { + logger.Warnf("couldn't create directories for Metadata: %v", err) + } + if err := utils.EnsureDir(jsonPaths.Scenes); err != nil { + logger.Warnf("couldn't create directories for Scenes: %v", err) + } + if err := utils.EnsureDir(jsonPaths.Images); err != nil { + logger.Warnf("couldn't create directories for Images: %v", err) + } + if err := utils.EnsureDir(jsonPaths.Galleries); err != nil { + logger.Warnf("couldn't create directories for Galleries: %v", err) + } + if err := utils.EnsureDir(jsonPaths.Performers); err != nil { + logger.Warnf("couldn't create directories for Performers: %v", err) + } + if err := utils.EnsureDir(jsonPaths.Studios); err != nil { + logger.Warnf("couldn't create directories for Studios: %v", err) + } + if err := utils.EnsureDir(jsonPaths.Movies); err != nil { + logger.Warnf("couldn't create directories for Movies: %v", err) + } + if err := utils.EnsureDir(jsonPaths.Tags); err != nil { + logger.Warnf("couldn't create directories for Tags: %v", err) + } } func (jp *JSONPaths) PerformerJSONPath(checksum string) string { diff --git a/pkg/manager/running_streams.go b/pkg/manager/running_streams.go index 0b3357277..99446e614 100644 --- a/pkg/manager/running_streams.go +++ b/pkg/manager/running_streams.go @@ -91,10 +91,16 @@ func (s *SceneServer) ServeScreenshot(scene *models.Scene, w http.ResponseWriter http.ServeFile(w, r, filepath) } else { var cover []byte - s.TXNManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { + err := s.TXNManager.WithReadTxn(r.Context(), func(repo models.ReaderRepository) error { cover, _ = repo.Scene().GetCover(scene.ID) return nil }) - utils.ServeImage(cover, w, r) + if err != nil { + logger.Warnf("read transaction failed while serving screenshot: %v", err) + } + + if err = utils.ServeImage(cover, w, r); err != nil { + logger.Warnf("unable to serve screenshot image: %v", err) + } } } diff --git a/pkg/manager/screenshot.go b/pkg/manager/screenshot.go index fc417ede7..739162491 100644 --- a/pkg/manager/screenshot.go +++ b/pkg/manager/screenshot.go @@ -2,6 +2,7 @@ package manager import ( "github.com/stashapp/stash/pkg/ffmpeg" + "github.com/stashapp/stash/pkg/logger" ) func makeScreenshot(probeResult ffmpeg.VideoFile, outputPath string, quality int, width int, time float64) { @@ -12,5 +13,8 @@ func makeScreenshot(probeResult ffmpeg.VideoFile, outputPath string, quality int Time: time, Width: width, } - encoder.Screenshot(probeResult, options) + + if err := encoder.Screenshot(probeResult, options); err != nil { + logger.Warnf("[encoder] failure to generate screenshot: %v", err) + } } diff --git a/pkg/manager/task_export.go b/pkg/manager/task_export.go index 65d925d1b..58c1ca834 100644 --- a/pkg/manager/task_export.go +++ b/pkg/manager/task_export.go @@ -126,7 +126,7 @@ func (t *ExportTask) Start(wg *sync.WaitGroup) { paths.EnsureJSONDirs(t.baseDir) - t.txnManager.WithReadTxn(context.TODO(), func(r models.ReaderRepository) error { + txnErr := t.txnManager.WithReadTxn(context.TODO(), func(r models.ReaderRepository) error { // include movie scenes and gallery images if !t.full { // only include movie scenes if includeDependencies is also set @@ -154,6 +154,9 @@ func (t *ExportTask) Start(wg *sync.WaitGroup) { return nil }) + if txnErr != nil { + logger.Warnf("error while running export transaction: %v", txnErr) + } if err := t.json.saveMappings(t.Mappings); err != nil { logger.Errorf("[mappings] failed to save json: %s", err.Error()) @@ -202,17 +205,24 @@ func (t *ExportTask) zipFiles(w io.Writer) error { return err } - filepath.Walk(t.json.json.Tags, t.zipWalkFunc(u.json.Tags, z)) - filepath.Walk(t.json.json.Galleries, t.zipWalkFunc(u.json.Galleries, z)) - filepath.Walk(t.json.json.Performers, t.zipWalkFunc(u.json.Performers, z)) - filepath.Walk(t.json.json.Studios, t.zipWalkFunc(u.json.Studios, z)) - filepath.Walk(t.json.json.Movies, t.zipWalkFunc(u.json.Movies, z)) - filepath.Walk(t.json.json.Scenes, t.zipWalkFunc(u.json.Scenes, z)) - filepath.Walk(t.json.json.Images, t.zipWalkFunc(u.json.Images, z)) + walkWarn(t.json.json.Tags, t.zipWalkFunc(u.json.Tags, z)) + walkWarn(t.json.json.Galleries, t.zipWalkFunc(u.json.Galleries, z)) + walkWarn(t.json.json.Performers, t.zipWalkFunc(u.json.Performers, z)) + walkWarn(t.json.json.Studios, t.zipWalkFunc(u.json.Studios, z)) + walkWarn(t.json.json.Movies, t.zipWalkFunc(u.json.Movies, z)) + walkWarn(t.json.json.Scenes, t.zipWalkFunc(u.json.Scenes, z)) + walkWarn(t.json.json.Images, t.zipWalkFunc(u.json.Images, z)) return nil } +// like filepath.Walk but issue a warning on error +func walkWarn(root string, fn filepath.WalkFunc) { + if err := filepath.Walk(root, fn); err != nil { + logger.Warnf("error walking structure %v: %v", root, err) + } +} + func (t *ExportTask) zipWalkFunc(outDir string, z *zip.Writer) filepath.WalkFunc { return func(path string, info os.FileInfo, err error) error { if err != nil { diff --git a/pkg/manager/task_import.go b/pkg/manager/task_import.go index 1bf2f3476..374c61296 100644 --- a/pkg/manager/task_import.go +++ b/pkg/manager/task_import.go @@ -160,7 +160,9 @@ func (t *ImportTask) unzipFile() error { fn := filepath.Join(t.BaseDir, f.Name) if f.FileInfo().IsDir() { - os.MkdirAll(fn, os.ModePerm) + if err := os.MkdirAll(fn, os.ModePerm); err != nil { + logger.Warnf("couldn't create directory %v while unzipping import file: %v", fn, err) + } continue } diff --git a/pkg/manager/task_scan.go b/pkg/manager/task_scan.go index 33a159b04..34f7c2208 100644 --- a/pkg/manager/task_scan.go +++ b/pkg/manager/task_scan.go @@ -87,7 +87,9 @@ func (j *ScanJob) Execute(ctx context.Context, progress *job.Progress) { galleries = append(galleries, path) } - instance.Paths.Generated.EnsureTmpDir() + if err := instance.Paths.Generated.EnsureTmpDir(); err != nil { + logger.Warnf("couldn't create temporary directory: %v", err) + } wg.Add() task := ScanTask{ @@ -126,7 +128,11 @@ func (j *ScanJob) Execute(ctx context.Context, progress *job.Progress) { } wg.Wait() - instance.Paths.Generated.EmptyTmpDir() + + if err := instance.Paths.Generated.EmptyTmpDir(); err != nil { + logger.Warnf("couldn't empty temporary directory: %v", err) + } + elapsed := time.Since(start) logger.Info(fmt.Sprintf("Scan finished (%s)", elapsed)) @@ -753,7 +759,7 @@ func (t *ScanTask) scanScene() *models.Scene { // check for scene by checksum and oshash - MD5 should be // redundant, but check both - t.TxnManager.WithReadTxn(context.TODO(), func(r models.ReaderRepository) error { + txnErr := t.TxnManager.WithReadTxn(context.TODO(), func(r models.ReaderRepository) error { qb := r.Scene() if checksum != "" { s, _ = qb.FindByChecksum(checksum) @@ -765,6 +771,9 @@ func (t *ScanTask) scanScene() *models.Scene { return nil }) + if txnErr != nil { + logger.Warnf("error in read transaction: %v", txnErr) + } sceneHash := oshash @@ -1320,7 +1329,7 @@ func (t *ScanTask) doesPathExist() bool { gExt := config.GetGalleryExtensions() ret := false - t.TxnManager.WithReadTxn(context.TODO(), func(r models.ReaderRepository) error { + txnErr := t.TxnManager.WithReadTxn(context.TODO(), func(r models.ReaderRepository) error { if matchExtension(t.FilePath, gExt) { gallery, _ := r.Gallery().FindByPath(t.FilePath) if gallery != nil { @@ -1340,6 +1349,9 @@ func (t *ScanTask) doesPathExist() bool { return nil }) + if txnErr != nil { + logger.Warnf("error while executing read transaction: %v", txnErr) + } return ret } diff --git a/pkg/manager/task_stash_box_tag.go b/pkg/manager/task_stash_box_tag.go index edbcd83e4..8429fb2ee 100644 --- a/pkg/manager/task_stash_box_tag.go +++ b/pkg/manager/task_stash_box_tag.go @@ -47,7 +47,7 @@ func (t *StashBoxPerformerTagTask) stashBoxPerformerTag() { if t.refresh { var performerID string - t.txnManager.WithReadTxn(context.TODO(), func(r models.ReaderRepository) error { + txnErr := t.txnManager.WithReadTxn(context.TODO(), func(r models.ReaderRepository) error { stashids, _ := r.Performer().GetStashIDs(t.performer.ID) for _, id := range stashids { if id.Endpoint == t.box.Endpoint { @@ -56,6 +56,9 @@ func (t *StashBoxPerformerTagTask) stashBoxPerformerTag() { } return nil }) + if txnErr != nil { + logger.Warnf("error while executing read transaction: %v", err) + } if performerID != "" { performer, err = client.FindStashBoxPerformerByID(performerID) } diff --git a/pkg/plugin/js.go b/pkg/plugin/js.go index 6b070ce46..1c8f18a32 100644 --- a/pkg/plugin/js.go +++ b/pkg/plugin/js.go @@ -2,6 +2,7 @@ package plugin import ( "errors" + "fmt" "path/filepath" "sync" @@ -71,10 +72,21 @@ func (t *jsPluginTask) Start() error { return err } - t.vm.Set("input", t.input) - js.AddLogAPI(t.vm, t.progress) - js.AddUtilAPI(t.vm) - js.AddGQLAPI(t.vm, t.input.ServerConnection.SessionCookie, t.gqlHandler) + if err := t.vm.Set("input", t.input); err != nil { + return fmt.Errorf("error setting input: %w", err) + } + + if err := js.AddLogAPI(t.vm, t.progress); err != nil { + return fmt.Errorf("error adding log API: %w", err) + } + + if err := js.AddUtilAPI(t.vm); err != nil { + return fmt.Errorf("error adding util API: %w", err) + } + + if err := js.AddGQLAPI(t.vm, t.input.ServerConnection.SessionCookie, t.gqlHandler); err != nil { + return fmt.Errorf("error adding GraphQL API: %w", err) + } t.vm.Interrupt = make(chan func(), 1) diff --git a/pkg/plugin/js/gql.go b/pkg/plugin/js/gql.go index 13d5fe003..45f9ac9e7 100644 --- a/pkg/plugin/js/gql.go +++ b/pkg/plugin/js/gql.go @@ -103,9 +103,15 @@ func gqlRequestFunc(vm *otto.Otto, cookie *http.Cookie, gqlHandler http.Handler) } } -func AddGQLAPI(vm *otto.Otto, cookie *http.Cookie, gqlHandler http.Handler) { +func AddGQLAPI(vm *otto.Otto, cookie *http.Cookie, gqlHandler http.Handler) error { gql, _ := vm.Object("({})") - gql.Set("Do", gqlRequestFunc(vm, cookie, gqlHandler)) + if err := gql.Set("Do", gqlRequestFunc(vm, cookie, gqlHandler)); err != nil { + return fmt.Errorf("unable to set GraphQL Do function: %w", err) + } - vm.Set("gql", gql) + if err := vm.Set("gql", gql); err != nil { + return fmt.Errorf("unable to set gql: %w", err) + } + + return nil } diff --git a/pkg/plugin/js/log.go b/pkg/plugin/js/log.go index 35d23a537..9f5acde89 100644 --- a/pkg/plugin/js/log.go +++ b/pkg/plugin/js/log.go @@ -2,6 +2,7 @@ package js import ( "encoding/json" + "fmt" "math" "github.com/robertkrimen/otto" @@ -64,14 +65,29 @@ func logProgressFunc(c chan float64) func(call otto.FunctionCall) otto.Value { } } -func AddLogAPI(vm *otto.Otto, progress chan float64) { +func AddLogAPI(vm *otto.Otto, progress chan float64) error { log, _ := vm.Object("({})") - log.Set("Trace", logTrace) - log.Set("Debug", logDebug) - log.Set("Info", logInfo) - log.Set("Warn", logWarn) - log.Set("Error", logError) - log.Set("Progress", logProgressFunc(progress)) + if err := log.Set("Trace", logTrace); err != nil { + return fmt.Errorf("error setting Trace: %w", err) + } + if err := log.Set("Debug", logDebug); err != nil { + return fmt.Errorf("error setting Debug: %w", err) + } + if err := log.Set("Info", logInfo); err != nil { + return fmt.Errorf("error setting Info: %w", err) + } + if err := log.Set("Warn", logWarn); err != nil { + return fmt.Errorf("error setting Warn: %w", err) + } + if err := log.Set("Error", logError); err != nil { + return fmt.Errorf("error setting Error: %w", err) + } + if err := log.Set("Progress", logProgressFunc(progress)); err != nil { + return fmt.Errorf("error setting Progress: %v", err) + } + if err := vm.Set("log", log); err != nil { + return fmt.Errorf("unable to set log: %w", err) + } - vm.Set("log", log) + return nil } diff --git a/pkg/plugin/js/util.go b/pkg/plugin/js/util.go index 4590d5e25..1fe5bb3ff 100644 --- a/pkg/plugin/js/util.go +++ b/pkg/plugin/js/util.go @@ -1,6 +1,7 @@ package js import ( + "fmt" "time" "github.com/robertkrimen/otto" @@ -14,9 +15,15 @@ func sleepFunc(call otto.FunctionCall) otto.Value { return otto.UndefinedValue() } -func AddUtilAPI(vm *otto.Otto) { +func AddUtilAPI(vm *otto.Otto) error { util, _ := vm.Object("({})") - util.Set("Sleep", sleepFunc) + if err := util.Set("Sleep", sleepFunc); err != nil { + return fmt.Errorf("unable to set sleep func: %w", err) + } - vm.Set("util", util) + if err := vm.Set("util", util); err != nil { + return fmt.Errorf("unable to set util: %w", err) + } + + return nil } diff --git a/pkg/plugin/plugins.go b/pkg/plugin/plugins.go index 844834339..7322221a9 100644 --- a/pkg/plugin/plugins.go +++ b/pkg/plugin/plugins.go @@ -219,7 +219,9 @@ func (c Cache) executePostHooks(ctx context.Context, hookType HookTriggerEnum, h select { case <-ctx.Done(): - task.Stop() + if err := task.Stop(); err != nil { + logger.Warnf("could not stop task: %v", err) + } return fmt.Errorf("operation cancelled") case <-c: // task finished normally diff --git a/pkg/plugin/raw.go b/pkg/plugin/raw.go index 5d5ce417c..7ded78bd6 100644 --- a/pkg/plugin/raw.go +++ b/pkg/plugin/raw.go @@ -51,7 +51,9 @@ func (t *rawPluginTask) Start() error { defer stdin.Close() inBytes, _ := json.Marshal(t.input) - io.WriteString(stdin, string(inBytes)) + if k, err := io.WriteString(stdin, string(inBytes)); err != nil { + logger.Warnf("error writing input to plugins stdin (wrote %v bytes out of %v): %v", k, len(string(inBytes)), err) + } }() stderr, err := cmd.StderrPipe() diff --git a/pkg/utils/crypto.go b/pkg/utils/crypto.go index 8f20ad9b2..6022aeac8 100644 --- a/pkg/utils/crypto.go +++ b/pkg/utils/crypto.go @@ -7,6 +7,8 @@ import ( "hash/fnv" "io" "os" + + "github.com/stashapp/stash/pkg/logger" ) func MD5FromBytes(data []byte) string { @@ -40,7 +42,9 @@ func MD5FromReader(src io.Reader) (string, error) { func GenerateRandomKey(l int) string { b := make([]byte, l) - rand.Read(b) + if n, err := rand.Read(b); err != nil { + logger.Warnf("failure generating random key: %v (only read %v bytes)", err, n) + } return fmt.Sprintf("%x", b) } diff --git a/scripts/test_db_generator/makeTestDB.go b/scripts/test_db_generator/makeTestDB.go index b7b35089d..300e022c5 100644 --- a/scripts/test_db_generator/makeTestDB.go +++ b/scripts/test_db_generator/makeTestDB.go @@ -6,6 +6,7 @@ import ( "context" "database/sql" "fmt" + "log" "math/rand" "os" "strconv" @@ -43,12 +44,14 @@ func main() { var err error c, err = loadConfig() if err != nil { - panic(err) + log.Fatalf("couldn't load configuration: %v", err) } initNaming(*c) - database.Initialize(c.Database) + if err = database.Initialize(c.Database); err != nil { + log.Fatalf("couldn't initialize database: %v", err) + } populateDB() }