From 0b04f60b6c7edde99b2410e0b083894982a8f2c3 Mon Sep 17 00:00:00 2001 From: henrygd Date: Mon, 23 Jun 2025 19:50:11 -0400 Subject: [PATCH] Add panic recovery for sensors.TemperaturesWithContext (#796) --- beszel/internal/agent/sensors.go | 35 ++++++++-- beszel/internal/agent/sensors_test.go | 97 +++++++++++++++++++++++++++ 2 files changed, 128 insertions(+), 4 deletions(-) diff --git a/beszel/internal/agent/sensors.go b/beszel/internal/agent/sensors.go index 384bccc..ca7370c 100644 --- a/beszel/internal/agent/sensors.go +++ b/beszel/internal/agent/sensors.go @@ -3,6 +3,7 @@ package agent import ( "beszel/internal/entities/system" "context" + "fmt" "log/slog" "path" "strconv" @@ -30,6 +31,9 @@ func (a *Agent) newSensorConfig() *SensorConfig { return a.newSensorConfigWithEnv(primarySensor, sysSensors, sensorsEnvVal, skipCollection) } +// Matches sensors.TemperaturesWithContext to allow for panic recovery (gopsutil/issues/1832) +type getTempsFn func(ctx context.Context) ([]sensors.TemperatureStat, error) + // newSensorConfigWithEnv creates a SensorConfig with the provided environment variables // sensorsSet indicates if the SENSORS environment variable was explicitly set (even to empty string) func (a *Agent) newSensorConfigWithEnv(primarySensor, sysSensors, sensorsEnvVal string, skipCollection bool) *SensorConfig { @@ -78,8 +82,18 @@ func (a *Agent) updateTemperatures(systemStats *system.Stats) { // reset high temp a.systemInfo.DashboardTemp = 0 - // get sensor data - temps, _ := sensors.TemperaturesWithContext(a.sensorConfig.context) + temps, err := a.getTempsWithPanicRecovery(sensors.TemperaturesWithContext) + if err != nil { + // retry once on panic (gopsutil/issues/1832) + temps, err = a.getTempsWithPanicRecovery(sensors.TemperaturesWithContext) + if err != nil { + slog.Warn("Error updating temperatures", "err", err) + if len(systemStats.Temperatures) > 0 { + systemStats.Temperatures = make(map[string]float64) + } + return + } + } slog.Debug("Temperature", "sensors", temps) // return if no sensors @@ -107,15 +121,28 @@ func (a *Agent) updateTemperatures(systemStats *system.Stats) { continue } // set dashboard temperature - if a.sensorConfig.primarySensor == "" { + switch a.sensorConfig.primarySensor { + case "": a.systemInfo.DashboardTemp = max(a.systemInfo.DashboardTemp, sensor.Temperature) - } else if a.sensorConfig.primarySensor == sensorName { + case sensorName: a.systemInfo.DashboardTemp = sensor.Temperature } systemStats.Temperatures[sensorName] = twoDecimals(sensor.Temperature) } } +// getTempsWithPanicRecovery wraps sensors.TemperaturesWithContext to recover from panics (gopsutil/issues/1832) +func (a *Agent) getTempsWithPanicRecovery(getTemps getTempsFn) (temps []sensors.TemperatureStat, err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("panic: %v", r) + } + }() + // get sensor data (error ignored intentionally as it may be only with one sensor) + temps, _ = getTemps(a.sensorConfig.context) + return +} + // isValidSensor checks if a sensor is valid based on the sensor name and the sensor config func isValidSensor(sensorName string, config *SensorConfig) bool { // if no sensors configured, everything is valid diff --git a/beszel/internal/agent/sensors_test.go b/beszel/internal/agent/sensors_test.go index 85970ac..32a05dc 100644 --- a/beszel/internal/agent/sensors_test.go +++ b/beszel/internal/agent/sensors_test.go @@ -4,11 +4,14 @@ package agent import ( + "beszel/internal/entities/system" "context" + "fmt" "os" "testing" "github.com/shirou/gopsutil/v4/common" + "github.com/shirou/gopsutil/v4/sensors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -454,3 +457,97 @@ func TestScaleTemperatureLogic(t *testing.T) { result, expected) }) } + +func TestGetTempsWithPanicRecovery(t *testing.T) { + agent := &Agent{ + systemInfo: system.Info{}, + sensorConfig: &SensorConfig{ + context: context.Background(), + }, + } + + tests := []struct { + name string + getTempsFn getTempsFn + expectError bool + errorMsg string + }{ + { + name: "successful_function_call", + getTempsFn: func(ctx context.Context) ([]sensors.TemperatureStat, error) { + return []sensors.TemperatureStat{ + {SensorKey: "test_sensor", Temperature: 45.0}, + }, nil + }, + expectError: false, + }, + { + name: "function_returns_error", + getTempsFn: func(ctx context.Context) ([]sensors.TemperatureStat, error) { + return []sensors.TemperatureStat{ + {SensorKey: "test_sensor", Temperature: 45.0}, + }, fmt.Errorf("sensor error") + }, + expectError: false, // getTempsWithPanicRecovery ignores errors from the function + }, + { + name: "function_panics_with_string", + getTempsFn: func(ctx context.Context) ([]sensors.TemperatureStat, error) { + panic("test panic") + }, + expectError: true, + errorMsg: "panic: test panic", + }, + { + name: "function_panics_with_error", + getTempsFn: func(ctx context.Context) ([]sensors.TemperatureStat, error) { + panic(fmt.Errorf("panic error")) + }, + expectError: true, + errorMsg: "panic:", + }, + { + name: "function_panics_with_index_out_of_bounds", + getTempsFn: func(ctx context.Context) ([]sensors.TemperatureStat, error) { + slice := []int{1, 2, 3} + _ = slice[10] // out of bounds panic + return nil, nil + }, + expectError: true, + errorMsg: "panic:", + }, + { + name: "function_panics_with_any_conversion", + getTempsFn: func(ctx context.Context) ([]sensors.TemperatureStat, error) { + var i any = "string" + _ = i.(int) // type assertion panic + return nil, nil + }, + expectError: true, + errorMsg: "panic:", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var temps []sensors.TemperatureStat + var err error + + // The function should not panic, regardless of what the injected function does + assert.NotPanics(t, func() { + temps, err = agent.getTempsWithPanicRecovery(tt.getTempsFn) + }, "getTempsWithPanicRecovery should not panic") + + if tt.expectError { + assert.Error(t, err, "Expected an error to be returned") + if tt.errorMsg != "" { + assert.Contains(t, err.Error(), tt.errorMsg, + "Error message should contain expected text") + } + assert.Nil(t, temps, "Temps should be nil when panic occurs") + } else { + assert.NoError(t, err, "Should not return error for successful calls") + } + }) + } +}