From aae9e81047b625efafae4dbe9b1da48e62993d15 Mon Sep 17 00:00:00 2001 From: Joshua Rich Date: Thu, 13 Jun 2024 22:40:50 +1000 Subject: [PATCH] refactor(linux,dbusx): :recycle: better error handling of session bus path retrieval --- internal/linux/power/idle.go | 11 ++++++++-- internal/linux/power/laptop.go | 23 ++++++++++++++------- internal/linux/power/powerControl.go | 10 ++++++++-- internal/linux/power/screenLock.go | 22 +++++++++++++------- internal/linux/system/dbusCommand.go | 30 +++++++++++++++++----------- pkg/linux/dbusx/dbus.go | 11 +++++----- 6 files changed, 72 insertions(+), 35 deletions(-) diff --git a/internal/linux/power/idle.go b/internal/linux/power/idle.go index 8580c8527..9a4d5f822 100644 --- a/internal/linux/power/idle.go +++ b/internal/linux/power/idle.go @@ -89,6 +89,7 @@ func newIdleSensor(ctx context.Context) (*idleSensor, error) { } //nolint:cyclop,exhaustruct +//revive:disable:function-length func IdleUpdater(ctx context.Context) chan sensor.Details { sensorCh := make(chan sensor.Details) @@ -100,13 +101,19 @@ func IdleUpdater(ctx context.Context) chan sensor.Details { return sensorCh } - sessionPath := dbusx.GetSessionPath(ctx) + sessionPath, err := dbusx.GetSessionPath(ctx) + if err != nil { + log.Debug().Err(err).Msg("Cannot create idle sensor.") + close(sensorCh) + + return sensorCh + } events, err := dbusx.WatchBus(ctx, &dbusx.Watch{ Bus: dbusx.SystemBus, Names: []string{sessionIdleProp, sessionIdleTimeProp}, Interface: sessionInterface, - Path: string(sessionPath), + Path: sessionPath, }) if err != nil { log.Debug().Err(err). diff --git a/internal/linux/power/laptop.go b/internal/linux/power/laptop.go index 6ab36125a..5e1973d66 100644 --- a/internal/linux/power/laptop.go +++ b/internal/linux/power/laptop.go @@ -8,6 +8,7 @@ package power import ( "context" + "fmt" "path/filepath" "slices" @@ -85,17 +86,17 @@ func newLaptopEvent(prop string, state bool) *laptopSensor { return sensorEvent } -type laptopWorker struct{} +type laptopWorker struct { + sessionPath string +} //nolint:exhaustruct -func (w *laptopWorker) Setup(ctx context.Context) *dbusx.Watch { - sessionPath := dbusx.GetSessionPath(ctx) - +func (w *laptopWorker) Setup(_ context.Context) *dbusx.Watch { return &dbusx.Watch{ Bus: dbusx.SystemBus, Names: []string{dbusx.PropChangedSignal}, Interface: managerInterface, - Path: string(sessionPath), + Path: w.sessionPath, } } @@ -160,16 +161,24 @@ func (w *laptopWorker) Sensors(ctx context.Context) ([]sensor.Details, error) { return sensors, nil } -func NewLaptopWorker(_ context.Context) (*linux.SensorWorker, error) { +func NewLaptopWorker(ctx context.Context) (*linux.SensorWorker, error) { // Don't run this worker if we are not running on a laptop. if linux.Chassis() != "laptop" { return nil, linux.ErrUnsupportedHardware } + // If we can't get a session path, we can't run. + sessionPath, err := dbusx.GetSessionPath(ctx) + if err != nil { + return nil, fmt.Errorf("could not create laptop worker: %w", err) + } + return &linux.SensorWorker{ WorkerName: "Laptop State Sensors", WorkerDesc: "Sensors for laptop lid, dock and external power states.", - Value: &laptopWorker{}, + Value: &laptopWorker{ + sessionPath: sessionPath, + }, }, nil } diff --git a/internal/linux/power/powerControl.go b/internal/linux/power/powerControl.go index f0a50a040..98c4119e1 100644 --- a/internal/linux/power/powerControl.go +++ b/internal/linux/power/powerControl.go @@ -74,13 +74,19 @@ var commands = map[string]commandConfig{ func NewPowerControl(ctx context.Context) []*mqtthass.ButtonEntity { entities := make([]*mqtthass.ButtonEntity, 0, len(commands)) device := linux.MQTTDevice() - sessionPath := dbusx.GetSessionPath(ctx) + + sessionPath, err := dbusx.GetSessionPath(ctx) + if err != nil { + log.Warn().Err(err).Msg("Cannot create entity.") + + return nil + } for cmdName, cmdInfo := range commands { var callback func(p *paho.Publish) if cmdInfo.path == "" { callback = func(_ *paho.Publish) { - err := dbusx.Call(ctx, dbusx.SystemBus, string(sessionPath), loginBaseInterface, cmdInfo.method) + err := dbusx.Call(ctx, dbusx.SystemBus, sessionPath, loginBaseInterface, cmdInfo.method) if err != nil { log.Warn().Err(err).Msgf("Could not %s session.", cmdInfo.name) } diff --git a/internal/linux/power/screenLock.go b/internal/linux/power/screenLock.go index bd415cd28..50bf79922 100644 --- a/internal/linux/power/screenLock.go +++ b/internal/linux/power/screenLock.go @@ -8,6 +8,7 @@ package power import ( "context" + "fmt" "github.com/rs/zerolog/log" @@ -45,17 +46,17 @@ func newScreenlockEvent(value bool) *screenlockSensor { } } -type screenLockWorker struct{} +type screenLockWorker struct { + sessionPath string +} //nolint:exhaustruct -func (w *screenLockWorker) Setup(ctx context.Context) *dbusx.Watch { - sessionPath := dbusx.GetSessionPath(ctx) - +func (w *screenLockWorker) Setup(_ context.Context) *dbusx.Watch { return &dbusx.Watch{ Bus: dbusx.SystemBus, Names: []string{sessionLockSignal, sessionUnlockSignal, sessionLockedProp}, Interface: sessionInterface, - Path: string(sessionPath), + Path: w.sessionPath, } } @@ -100,11 +101,18 @@ func (w *screenLockWorker) Sensors(_ context.Context) ([]sensor.Details, error) return nil, linux.ErrUnimplemented } -func NewScreenLockWorker(_ context.Context) (*linux.SensorWorker, error) { +func NewScreenLockWorker(ctx context.Context) (*linux.SensorWorker, error) { + sessionPath, err := dbusx.GetSessionPath(ctx) + if err != nil { + return nil, fmt.Errorf("could not create screen lock worker: %w", err) + } + return &linux.SensorWorker{ WorkerName: "Screen Lock Sensor", WorkerDesc: "Sensor to track whether the screen is currently locked.", - Value: &screenLockWorker{}, + Value: &screenLockWorker{ + sessionPath: sessionPath, + }, }, nil } diff --git a/internal/linux/system/dbusCommand.go b/internal/linux/system/dbusCommand.go index cefc6ec0e..894aec257 100644 --- a/internal/linux/system/dbusCommand.go +++ b/internal/linux/system/dbusCommand.go @@ -10,7 +10,6 @@ import ( "encoding/json" "github.com/eclipse/paho.golang/paho" - "github.com/godbus/dbus/v5" mqttapi "github.com/joshuar/go-hass-anything/v9/pkg/mqtt" "github.com/rs/zerolog/log" @@ -22,12 +21,12 @@ const ( ) type dbusCommandMsg struct { - Bus string `json:"bus"` - Destination string `json:"destination"` - Path dbus.ObjectPath `json:"path"` - Method string `json:"method"` - Args []any `json:"args"` - UseSessionPath bool `json:"use_session_path"` + Bus string `json:"bus"` + Destination string `json:"destination"` + Path string `json:"path"` + Method string `json:"method"` + Args []any `json:"args"` + UseSessionPath bool `json:"use_session_path"` } func NewDBusCommandSubscription(ctx context.Context) *mqttapi.Subscription { @@ -36,13 +35,20 @@ func NewDBusCommandSubscription(ctx context.Context) *mqttapi.Subscription { var dbusMsg dbusCommandMsg if err := json.Unmarshal(p.Payload, &dbusMsg); err != nil { - log.Warn().Err(err).Msg("could not unmarshal dbus MQTT message") + log.Warn().Err(err).Msg("Could not unmarshal dbus MQTT message") return } if dbusMsg.UseSessionPath { - dbusMsg.Path = dbusx.GetSessionPath(ctx) + var err error + + dbusMsg.Path, err = dbusx.GetSessionPath(ctx) + if err != nil { + log.Warn().Err(err).Msg("Could not determine session path.") + + return + } } dbusType, ok := dbusx.DbusTypeMap[dbusMsg.Bus] @@ -55,16 +61,16 @@ func NewDBusCommandSubscription(ctx context.Context) *mqttapi.Subscription { log.Info(). Str("bus", dbusMsg.Bus). Str("destination", dbusMsg.Destination). - Str("path", string(dbusMsg.Path)). + Str("path", dbusMsg.Path). Str("method", dbusMsg.Method). Msg("Dispatching D-Bus command to MQTT.") - err := dbusx.Call(ctx, dbusType, string(dbusMsg.Path), dbusMsg.Destination, dbusMsg.Method, dbusMsg.Args...) + err := dbusx.Call(ctx, dbusType, dbusMsg.Path, dbusMsg.Destination, dbusMsg.Method, dbusMsg.Args...) if err != nil { log.Warn().Err(err). Str("bus", dbusMsg.Bus). Str("destination", dbusMsg.Destination). - Str("path", string(dbusMsg.Path)). + Str("path", dbusMsg.Path). Str("method", dbusMsg.Method). Msg("Error dispatching D-Bus command.") } diff --git a/pkg/linux/dbusx/dbus.go b/pkg/linux/dbusx/dbus.go index 017f06cf3..92e766fa4 100644 --- a/pkg/linux/dbusx/dbus.go +++ b/pkg/linux/dbusx/dbus.go @@ -40,6 +40,7 @@ var ( ErrNotValChanged = errors.New("signal contents do not appear to represent a value change") ErrParseNewVal = errors.New("could not parse new value") ErrParseOldVal = errors.New("could not parse old value") + ErrNoSessionPath = errors.New("could not determine session path") ) var DbusTypeMap = map[string]dbusType{ @@ -317,26 +318,26 @@ func WatchBus(ctx context.Context, conditions *Watch) (chan Trigger, error) { return outCh, nil } -func GetSessionPath(ctx context.Context) dbus.ObjectPath { +func GetSessionPath(ctx context.Context) (string, error) { usr, err := user.Current() if err != nil { - return "" + return "", fmt.Errorf("unable to determine user: %w", err) } sessions, err := GetData[[][]any](ctx, SystemBus, loginBasePath, loginBaseInterface, listSessionsMethod) if err != nil { - return "" + return "", fmt.Errorf("unable to retrieve session path: %w", err) } for _, s := range sessions { if thisUser, ok := s[2].(string); ok && thisUser == usr.Username { if p, ok := s[4].(dbus.ObjectPath); ok { - return p + return string(p), nil } } } - return "" + return "", ErrNoSessionPath } // ParsePropertiesChanged treats the given signal body as matching the canonical