Require a positive RETENTION_SWEEP_INTERVAL (closes #140)
All checks were successful
check / check (push) Successful in 6s

A non-positive value reached time.NewTicker in the retention reaper and the archive sweeper, panicking both goroutines after startup had already reported success. envPositiveDuration now rejects it in loadFromEnv, matching how PORT and RECEIVER_RATE_LIMIT fail. SESSION_IDLE_TIMEOUT keeps treating non-positive as disabled, which is guarded at every use site.
This commit was merged in pull request #142.
This commit is contained in:
2026-08-12 12:46:39 +02:00
parent 543005c0c2
commit 3941f0b0ff
3 changed files with 81 additions and 5 deletions

View File

@@ -92,6 +92,7 @@ type Config struct {
SentryDSN string
// RetentionSweepInterval is how often the retention reaper runs.
// Always positive: it becomes a time.NewTicker period.
RetentionSweepInterval time.Duration
// SessionIdleTimeout is the sliding inactivity window after
@@ -235,6 +236,34 @@ func envDuration(
return d, nil
}
// envPositiveDuration returns the value of the named environment
// variable parsed as a Go duration that must be greater than zero.
// Returns defaultValue if not set. A set value that is unparseable or
// non-positive is a hard error naming the key and the bad value.
//
// This is for durations that reach time.NewTicker, which panics on a
// non-positive period, in a goroutine started after startup has
// already reported success. It is deliberately not used for durations
// where non-positive means "disabled" (SESSION_IDLE_TIMEOUT).
func envPositiveDuration(
key string,
defaultValue time.Duration,
) (time.Duration, error) {
d, err := envDuration(key, defaultValue)
if err != nil {
return 0, err
}
if d <= 0 {
return 0, fmt.Errorf(
"%w: %s must be greater than zero, got %s",
ErrNonPositiveValue, key, d,
)
}
return d, nil
}
// parseCIDR parses one trusted-proxy list entry, which may be a
// CIDR block ("10.0.0.0/8") or a bare address ("10.0.0.1", treated
// as a single-host block).
@@ -346,7 +375,7 @@ func loadFromEnv() (*Config, error) {
return nil, err
}
retentionSweepInterval, err := envDuration(
retentionSweepInterval, err := envPositiveDuration(
"RETENTION_SWEEP_INTERVAL",
defaultRetentionSweepInterval,
)
@@ -354,6 +383,8 @@ func loadFromEnv() (*Config, error) {
return nil, err
}
// Non-positive is "disabled" here, not invalid, so this stays on
// envDuration.
sessionIdleTimeout, err := envDuration(
"SESSION_IDLE_TIMEOUT",
defaultSessionIdleTimeout,

View File

@@ -139,7 +139,11 @@ func TestRetentionSweepInterval(t *testing.T) {
set bool
value string
expectError bool
expected time.Duration
// sentinel, when set, must be wrapped by the startup
// error; every error case must additionally name the
// variable in its message.
sentinel error
expected time.Duration
}{
{
name: caseUnsetUsesDefault,
@@ -158,6 +162,24 @@ func TestRetentionSweepInterval(t *testing.T) {
value: "not-a-duration",
expectError: true,
},
{
// A non-positive period panics the ticker in the
// reaper and archive-sweeper goroutines, long after
// startup has reported success, so it has to fail
// here instead.
name: "zero fails startup",
set: true,
value: "0s",
expectError: true,
sentinel: config.ErrNonPositiveValue,
},
{
name: "negative fails startup",
set: true,
value: "-1h",
expectError: true,
sentinel: config.ErrNonPositiveValue,
},
}
for _, tt := range tests {
@@ -175,7 +197,9 @@ func TestRetentionSweepInterval(t *testing.T) {
}
if tt.expectError {
expectStartupError(t)
expectStartupErrorFor(
t, "RETENTION_SWEEP_INTERVAL", tt.sentinel,
)
} else {
testRetentionSweepIntervalSuccess(t, tt.expected)
}
@@ -281,6 +305,22 @@ func TestSessionIdleTimeout(t *testing.T) {
value: "not-a-duration",
expectError: true,
},
{
// Non-positive is "idle expiry disabled" for this
// variable, not a configuration error: unlike
// RETENTION_SWEEP_INTERVAL it never becomes a ticker
// period.
name: "zero disables idle expiry",
set: true,
value: "0s",
expected: 0,
},
{
name: "negative disables idle expiry",
set: true,
value: "-1h",
expected: -time.Hour,
},
}
for _, tt := range tests {