mirror of
https://github.com/unpoller/unpoller.git
synced 2026-09-29 19:11:17 +02:00
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5
parent
a83d2cb019
commit
cc60dc9239
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
Reference in New Issue
Block a user