From e3b1340513f43475cba2a0ed14de23d0e50f75ea Mon Sep 17 00:00:00 2001 From: andig Date: Mon, 22 Jun 2026 11:11:24 +0200 Subject: [PATCH] Config: bound device check value probing with context (#31115) --- server/http_config_device_handler.go | 13 +- server/http_config_helper.go | 312 ++++++++++++++------------- server/http_config_helper_test.go | 26 +++ 3 files changed, 202 insertions(+), 149 deletions(-) diff --git a/server/http_config_device_handler.go b/server/http_config_device_handler.go index e693c8b36..67aeed82c 100644 --- a/server/http_config_device_handler.go +++ b/server/http_config_device_handler.go @@ -9,6 +9,7 @@ import ( "reflect" "slices" "strconv" + "time" "dario.cat/mergo" "github.com/evcc-io/evcc/api/globalconfig" @@ -280,7 +281,11 @@ func deviceStatusHandler(w http.ResponseWriter, r *http.Request) { return } - jsonWrite(w, testInstance(instance)) + // bound the value-probe phase so a blocking getter cannot stall the response + ctx, cancel := context.WithTimeout(r.Context(), 10*time.Second) + defer cancel() + + jsonWrite(w, testInstance(ctx, instance)) } func newDevice[T any](ctx context.Context, class templates.Class, req configReq, newFromConf newFromConfFunc[T], h config.Handler[T], force bool) (*config.Config, error) { @@ -697,5 +702,9 @@ func testConfigHandler(w http.ResponseWriter, r *http.Request) { close(done) defer cancel() - jsonWrite(w, testInstance(instance)) + // bound the value-probe phase so a blocking getter cannot stall the response + probeCtx, probeCancel := context.WithTimeout(r.Context(), 10*time.Second) + defer probeCancel() + + jsonWrite(w, testInstance(probeCtx, instance)) } diff --git a/server/http_config_helper.go b/server/http_config_helper.go index 3ace73443..58f7d3b4f 100644 --- a/server/http_config_helper.go +++ b/server/http_config_helper.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "io" + "maps" "reflect" "slices" "strings" @@ -255,7 +256,8 @@ func hasFeature(instance any, f api.Feature) bool { // testInstance tests the given instance similar to dump // TODO refactor together with dump -func testInstance(instance any) map[string]testResult { +func testInstance(ctx context.Context, instance any) map[string]testResult { + var resMu sync.Mutex res := make(map[string]testResult) makeResult := func(key string, val any, err error) { @@ -266,164 +268,180 @@ func testInstance(instance any) map[string]testResult { } tr.Error = err.Error() } + resMu.Lock() res[key] = tr + resMu.Unlock() } - if dev, ok := api.Cap[api.Meter](instance); ok { - val, err := dev.CurrentPower() - makeResult("power", val, err) - } + done := make(chan struct{}) + go func() { + defer close(done) - if dev, ok := api.Cap[api.MeterEnergy](instance); ok { - val, err := dev.TotalEnergy() - makeResult("energy", val, err) - } - - if dev, ok := api.Cap[api.MeterReturnEnergy](instance); ok { - val, err := dev.ReturnEnergy() - makeResult("returnEnergy", val, err) - } - - if dev, ok := api.Cap[api.Battery](instance); ok { - val, err := dev.Soc() - key := "soc" - if hasFeature(instance, api.Heating) { - key = "temp" - } - makeResult(key, val, err) - } - - if api.HasCap[api.BatteryController](instance) { - makeResult("controllable", true, nil) - } - - if dev, ok := api.Cap[api.VehicleOdometer](instance); ok { - val, err := dev.Odometer() - makeResult("odometer", val, err) - } - - if dev, ok := api.Cap[api.BatteryCapacity](instance); ok { - val := dev.Capacity() - makeResult("capacity", val, nil) - } - - if dev, ok := api.Cap[api.PhaseCurrents](instance); ok { - i1, i2, i3, err := dev.Currents() - makeResult("phaseCurrents", []float64{i1, i2, i3}, err) - } - - if dev, ok := api.Cap[api.PhaseVoltages](instance); ok { - u1, u2, u3, err := dev.Voltages() - makeResult("phaseVoltages", []float64{u1, u2, u3}, err) - } - - if dev, ok := api.Cap[api.PhasePowers](instance); ok { - p1, p2, p3, err := dev.Powers() - makeResult("phasePowers", []float64{p1, p2, p3}, err) - } - - if dev, ok := api.Cap[api.ChargeState](instance); ok { - val, err := dev.Status() - makeResult("chargeStatus", val, err) - } - - if dev, ok := api.Cap[api.Charger](instance); ok { - val, err := dev.Enabled() - makeResult("enabled", val, err) - } - - if dev, ok := api.Cap[api.ChargeRater](instance); ok { - val, err := dev.ChargedEnergy() - makeResult("chargedEnergy", val, err) - } - - if api.HasCap[api.PhaseSwitcher](instance) { - makeResult("phases1p3p", true, nil) - } - - if hasFeature(instance, api.Heating) { - makeResult("heating", true, nil) - } - - if hasFeature(instance, api.IntegratedDevice) { - makeResult("integratedDevice", true, nil) - } - - if hasFeature(instance, api.SwitchDevice) { - makeResult("switchDevice", true, nil) - } - - if dev, ok := api.Cap[api.IconDescriber](instance); ok && dev.Icon() != "" { - makeResult("icon", dev.Icon(), nil) - } - - if cc, ok := api.Cap[api.PhaseDescriber](instance); ok && cc.Phases() == 1 { - makeResult("singlePhase", true, nil) - } - - if dev, ok := api.Cap[api.VehicleRange](instance); ok { - val, err := dev.Range() - makeResult("range", val, err) - } - - if dev, ok := api.Cap[api.SocLimiter](instance); ok { - val, err := dev.GetLimitSoc() - key := "vehicleLimitSoc" - if hasFeature(instance, api.Heating) { - key = "heaterTempLimit" - } - makeResult(key, val, err) - } - - if dev, ok := api.Cap[api.Dimmer](instance); ok { - val, err := dev.Dimmed() - makeResult("dimmed", val, err) - } - - if dev, ok := api.Cap[api.Curtailer](instance); ok { - makeResult("curtailable", true, nil) - if val, err := dev.Curtailed(); err != nil || val { - makeResult("curtailed", true, err) - } - } - - if dev, ok := api.Cap[api.Identifier](instance); ok { - val, err := dev.Identify() - makeResult("identifier", val, err) - } - - if dev, ok := api.Cap[api.Tariff](instance); ok { - rates, err := dev.Rates() - - // Determine field names based on tariff type - var valueKey, ratesKey string - switch dev.Type() { - case api.TariffTypePriceDynamic, api.TariffTypePriceForecast: - valueKey = "price" - ratesKey = "priceRates" - case api.TariffTypeCo2: - valueKey = "co2" - ratesKey = "co2Rates" - case api.TariffTypeSolar: - valueKey = "power" - ratesKey = "solarRates" - default: - valueKey = "price" + if dev, ok := api.Cap[api.Meter](instance); ok { + val, err := dev.CurrentPower() + makeResult("power", val, err) } - if err == nil && len(rates) > 0 { - // Get current rate value - if rate, err := rates.At(time.Now()); err == nil { - makeResult(valueKey, rate.Value, nil) + if dev, ok := api.Cap[api.MeterEnergy](instance); ok { + val, err := dev.TotalEnergy() + makeResult("energy", val, err) + } + + if dev, ok := api.Cap[api.MeterReturnEnergy](instance); ok { + val, err := dev.ReturnEnergy() + makeResult("returnEnergy", val, err) + } + + if dev, ok := api.Cap[api.Battery](instance); ok { + val, err := dev.Soc() + key := "soc" + if hasFeature(instance, api.Heating) { + key = "temp" } + makeResult(key, val, err) + } - if ratesKey != "" { - makeResult(ratesKey, rates, nil) + if api.HasCap[api.BatteryController](instance) { + makeResult("controllable", true, nil) + } + + if dev, ok := api.Cap[api.VehicleOdometer](instance); ok { + val, err := dev.Odometer() + makeResult("odometer", val, err) + } + + if dev, ok := api.Cap[api.BatteryCapacity](instance); ok { + val := dev.Capacity() + makeResult("capacity", val, nil) + } + + if dev, ok := api.Cap[api.PhaseCurrents](instance); ok { + i1, i2, i3, err := dev.Currents() + makeResult("phaseCurrents", []float64{i1, i2, i3}, err) + } + + if dev, ok := api.Cap[api.PhaseVoltages](instance); ok { + u1, u2, u3, err := dev.Voltages() + makeResult("phaseVoltages", []float64{u1, u2, u3}, err) + } + + if dev, ok := api.Cap[api.PhasePowers](instance); ok { + p1, p2, p3, err := dev.Powers() + makeResult("phasePowers", []float64{p1, p2, p3}, err) + } + + if dev, ok := api.Cap[api.ChargeState](instance); ok { + val, err := dev.Status() + makeResult("chargeStatus", val, err) + } + + if dev, ok := api.Cap[api.Charger](instance); ok { + val, err := dev.Enabled() + makeResult("enabled", val, err) + } + + if dev, ok := api.Cap[api.ChargeRater](instance); ok { + val, err := dev.ChargedEnergy() + makeResult("chargedEnergy", val, err) + } + + if api.HasCap[api.PhaseSwitcher](instance) { + makeResult("phases1p3p", true, nil) + } + + if hasFeature(instance, api.Heating) { + makeResult("heating", true, nil) + } + + if hasFeature(instance, api.IntegratedDevice) { + makeResult("integratedDevice", true, nil) + } + + if hasFeature(instance, api.SwitchDevice) { + makeResult("switchDevice", true, nil) + } + + if dev, ok := api.Cap[api.IconDescriber](instance); ok && dev.Icon() != "" { + makeResult("icon", dev.Icon(), nil) + } + + if cc, ok := api.Cap[api.PhaseDescriber](instance); ok && cc.Phases() == 1 { + makeResult("singlePhase", true, nil) + } + + if dev, ok := api.Cap[api.VehicleRange](instance); ok { + val, err := dev.Range() + makeResult("range", val, err) + } + + if dev, ok := api.Cap[api.SocLimiter](instance); ok { + val, err := dev.GetLimitSoc() + key := "vehicleLimitSoc" + if hasFeature(instance, api.Heating) { + key = "heaterTempLimit" + } + makeResult(key, val, err) + } + + if dev, ok := api.Cap[api.Dimmer](instance); ok { + val, err := dev.Dimmed() + makeResult("dimmed", val, err) + } + + if dev, ok := api.Cap[api.Curtailer](instance); ok { + makeResult("curtailable", true, nil) + if val, err := dev.Curtailed(); err != nil || val { + makeResult("curtailed", true, err) } } + + if dev, ok := api.Cap[api.Identifier](instance); ok { + val, err := dev.Identify() + makeResult("identifier", val, err) + } + + if dev, ok := api.Cap[api.Tariff](instance); ok { + rates, err := dev.Rates() + + // Determine field names based on tariff type + var valueKey, ratesKey string + switch dev.Type() { + case api.TariffTypePriceDynamic, api.TariffTypePriceForecast: + valueKey = "price" + ratesKey = "priceRates" + case api.TariffTypeCo2: + valueKey = "co2" + ratesKey = "co2Rates" + case api.TariffTypeSolar: + valueKey = "power" + ratesKey = "solarRates" + default: + valueKey = "price" + } + + if err == nil && len(rates) > 0 { + // Get current rate value + if rate, err := rates.At(time.Now()); err == nil { + makeResult(valueKey, rate.Value, nil) + } + + if ratesKey != "" { + makeResult(ratesKey, rates, nil) + } + } + } + }() + + // bound the probe phase: on ctx.Done return collected results so far. A leaked + // getter goroutine keeps writing to res safely under resMu while we return a copy. + select { + case <-done: + case <-ctx.Done(): } - return res + resMu.Lock() + defer resMu.Unlock() + return maps.Clone(res) } // mergeMaskedAny similar to mergeMasked but for interfaces diff --git a/server/http_config_helper_test.go b/server/http_config_helper_test.go index dcc3c22ec..425e78269 100644 --- a/server/http_config_helper_test.go +++ b/server/http_config_helper_test.go @@ -1,8 +1,10 @@ package server import ( + "context" "encoding/json" "testing" + "time" "github.com/evcc-io/evcc/api/globalconfig" "github.com/evcc-io/evcc/plugin/mqtt" @@ -12,6 +14,30 @@ import ( "github.com/stretchr/testify/require" ) +// blockingMeter is an api.Meter whose CurrentPower blocks until the test ends. +type blockingMeter struct { + done chan struct{} +} + +func (m blockingMeter) CurrentPower() (float64, error) { + <-m.done + return 0, nil +} + +// TestInstanceBoundedByContext ensures testInstance returns promptly when a +// getter blocks, bounded by the context deadline rather than the getter itself. +func TestInstanceBoundedByContext(t *testing.T) { + done := make(chan struct{}) + defer close(done) + + ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) + defer cancel() + + start := time.Now() + testInstance(ctx, blockingMeter{done: done}) + require.Less(t, time.Since(start), 500*time.Millisecond, "testInstance must not wait for blocking getter") +} + func TestConfigReqUnmarshal(t *testing.T) { var req configReq require.NoError(t, json.Unmarshal([]byte(`{