From 90a437a6889405fea1b2ed0a4ac746850c944920 Mon Sep 17 00:00:00 2001 From: Michael Geers Date: Sun, 2 Mar 2025 13:28:54 +0100 Subject: [PATCH] Config UI: improve device error handling (#19267) --- assets/js/components/Config/ChargerModal.vue | 11 ++-- assets/js/components/Config/MeterModal.vue | 2 +- assets/js/components/Config/VehicleModal.vue | 2 +- assets/js/components/Config/mixins/test.js | 17 ++++++ cmd/setup.go | 2 +- core/circuit/circuit.go | 6 +- server/http_config_device_handler.go | 58 +++++++++++++------- server/http_config_helper.go | 22 +++++++- 8 files changed, 84 insertions(+), 36 deletions(-) diff --git a/assets/js/components/Config/ChargerModal.vue b/assets/js/components/Config/ChargerModal.vue index 903212fbb..7dcd008c7 100644 --- a/assets/js/components/Config/ChargerModal.vue +++ b/assets/js/components/Config/ChargerModal.vue @@ -355,8 +355,7 @@ export default { this.$emit("updated"); this.close(); } catch (e) { - console.error(e); - alert("create failed"); + this.handleCreateError(e); } this.saving = false; }, @@ -368,7 +367,7 @@ export default { if (!this.isNew) { url += `/merge/${this.id}`; } - return await api.post(url, this.apiData); + return await api.post(url, this.apiData, { timeout: this.testTimeout }); }, async update() { if (this.testUnknown) { @@ -382,8 +381,7 @@ export default { this.$emit("updated"); this.close(); } catch (e) { - console.error(e); - alert("update failed"); + this.handleUpdateError(e); } this.saving = false; }, @@ -394,8 +392,7 @@ export default { this.$emit("updated"); this.close(); } catch (e) { - console.error(e); - alert("delete failed"); + this.handleRemoveError(e); } }, open() { diff --git a/assets/js/components/Config/MeterModal.vue b/assets/js/components/Config/MeterModal.vue index cf129fc3f..6d81158d6 100644 --- a/assets/js/components/Config/MeterModal.vue +++ b/assets/js/components/Config/MeterModal.vue @@ -367,7 +367,7 @@ export default { if (!this.isNew) { url += `/merge/${this.id}`; } - return await api.post(url, this.apiData); + return await api.post(url, this.apiData, { timeout: this.testTimeout }); }, async update() { if (this.testUnknown) { diff --git a/assets/js/components/Config/VehicleModal.vue b/assets/js/components/Config/VehicleModal.vue index 2b3d4415a..a55bb031b 100644 --- a/assets/js/components/Config/VehicleModal.vue +++ b/assets/js/components/Config/VehicleModal.vue @@ -436,7 +436,7 @@ export default { if (!this.isNew) { url += `/merge/${this.id}`; } - return await api.post(url, this.apiData); + return await api.post(url, this.apiData, { timeout: this.testTimeout }); }, async update() { if (this.testUnknown) { diff --git a/assets/js/components/Config/mixins/test.js b/assets/js/components/Config/mixins/test.js index 1667e20d8..9b1990e61 100644 --- a/assets/js/components/Config/mixins/test.js +++ b/assets/js/components/Config/mixins/test.js @@ -9,6 +9,7 @@ export default { testState: TEST_UNKNOWN, testError: null, testResult: null, + testTimeout: 15000, // 15s }; }, computed: { @@ -57,5 +58,21 @@ export default { } return false; }, + handleCreateError(e) { + this.handleError(e, "create failed"); + }, + handleUpdateError(e) { + this.handleError(e, "update failed"); + }, + handleRemoveError(e) { + this.handleError(e, "remove failed"); + }, + handleError(e, msg) { + console.error(e); + let message = msg; + const { error } = e.response.data || {}; + if (error) message += `: ${error}`; + alert(message); + }, }, }; diff --git a/cmd/setup.go b/cmd/setup.go index 69295e0f6..d44db535a 100644 --- a/cmd/setup.go +++ b/cmd/setup.go @@ -171,7 +171,7 @@ NEXT: } log := util.NewLogger("circuit-" + cc.Name) - instance, err := circuit.NewFromConfig(log, cc.Other) + instance, err := circuit.NewFromConfig(context.TODO(), log, cc.Other) if err != nil { return fmt.Errorf("cannot create circuit '%s': %w", cc.Name, err) } diff --git a/core/circuit/circuit.go b/core/circuit/circuit.go index 4741982d4..76fd821f1 100644 --- a/core/circuit/circuit.go +++ b/core/circuit/circuit.go @@ -39,7 +39,7 @@ type Circuit struct { } // NewFromConfig creates a new Circuit -func NewFromConfig(log *util.Logger, other map[string]interface{}) (api.Circuit, error) { +func NewFromConfig(ctx context.Context, log *util.Logger, other map[string]interface{}) (api.Circuit, error) { cc := struct { Title string // title ParentRef string `mapstructure:"parent"` // parent circuit reference @@ -71,12 +71,12 @@ func NewFromConfig(log *util.Logger, other map[string]interface{}) (api.Circuit, return nil, err } - circuit.getMaxPower, err = cc.GetMaxPower.FloatGetter(context.TODO()) + circuit.getMaxPower, err = cc.GetMaxPower.FloatGetter(ctx) if err != nil { return nil, err } - circuit.getMaxCurrent, err = cc.GetMaxCurrent.FloatGetter(context.TODO()) + circuit.getMaxCurrent, err = cc.GetMaxCurrent.FloatGetter(ctx) if err != nil { return nil, err } diff --git a/server/http_config_device_handler.go b/server/http_config_device_handler.go index 4de4ef190..dbcffcbb2 100644 --- a/server/http_config_device_handler.go +++ b/server/http_config_device_handler.go @@ -206,8 +206,8 @@ func deviceStatusHandler(w http.ResponseWriter, r *http.Request) { jsonResult(w, testInstance(instance)) } -func newDevice[T any](class templates.Class, req map[string]any, newFromConf newFromConfFunc[T], h config.Handler[T]) (*config.Config, error) { - instance, err := newFromConf(context.TODO(), typeTemplate, req) +func newDevice[T any](ctx context.Context, class templates.Class, req map[string]any, newFromConf newFromConfFunc[T], h config.Handler[T]) (*config.Config, error) { + instance, err := newFromConf(ctx, typeTemplate, req) if err != nil { return nil, err } @@ -217,7 +217,7 @@ func newDevice[T any](class templates.Class, req map[string]any, newFromConf new return nil, err } - return &conf, h.Add(config.NewConfigurableDevice[T](&conf, instance)) + return &conf, h.Add(config.NewConfigurableDevice(&conf, instance)) } // newDeviceHandler creates a new device by class @@ -240,28 +240,33 @@ func newDeviceHandler(w http.ResponseWriter, r *http.Request) { delete(req, "type") var conf *config.Config + ctx, cancel, done := startDeviceTimeout() switch class { case templates.Charger: - conf, err = newDevice(class, req, charger.NewFromConfig, config.Chargers()) + conf, err = newDevice(ctx, class, req, charger.NewFromConfig, config.Chargers()) case templates.Meter: - conf, err = newDevice(class, req, meter.NewFromConfig, config.Meters()) + conf, err = newDevice(ctx, class, req, meter.NewFromConfig, config.Meters()) case templates.Vehicle: - conf, err = newDevice(class, req, vehicle.NewFromConfig, config.Vehicles()) + conf, err = newDevice(ctx, class, req, vehicle.NewFromConfig, config.Vehicles()) case templates.Circuit: - conf, err = newDevice(class, req, func(_ context.Context, _ string, other map[string]interface{}) (api.Circuit, error) { - return circuit.NewFromConfig(util.NewLogger("circuit"), other) + conf, err = newDevice(ctx, class, req, func(ctx context.Context, _ string, other map[string]interface{}) (api.Circuit, error) { + return circuit.NewFromConfig(ctx, util.NewLogger("circuit"), other) }, config.Circuits()) } if err != nil { + cancel() jsonError(w, http.StatusBadRequest, err) return } + // prevent context from being cancelled + close(done) + setConfigDirty() res := struct { @@ -275,8 +280,8 @@ func newDeviceHandler(w http.ResponseWriter, r *http.Request) { jsonResult(w, res) } -func updateDevice[T any](id int, class templates.Class, conf map[string]any, newFromConf newFromConfFunc[T], h config.Handler[T]) error { - dev, instance, merged, err := deviceInstanceFromMergedConfig(id, class, conf, newFromConf, h) +func updateDevice[T any](ctx context.Context, id int, class templates.Class, conf map[string]any, newFromConf newFromConfFunc[T], h config.Handler[T]) error { + dev, instance, merged, err := deviceInstanceFromMergedConfig(ctx, id, class, conf, newFromConf, h) if err != nil { return err } @@ -314,29 +319,35 @@ func updateDeviceHandler(w http.ResponseWriter, r *http.Request) { } delete(req, "type") + ctx, cancel, done := startDeviceTimeout() + switch class { case templates.Charger: - err = updateDevice(id, class, req, charger.NewFromConfig, config.Chargers()) + err = updateDevice(ctx, id, class, req, charger.NewFromConfig, config.Chargers()) case templates.Meter: - err = updateDevice(id, class, req, meter.NewFromConfig, config.Meters()) + err = updateDevice(ctx, id, class, req, meter.NewFromConfig, config.Meters()) case templates.Vehicle: - err = updateDevice(id, class, req, vehicle.NewFromConfig, config.Vehicles()) + err = updateDevice(ctx, id, class, req, vehicle.NewFromConfig, config.Vehicles()) case templates.Circuit: - err = updateDevice(id, class, req, func(_ context.Context, _ string, other map[string]interface{}) (api.Circuit, error) { - return circuit.NewFromConfig(util.NewLogger("circuit"), other) + err = updateDevice(ctx, id, class, req, func(ctx context.Context, _ string, other map[string]interface{}) (api.Circuit, error) { + return circuit.NewFromConfig(ctx, util.NewLogger("circuit"), other) }, config.Circuits()) } setConfigDirty() if err != nil { + cancel() jsonError(w, http.StatusBadRequest, err) return } + // prevent context from being cancelled + close(done) + res := struct { ID int `json:"id"` }{ @@ -412,12 +423,12 @@ func deleteDeviceHandler(w http.ResponseWriter, r *http.Request) { jsonResult(w, res) } -func testConfig[T any](id int, class templates.Class, conf map[string]any, newFromConf newFromConfFunc[T], h config.Handler[T]) (T, error) { +func testConfig[T any](ctx context.Context, id int, class templates.Class, conf map[string]any, newFromConf newFromConfFunc[T], h config.Handler[T]) (T, error) { if id == 0 { - return newFromConf(context.TODO(), typeTemplate, conf) + return newFromConf(ctx, typeTemplate, conf) } - _, instance, _, err := deviceInstanceFromMergedConfig(id, class, conf, newFromConf, h) + _, instance, _, err := deviceInstanceFromMergedConfig(ctx, id, class, conf, newFromConf, h) return instance, err } @@ -450,25 +461,30 @@ func testConfigHandler(w http.ResponseWriter, r *http.Request) { delete(req, "type") var instance any + ctx, cancel, done := startDeviceTimeout() switch class { case templates.Charger: - instance, err = testConfig(id, class, req, charger.NewFromConfig, config.Chargers()) + instance, err = testConfig(ctx, id, class, req, charger.NewFromConfig, config.Chargers()) case templates.Meter: - instance, err = testConfig(id, class, req, meter.NewFromConfig, config.Meters()) + instance, err = testConfig(ctx, id, class, req, meter.NewFromConfig, config.Meters()) case templates.Vehicle: - instance, err = testConfig(id, class, req, vehicle.NewFromConfig, config.Vehicles()) + instance, err = testConfig(ctx, id, class, req, vehicle.NewFromConfig, config.Vehicles()) case templates.Circuit: err = api.ErrNotAvailable } if err != nil { + cancel() jsonError(w, http.StatusBadRequest, err) return } + // prevent context from being cancelled + close(done) + jsonResult(w, testInstance(instance)) } diff --git a/server/http_config_helper.go b/server/http_config_helper.go index d40d37a45..244826313 100644 --- a/server/http_config_helper.go +++ b/server/http_config_helper.go @@ -5,6 +5,7 @@ import ( "errors" "slices" "sync" + "time" "github.com/evcc-io/evcc/api" "github.com/evcc-io/evcc/util/config" @@ -89,7 +90,24 @@ func mergeMasked(class templates.Class, conf, old map[string]any) (map[string]an return res, nil } -func deviceInstanceFromMergedConfig[T any](id int, class templates.Class, conf map[string]any, newFromConf newFromConfFunc[T], h config.Handler[T]) (config.Device[T], T, map[string]any, error) { +func startDeviceTimeout() (context.Context, context.CancelFunc, chan struct{}) { + done := make(chan struct{}) + ctx, cancel := context.WithCancel(context.Background()) + + go func() { + select { + case <-time.After(10 * time.Second): + // timeout - cancel context + cancel() + case <-done: + // success + } + }() + + return ctx, cancel, done +} + +func deviceInstanceFromMergedConfig[T any](ctx context.Context, id int, class templates.Class, conf map[string]any, newFromConf newFromConfFunc[T], h config.Handler[T]) (config.Device[T], T, map[string]any, error) { var zero T dev, err := h.ByName(config.NameForID(id)) @@ -102,7 +120,7 @@ func deviceInstanceFromMergedConfig[T any](id int, class templates.Class, conf m return nil, zero, nil, err } - instance, err := newFromConf(context.TODO(), typeTemplate, merged) + instance, err := newFromConf(ctx, typeTemplate, merged) return dev, instance, merged, err }