From 377c6c6e0d580fef3f0a2ee94f449933441f58e8 Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Sat, 23 Aug 2025 14:09:40 -0300 Subject: [PATCH] fix: Remove log.Fatal in favor of returning errors (#953) * fix: Remove log.Fatal in favor of returning errors This change is useful for including error tracking, which needs the application to not terminate immediately, and instead give the tracer time to capture and flush errors. * Fix CodeRabbit issues --------- Co-authored-by: Matthew Kilgore --- backend/app/api/main.go | 31 +++++++++++++--- backend/app/api/recurring.go | 7 ++-- backend/app/api/setup.go | 37 ++++++++++++------- .../internal/data/migrations/migrations.go | 13 ++++--- 4 files changed, 60 insertions(+), 28 deletions(-) diff --git a/backend/app/api/main.go b/backend/app/api/main.go index ec01c9c2..c651f3c1 100644 --- a/backend/app/api/main.go +++ b/backend/app/api/main.go @@ -103,31 +103,50 @@ func run(cfg *config.Config) error { // ========================================================================= // Initialize Database & Repos - setupStorageDir(cfg) + err := setupStorageDir(cfg) + if err != nil { + return err + } if strings.ToLower(cfg.Database.Driver) == "postgres" { if !validatePostgresSSLMode(cfg.Database.SslMode) { - log.Fatal().Str("sslmode", cfg.Database.SslMode).Msg("invalid sslmode") + log.Error().Str("sslmode", cfg.Database.SslMode).Msg("invalid sslmode") + return fmt.Errorf("invalid sslmode: %s", cfg.Database.SslMode) } } - databaseURL := setupDatabaseURL(cfg) + databaseURL, err := setupDatabaseURL(cfg) + if err != nil { + return err + } c, err := ent.Open(strings.ToLower(cfg.Database.Driver), databaseURL) if err != nil { - log.Fatal(). + log.Error(). Err(err). Str("driver", strings.ToLower(cfg.Database.Driver)). Str("host", cfg.Database.Host). Str("port", cfg.Database.Port). Str("database", cfg.Database.Database). Msg("failed opening connection to {driver} database at {host}:{port}/{database}") + return fmt.Errorf("failed opening connection to %s database at %s:%s/%s: %w", + strings.ToLower(cfg.Database.Driver), + cfg.Database.Host, + cfg.Database.Port, + cfg.Database.Database, + err, + ) } - goose.SetBaseFS(migrations.Migrations(strings.ToLower(cfg.Database.Driver))) + migrationsFs, err := migrations.Migrations(strings.ToLower(cfg.Database.Driver)) + if err != nil { + return fmt.Errorf("failed to get migrations for %s: %w", strings.ToLower(cfg.Database.Driver), err) + } + + goose.SetBaseFS(migrationsFs) err = goose.SetDialect(strings.ToLower(cfg.Database.Driver)) if err != nil { - log.Fatal().Str("driver", cfg.Database.Driver).Msg("unsupported database driver") + log.Error().Str("driver", cfg.Database.Driver).Msg("unsupported database driver") return fmt.Errorf("unsupported database driver: %s", cfg.Database.Driver) } diff --git a/backend/app/api/recurring.go b/backend/app/api/recurring.go index d6df8511..8c276836 100644 --- a/backend/app/api/recurring.go +++ b/backend/app/api/recurring.go @@ -3,13 +3,13 @@ package main import ( "context" "fmt" - "github.com/hay-kot/httpkit/graceful" - "github.com/sysadminsmedia/homebox/backend/internal/sys/config" "net/http" "time" "github.com/google/uuid" + "github.com/hay-kot/httpkit/graceful" "github.com/rs/zerolog/log" + "github.com/sysadminsmedia/homebox/backend/internal/sys/config" "github.com/sysadminsmedia/homebox/backend/pkgs/utils" "gocloud.dev/pubsub" ) @@ -22,7 +22,8 @@ func registerRecurringTasks(app *app, cfg *config.Config, runner *graceful.Runne log.Info().Msg("Running in demo mode, creating demo data") err := app.SetupDemo() if err != nil { - log.Fatal().Msg(err.Error()) + log.Error().Err(err).Msg("failed to setup demo data") + return fmt.Errorf("failed to setup demo data: %w", err) } } return nil diff --git a/backend/app/api/setup.go b/backend/app/api/setup.go index ecc7bad0..78b374bd 100644 --- a/backend/app/api/setup.go +++ b/backend/app/api/setup.go @@ -13,28 +13,32 @@ import ( ) // setupStorageDir handles the creation and validation of the storage directory. -func setupStorageDir(cfg *config.Config) { +func setupStorageDir(cfg *config.Config) error { if strings.HasPrefix(cfg.Storage.ConnString, "file:///./") { raw := strings.TrimPrefix(cfg.Storage.ConnString, "file:///./") clean := filepath.Clean(raw) absBase, err := filepath.Abs(clean) if err != nil { - log.Fatal().Err(err).Msg("failed to get absolute path for storage connection string") + log.Error().Err(err).Msg("failed to get absolute path for storage connection string") + return fmt.Errorf("failed to get absolute path for storage connection string: %w", err) } absBase = strings.ReplaceAll(absBase, "\\", "/") storageDir := filepath.Join(absBase, cfg.Storage.PrefixPath) storageDir = strings.ReplaceAll(storageDir, "\\", "/") if !strings.HasPrefix(storageDir, absBase+"/") && storageDir != absBase { - log.Fatal().Str("path", storageDir).Msg("invalid storage path: you tried to use a prefix that is not a subdirectory of the base path") + log.Error().Str("path", storageDir).Msg("invalid storage path: you tried to use a prefix that is not a subdirectory of the base path") + return fmt.Errorf("invalid storage path: you tried to use a prefix that is not a subdirectory of the base path") } if err := os.MkdirAll(storageDir, 0o750); err != nil { - log.Fatal().Err(err).Msg("failed to create data directory") + log.Error().Err(err).Msg("failed to create data directory") + return fmt.Errorf("failed to create data directory: %w", err) } } + return nil } // setupDatabaseURL returns the database URL and ensures any required directories exist. -func setupDatabaseURL(cfg *config.Config) string { +func setupDatabaseURL(cfg *config.Config) (string, error) { databaseURL := "" switch strings.ToLower(cfg.Database.Driver) { case "sqlite3": @@ -42,7 +46,8 @@ func setupDatabaseURL(cfg *config.Config) string { dbFilePath := strings.Split(cfg.Database.SqlitePath, "?")[0] dbDir := filepath.Dir(dbFilePath) if err := os.MkdirAll(dbDir, 0o755); err != nil { - log.Fatal().Err(err).Str("path", dbDir).Msg("failed to create SQLite database directory") + log.Error().Err(err).Str("path", dbDir).Msg("failed to create SQLite database directory") + return "", fmt.Errorf("failed to create SQLite database directory: %w", err) } case "postgres": databaseURL = fmt.Sprintf("host=%s port=%s dbname=%s sslmode=%s", cfg.Database.Host, cfg.Database.Port, cfg.Database.Database, cfg.Database.SslMode) @@ -53,27 +58,31 @@ func setupDatabaseURL(cfg *config.Config) string { databaseURL += fmt.Sprintf(" password=%s", cfg.Database.Password) } if cfg.Database.SslRootCert != "" { - if _, err := os.Stat(cfg.Database.SslRootCert); err != nil || !os.IsNotExist(err) { - log.Fatal().Err(err).Str("path", cfg.Database.SslRootCert).Msg("SSL root certificate file does not accessible") + if _, err := os.Stat(cfg.Database.SslRootCert); err != nil { + log.Error().Err(err).Str("path", cfg.Database.SslRootCert).Msg("SSL root certificate file is not accessible") + return "", fmt.Errorf("SSL root certificate file is not accessible: %w", err) } databaseURL += fmt.Sprintf(" sslrootcert=%s", cfg.Database.SslRootCert) } if cfg.Database.SslCert != "" { - if _, err := os.Stat(cfg.Database.SslCert); err != nil || !os.IsNotExist(err) { - log.Fatal().Err(err).Str("path", cfg.Database.SslCert).Msg("SSL certificate file does not accessible") + if _, err := os.Stat(cfg.Database.SslCert); err != nil { + log.Error().Err(err).Str("path", cfg.Database.SslCert).Msg("SSL certificate file is not accessible") + return "", fmt.Errorf("SSL certificate file is not accessible: %w", err) } databaseURL += fmt.Sprintf(" sslcert=%s", cfg.Database.SslCert) } if cfg.Database.SslKey != "" { - if _, err := os.Stat(cfg.Database.SslKey); err != nil || !os.IsNotExist(err) { - log.Fatal().Err(err).Str("path", cfg.Database.SslKey).Msg("SSL key file does not accessible") + if _, err := os.Stat(cfg.Database.SslKey); err != nil { + log.Error().Err(err).Str("path", cfg.Database.SslKey).Msg("SSL key file is not accessible") + return "", fmt.Errorf("SSL key file is not accessible: %w", err) } databaseURL += fmt.Sprintf(" sslkey=%s", cfg.Database.SslKey) } default: - log.Fatal().Str("driver", cfg.Database.Driver).Msg("unsupported database driver") + log.Error().Str("driver", cfg.Database.Driver).Msg("unsupported database driver") + return "", fmt.Errorf("unsupported database driver: %s", cfg.Database.Driver) } - return databaseURL + return databaseURL, nil } // loadCurrencies loads currency data from config if provided. diff --git a/backend/internal/data/migrations/migrations.go b/backend/internal/data/migrations/migrations.go index 01839220..838ba5eb 100644 --- a/backend/internal/data/migrations/migrations.go +++ b/backend/internal/data/migrations/migrations.go @@ -3,6 +3,8 @@ package migrations import ( "embed" + "fmt" + "github.com/rs/zerolog/log" ) @@ -17,15 +19,16 @@ var sqliteFiles embed.FS // migration files in the binary at build time. The function takes a string // parameter "dialect" which specifies the SQL dialect to use. It returns an // embedded file system containing the migration files for the specified dialect. -func Migrations(dialect string) embed.FS { +func Migrations(dialect string) (embed.FS, error) { switch dialect { case "postgres": - return postgresFiles + return postgresFiles, nil case "sqlite3": - return sqliteFiles + return sqliteFiles, nil default: - log.Fatal().Str("dialect", dialect).Msg("unknown sql dialect") + log.Error().Str("dialect", dialect).Msg("unknown sql dialect") + return embed.FS{}, fmt.Errorf("unknown sql dialect: %s", dialect) } // This should never get hit, but just in case - return sqliteFiles + return sqliteFiles, nil }