Skip to content

Validate config at startup to prevent ticker panic#30

Merged
LarsLaskowski merged 1 commit into
mainfrom
claude/issue-25-m5dgn8
Jul 13, 2026
Merged

Validate config at startup to prevent ticker panic#30
LarsLaskowski merged 1 commit into
mainfrom
claude/issue-25-m5dgn8

Conversation

@LarsLaskowski

@LarsLaskowski LarsLaskowski commented Jul 13, 2026

Copy link
Copy Markdown
Owner

Summary

internal/config/config.go performed no validation after merging defaults,
YAML, and flags, so broken values reached the runtime unchecked. Most
seriously, poll_interval_seconds: 0 (or negative) produced a 0-duration
time.NewTicker in Collector.Run, which panics and crashes the whole
service — a single typo in /etc/pimonitor/config.yaml took the daemon down
in a Restart=on-failure loop. Other broken values were silently misapplied
(empty listen_addr → port 80, unknown log_level → info, negative
history_window_minutes → broken sparklines, inverted thresholds).

This change makes the daemon fail fast at startup with a clear message:

  • Add Config.Validate() checking poll_interval_seconds > 0,
    updates_check_minutes > 0, updates_stale_threshold_minutes >= 0,
    history_window_minutes > 0, non-empty listen_addr, log_level
    {debug, info, warn, error}, and all thresholds >= 0 with each warn <=
    its crit counterpart. Returns one descriptive error per first violation.
  • Call cfg.Validate() at the end of Load() (after flag overrides). A
    -version request short-circuits before validation so pimonitor -version
    still works against an otherwise invalid config. run() already wraps the
    error as load config: ....
  • As defense in depth, clamp non-positive tick intervals to 1s with a warning
    in Collector.Run so no future caller can trigger the ticker panic.

Verified by building the binary and running it against
poll_interval_seconds: 0:

pimonitor: load config: poll_interval_seconds must be > 0 (got 0)  (exit 1)

No panic, no stack trace.

Related Issue

Closes #25

Checklist

  • Tests added/updated for the change (go test ./... passes locally) —
    table-driven tests in internal/config/config_test.go cover each
    rejected field, valid edge cases (warn == crit, zero stale threshold),
    that Default() passes Validate(), that Load rejects
    poll_interval_seconds: 0, and that -version bypasses validation.
    go test ./... -race passes.
  • go vet ./... and golangci-lint run are clean (0 issues)
  • Documentation updated if this changes the REST API (docs/API.md),
    configuration (README.md, packaging/pimonitor.example.yaml), or
    installation/packaging (packaging/install.sh, systemd units) — n/a,
    no config keys, defaults, or API shapes changed
  • No breaking change to /api/v1/... response shapes, or a new API
    version was introduced instead

An empty or non-positive poll_interval_seconds (or updates_check_minutes)
reached time.NewTicker unchecked and panicked, crashing the service in a
restart loop from a single config typo. Other broken values (empty
listen_addr, unknown log_level, inverted thresholds) were silently
misapplied.

Add Config.Validate() and call it at the end of Load() (after flag
overrides, but skipped for -version) so the daemon fails fast with a
descriptive error. As defense in depth, clamp non-positive tick intervals
to 1s with a warning in Collector.Run so no future caller can trigger the
panic.

Closes #25

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016FQNxpLCapteFxih5iPHy4
@LarsLaskowski
LarsLaskowski merged commit 8b75600 into main Jul 13, 2026
3 checks passed
@LarsLaskowski
LarsLaskowski deleted the claude/issue-25-m5dgn8 branch July 13, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Robustness: config values are not validated — poll_interval_seconds: 0 crashes the collector (ticker panic)

2 participants