From d0e73c7e7fffa0850f19fa911798bfdf2e36929c Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 7 Aug 2026 20:45:40 +0700 Subject: [PATCH] Fail startup on an unparseable RETENTION_SWEEP_INTERVAL --- internal/config/config.go | 59 +++++++++++++-------- internal/config/config_test.go | 95 ++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+), 22 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 95749ac..414a3bb 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -106,19 +106,26 @@ func envInt(key string, defaultValue int) int { // envDuration returns the value of the named environment variable // parsed as a Go duration (e.g. "1h", "30m"). Returns defaultValue if -// not set or unparseable. +// not set. If the variable is set but cannot be parsed, it returns a +// wrapped error naming the key and the bad value, so startup fails +// loudly rather than silently falling back to the default. func envDuration( key string, defaultValue time.Duration, -) time.Duration { - if v := os.Getenv(key); v != "" { - d, err := time.ParseDuration(v) - if err == nil { - return d - } +) (time.Duration, error) { + v := os.Getenv(key) + if v == "" { + return defaultValue, nil } - return defaultValue + d, err := time.ParseDuration(v) + if err != nil { + return 0, fmt.Errorf( + "invalid duration for %s: %q: %w", key, v, err, + ) + } + + return d, nil } // New creates a Config by reading environment variables. @@ -144,22 +151,30 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) { ) } + // Parse the retention sweep interval; a set-but-unparseable value + // is a hard error so fx aborts startup rather than silently using + // the default. + retentionSweepInterval, err := envDuration( + "RETENTION_SWEEP_INTERVAL", + defaultRetentionSweepInterval, + ) + if err != nil { + return nil, err + } + // Load configuration values from environment variables s := &Config{ - DataDir: envString("DATA_DIR"), - Debug: envBool("DEBUG", false), - MaintenanceMode: envBool("MAINTENANCE_MODE", false), - Environment: environment, - MetricsUsername: envString("METRICS_USERNAME"), - MetricsPassword: envString("METRICS_PASSWORD"), - Port: envInt("PORT", defaultPort), - SentryDSN: envString("SENTRY_DSN"), - RetentionSweepInterval: envDuration( - "RETENTION_SWEEP_INTERVAL", - defaultRetentionSweepInterval, - ), - log: log, - params: ¶ms, + DataDir: envString("DATA_DIR"), + Debug: envBool("DEBUG", false), + MaintenanceMode: envBool("MAINTENANCE_MODE", false), + Environment: environment, + MetricsUsername: envString("METRICS_USERNAME"), + MetricsPassword: envString("METRICS_PASSWORD"), + Port: envInt("PORT", defaultPort), + SentryDSN: envString("SENTRY_DSN"), + RetentionSweepInterval: retentionSweepInterval, + log: log, + params: ¶ms, } // Set default DataDir. All SQLite databases (main application diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b17b05d..7e3c2c8 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,6 +3,7 @@ package config_test import ( "os" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -120,6 +121,100 @@ func testEnvironmentConfigSuccess( assert.Equal(t, isProd, cfg.IsProd()) } +func TestRetentionSweepInterval(t *testing.T) { + tests := []struct { + name string + set bool + value string + expectError bool + expected time.Duration + }{ + { + name: "unset uses default", + set: false, + expected: time.Hour, + }, + { + name: "valid value is parsed", + set: true, + value: "15m", + expected: 15 * time.Minute, + }, + { + name: "unparseable value fails startup", + set: true, + value: "not-a-duration", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Cannot use t.Parallel() here because t.Setenv + // is incompatible with parallel subtests. + t.Setenv("WEBHOOKER_ENVIRONMENT", "dev") + + if tt.set { + t.Setenv("RETENTION_SWEEP_INTERVAL", tt.value) + } else { + require.NoError(t, os.Unsetenv( + "RETENTION_SWEEP_INTERVAL", + )) + } + + if tt.expectError { + testRetentionSweepIntervalError(t) + } else { + testRetentionSweepIntervalSuccess(t, tt.expected) + } + }) + } +} + +func testRetentionSweepIntervalError(t *testing.T) { + t.Helper() + + var cfg *config.Config + + app := fx.New( + fx.NopLogger, + fx.Provide( + globals.New, + logger.New, + config.New, + ), + fx.Populate(&cfg), + ) + + assert.Error(t, app.Err()) +} + +func testRetentionSweepIntervalSuccess( + t *testing.T, + expected time.Duration, +) { + t.Helper() + + var cfg *config.Config + + app := fxtest.New( + t, + fx.Provide( + globals.New, + logger.New, + config.New, + ), + fx.Populate(&cfg), + ) + require.NoError(t, app.Err()) + + app.RequireStart() + + defer app.RequireStop() + + assert.Equal(t, expected, cfg.RetentionSweepInterval) +} + func TestDefaultDataDir(t *testing.T) { for _, env := range []string{"", "dev", "prod"} { name := env