From cc60dc9239df875f672a2537f2d0589a9775bcf7 Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Mon, 17 Aug 2026 15:36:37 -0500 Subject: [PATCH 1/3] 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") +} From 8063f06777067e319d4eac594c682189068dfd4f Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Tue, 18 Aug 2026 08:31:34 -0500 Subject: [PATCH 2/3] Extract recover-and-convert-panic helpers for input plugin calls Replaces the sent-bool guard duplicated across three goroutines with a small per-call helper (recoverInitialize/recoverEvents/recoverMetrics) that wraps the plugin call and converts a panic into a returned error. Each goroutine then always sends its result exactly once, so there's no risk of a double-send deadlocking the collector. Co-Authored-By: Claude Sonnet 5 --- pkg/poller/inputs.go | 104 ++++++++++++++++++------------------------- 1 file changed, 44 insertions(+), 60 deletions(-) diff --git a/pkg/poller/inputs.go b/pkg/poller/inputs.go index 764f7672..1546a4f5 100644 --- a/pkg/poller/inputs.go +++ b/pkg/poller/inputs.go @@ -65,6 +65,21 @@ func NewInput(i *InputPlugin) { inputs = append(inputs, i) } +// recoverInitialize runs input.Initialize and converts a panic into an error. +// Each input plugin's Initialize call runs in its own goroutine, so a panic +// there cannot be caught by a recover() in the caller; left unrecovered, it +// crashes the whole process before the app even starts. +// See https://github.com/unpoller/unpoller/issues/1030 +func recoverInitialize(input *InputPlugin, l Logger) (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("input plugin %s panicked initializing: %v", input.Name, r) //nolint:err113 + } + }() + + return input.Initialize(l) +} + // InitializeInputs runs the passed-in initializer method for each input plugin. func (u *UnifiPoller) InitializeInputs() error { inputSync.RLock() @@ -82,28 +97,12 @@ 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 { + if err := recoverInitialize(input, u); err != nil { u.LogDebugf("error initializing input ... %s", input.Name) - sent = true - errChan <- err return @@ -111,8 +110,6 @@ func (u *UnifiPoller) InitializeInputs() error { u.LogDebugf("input successfully initialized ... %s", input.Name) - sent = true - errChan <- nil }(input) } @@ -149,6 +146,18 @@ type eventInputResult struct { err error } +// recoverEvents runs input.Events and converts a panic into an error. See +// recoverInitialize for why this is necessary. +func recoverEvents(input *InputPlugin, filter *Filter) (e *Events, err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("input plugin %s panicked collecting events: %v", input.Name, r) //nolint:err113 + } + }() + + return input.Events(filter) +} + func collectEvents(filter *Filter, inputs []*InputPlugin) (*Events, error) { resultChan := make(chan eventInputResult, len(inputs)) wg := &sync.WaitGroup{} @@ -159,41 +168,21 @@ 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 } - e, err := input.Events(filter) + e, err := recoverEvents(input, filter) if err != nil { - sent = true - resultChan <- eventInputResult{err: err} return } - sent = true - resultChan <- eventInputResult{logs: e.Logs} }(input) } @@ -238,6 +227,20 @@ type metricInputResult struct { err error } +// recoverMetrics runs input.Metrics and converts a panic into an error (e.g. +// from a malformed controller response, such as an unexpected Site Speed +// Test aggregated-dashboard payload). See recoverInitialize for why this is +// necessary. +func recoverMetrics(input *InputPlugin, filter *Filter) (m *Metrics, err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("input plugin %s panicked collecting metrics: %v", input.Name, r) //nolint:err113 + } + }() + + return input.Metrics(filter) +} + func collectMetrics(filter *Filter, inputs []*InputPlugin) (*Metrics, error) { resultChan := make(chan metricInputResult, len(inputs)) wg := &sync.WaitGroup{} @@ -248,34 +251,15 @@ 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 - + m, err := recoverMetrics(input, filter) resultChan <- metricInputResult{metric: m, err: err} }(input) } From 12073ba52337c8ac98af315e8d0e236dc0302ecd Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Tue, 18 Aug 2026 08:36:50 -0500 Subject: [PATCH 3/3] Fix nil-pointer panic in collectEvents for disabled input plugins InputUnifi.Events returns (nil, nil) when disabled, but collectEvents dereferenced e.Logs unconditionally after a successful (err == nil) call, crashing the poller. Same crash class as #1030, found while verifying the recover-based fix. --- pkg/poller/inputs.go | 6 ++++++ pkg/poller/inputs_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/pkg/poller/inputs.go b/pkg/poller/inputs.go index 1546a4f5..4b6d5073 100644 --- a/pkg/poller/inputs.go +++ b/pkg/poller/inputs.go @@ -183,6 +183,12 @@ func collectEvents(filter *Filter, inputs []*InputPlugin) (*Events, error) { return } + if e == nil { + resultChan <- eventInputResult{} + + return + } + resultChan <- eventInputResult{logs: e.Logs} }(input) } diff --git a/pkg/poller/inputs_test.go b/pkg/poller/inputs_test.go index 2206dd09..6a41f2d1 100644 --- a/pkg/poller/inputs_test.go +++ b/pkg/poller/inputs_test.go @@ -46,6 +46,38 @@ func TestCollectMetricsRecoversPanickingInput(t *testing.T) { assert.Contains(t, err.Error(), "panic-input") } +// nilEventsInput simulates a disabled input plugin, which returns (nil, nil) +// from Events. See https://github.com/unpoller/unpoller/issues/1030. +type nilEventsInput struct{} + +func (nilEventsInput) Initialize(poller.Logger) error { return nil } + +func (nilEventsInput) Metrics(*poller.Filter) (*poller.Metrics, error) { return nil, nil } + +func (nilEventsInput) Events(*poller.Filter) (*poller.Events, error) { return nil, nil } + +func (nilEventsInput) RawMetrics(*poller.Filter) ([]byte, error) { return nil, nil } + +func (nilEventsInput) DebugInput() (bool, error) { return false, nil } + +func TestCollectEventsHandlesNilEventsResult(t *testing.T) { + t.Parallel() + + collector := poller.NewTestCollector(t) + collector.AddInput(&poller.InputPlugin{Name: "nil-events-input", Input: nilEventsInput{}}) + + var events *poller.Events + + var err error + + require.NotPanics(t, func() { + events, err = collector.Events(nil) + }) + + assert.NotNil(t, events) + require.NoError(t, err) +} + func TestCollectEventsRecoversPanickingInput(t *testing.T) { t.Parallel()