mirror of
https://github.com/fankes/beszel.git
synced 2025-10-19 01:39:34 +08:00
Add panic recovery for sensors.TemperaturesWithContext (#796)
This commit is contained in:
@@ -3,6 +3,7 @@ package agent
|
|||||||
import (
|
import (
|
||||||
"beszel/internal/entities/system"
|
"beszel/internal/entities/system"
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"path"
|
"path"
|
||||||
"strconv"
|
"strconv"
|
||||||
@@ -30,6 +31,9 @@ func (a *Agent) newSensorConfig() *SensorConfig {
|
|||||||
return a.newSensorConfigWithEnv(primarySensor, sysSensors, sensorsEnvVal, skipCollection)
|
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
|
// newSensorConfigWithEnv creates a SensorConfig with the provided environment variables
|
||||||
// sensorsSet indicates if the SENSORS environment variable was explicitly set (even to empty string)
|
// 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 {
|
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
|
// reset high temp
|
||||||
a.systemInfo.DashboardTemp = 0
|
a.systemInfo.DashboardTemp = 0
|
||||||
|
|
||||||
// get sensor data
|
temps, err := a.getTempsWithPanicRecovery(sensors.TemperaturesWithContext)
|
||||||
temps, _ := sensors.TemperaturesWithContext(a.sensorConfig.context)
|
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)
|
slog.Debug("Temperature", "sensors", temps)
|
||||||
|
|
||||||
// return if no sensors
|
// return if no sensors
|
||||||
@@ -107,15 +121,28 @@ func (a *Agent) updateTemperatures(systemStats *system.Stats) {
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
// set dashboard temperature
|
// set dashboard temperature
|
||||||
if a.sensorConfig.primarySensor == "" {
|
switch a.sensorConfig.primarySensor {
|
||||||
|
case "":
|
||||||
a.systemInfo.DashboardTemp = max(a.systemInfo.DashboardTemp, sensor.Temperature)
|
a.systemInfo.DashboardTemp = max(a.systemInfo.DashboardTemp, sensor.Temperature)
|
||||||
} else if a.sensorConfig.primarySensor == sensorName {
|
case sensorName:
|
||||||
a.systemInfo.DashboardTemp = sensor.Temperature
|
a.systemInfo.DashboardTemp = sensor.Temperature
|
||||||
}
|
}
|
||||||
systemStats.Temperatures[sensorName] = twoDecimals(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
|
// isValidSensor checks if a sensor is valid based on the sensor name and the sensor config
|
||||||
func isValidSensor(sensorName string, config *SensorConfig) bool {
|
func isValidSensor(sensorName string, config *SensorConfig) bool {
|
||||||
// if no sensors configured, everything is valid
|
// if no sensors configured, everything is valid
|
||||||
|
@@ -4,11 +4,14 @@
|
|||||||
package agent
|
package agent
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"beszel/internal/entities/system"
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"os"
|
"os"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/shirou/gopsutil/v4/common"
|
"github.com/shirou/gopsutil/v4/common"
|
||||||
|
"github.com/shirou/gopsutil/v4/sensors"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
)
|
)
|
||||||
@@ -454,3 +457,97 @@ func TestScaleTemperatureLogic(t *testing.T) {
|
|||||||
result, expected)
|
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")
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
Reference in New Issue
Block a user