From cc60dc9239df875f672a2537f2d0589a9775bcf7 Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Mon, 17 Aug 2026 15:36:37 -0500 Subject: [PATCH] Recover input plugin panics in poller goroutines (fixes #1030) Metrics/Events/Initialize each fan out to input plugins in their own goroutines with no recover(), so a panic there (e.g. a UniFi controller returning an unexpected Site Speed Test aggregated-dashboard payload) crashes the whole process with exit code 2. Because the panic occurs in a child goroutine, promunifi's existing safeRefresh recover() in the caller's goroutine never sees it, which is why the crash survived the earlier robustness work. This converts a panicking input into a logged/returned error so polling continues instead of crashing. Co-Authored-By: Claude Sonnet 5 --- pkg/poller/inputs.go | 58 ++++++++++++++++++++++++++++++++++ pkg/poller/inputs_test.go | 66 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 pkg/poller/inputs_test.go diff --git a/pkg/poller/inputs.go b/pkg/poller/inputs.go index c3a913d5..764f7672 100644 --- a/pkg/poller/inputs.go +++ b/pkg/poller/inputs.go @@ -81,12 +81,29 @@ func (u *UnifiPoller) InitializeInputs() error { go func(input *InputPlugin) { defer wg.Done() + + sent := false + + // A panicking input plugin runs in its own goroutine, so it cannot be + // caught by a recover() in the caller. Without this, a panic here + // crashes the whole process before the app even starts. + // See https://github.com/unpoller/unpoller/issues/1030 + defer func() { + if r := recover(); r != nil && !sent { + u.LogErrorf("input plugin %s panicked initializing (see issue #1030): %v", input.Name, r) + + errChan <- fmt.Errorf("input plugin %s panicked initializing: %v", input.Name, r) //nolint:err113 + } + }() + // This must return, or the app locks up here. u.LogDebugf("inititalizing input... %s", input.Name) if err := input.Initialize(u); err != nil { u.LogDebugf("error initializing input ... %s", input.Name) + sent = true + errChan <- err return @@ -94,6 +111,8 @@ func (u *UnifiPoller) InitializeInputs() error { u.LogDebugf("input successfully initialized ... %s", input.Name) + sent = true + errChan <- nil }(input) } @@ -140,9 +159,25 @@ func collectEvents(filter *Filter, inputs []*InputPlugin) (*Events, error) { go func(input *InputPlugin) { defer wg.Done() + sent := false + + // A panicking input plugin runs in its own goroutine, so it cannot be + // caught by a recover() in the caller. Without this, a panic here + // (e.g. from a malformed controller response) crashes the whole + // process. See https://github.com/unpoller/unpoller/issues/1030 + defer func() { + if r := recover(); r != nil && !sent { + resultChan <- eventInputResult{ + err: fmt.Errorf("input plugin %s panicked collecting events (see issue #1030): %v", input.Name, r), + } + } + }() + if filter != nil && filter.Name != "" && !strings.EqualFold(input.Name, filter.Name) { + sent = true + resultChan <- eventInputResult{} return @@ -150,11 +185,15 @@ func collectEvents(filter *Filter, inputs []*InputPlugin) (*Events, error) { e, err := input.Events(filter) if err != nil { + sent = true + resultChan <- eventInputResult{err: err} return } + sent = true + resultChan <- eventInputResult{logs: e.Logs} }(input) } @@ -209,15 +248,34 @@ func collectMetrics(filter *Filter, inputs []*InputPlugin) (*Metrics, error) { go func(input *InputPlugin) { defer wg.Done() + sent := false + + // A panicking input plugin runs in its own goroutine, so it cannot be + // caught by a recover() in the caller. Without this, a panic here + // (e.g. from a malformed controller response, such as an unexpected + // Site Speed Test aggregated-dashboard payload) crashes the whole + // process. See https://github.com/unpoller/unpoller/issues/1030 + defer func() { + if r := recover(); r != nil && !sent { + resultChan <- metricInputResult{ + err: fmt.Errorf("input plugin %s panicked collecting metrics (see issue #1030): %v", input.Name, r), + } + } + }() + if filter != nil && filter.Name != "" && !strings.EqualFold(input.Name, filter.Name) { + sent = true + resultChan <- metricInputResult{} return } m, err := input.Metrics(filter) + sent = true + resultChan <- metricInputResult{metric: m, err: err} }(input) } diff --git a/pkg/poller/inputs_test.go b/pkg/poller/inputs_test.go new file mode 100644 index 00000000..2206dd09 --- /dev/null +++ b/pkg/poller/inputs_test.go @@ -0,0 +1,66 @@ +package poller_test + +import ( + "testing" + + "github.com/unpoller/unpoller/pkg/poller" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// panicInput is an Input that panics on Metrics and Events, simulating a +// malformed controller response crashing an input plugin. See issue #1030. +type panicInput struct{} + +func (panicInput) Initialize(poller.Logger) error { return nil } + +func (panicInput) Metrics(*poller.Filter) (*poller.Metrics, error) { + panic("simulated aggregated-dashboard panic") +} + +func (panicInput) Events(*poller.Filter) (*poller.Events, error) { + panic("simulated aggregated-dashboard panic") +} + +func (panicInput) RawMetrics(*poller.Filter) ([]byte, error) { return nil, nil } + +func (panicInput) DebugInput() (bool, error) { return false, nil } + +func TestCollectMetricsRecoversPanickingInput(t *testing.T) { + t.Parallel() + + collector := poller.NewTestCollector(t) + collector.AddInput(&poller.InputPlugin{Name: "panic-input", Input: panicInput{}}) + + var metrics *poller.Metrics + + var err error + + require.NotPanics(t, func() { + metrics, err = collector.Metrics(nil) + }) + + assert.NotNil(t, metrics) + require.Error(t, err) + assert.Contains(t, err.Error(), "panic-input") +} + +func TestCollectEventsRecoversPanickingInput(t *testing.T) { + t.Parallel() + + collector := poller.NewTestCollector(t) + collector.AddInput(&poller.InputPlugin{Name: "panic-input", Input: panicInput{}}) + + var events *poller.Events + + var err error + + require.NotPanics(t, func() { + events, err = collector.Events(nil) + }) + + assert.NotNil(t, events) + require.Error(t, err) + assert.Contains(t, err.Error(), "panic-input") +}