From d13b26322216971973b30617a03e9525950d2828 Mon Sep 17 00:00:00 2001 From: brent s Date: Sun, 9 Jan 2022 15:39:37 -0500 Subject: [PATCH] wrap errors Implement error wrapping so we catch all errors into a single error. --- README.adoc | 2 ++ collection_funcs.go | 28 +++++++++++++++------- doc.go | 4 ++++ funcs.go | 21 +++++++++++----- go.mod | 1 + go.sum | 5 ++++ multierr_funcs.go | 58 --------------------------------------------- service_funcs.go | 50 +++++++++++++++++++++++--------------- types.go | 24 +++++++------------ 9 files changed, 86 insertions(+), 107 deletions(-) delete mode 100644 multierr_funcs.go diff --git a/README.adoc b/README.adoc index 39218d2..299cad9 100644 --- a/README.adoc +++ b/README.adoc @@ -224,6 +224,8 @@ func main() { } ---- +Note that many functions/methods may return a https://pkg.go.dev/r00t2.io/goutils/multierr#MultiError[`(r00t2.io/goutils/)multierr.MultiError`^], which you may attempt to typeswitch to receive the original errors in their native error format. The functions/methods which may return a MultiError are noted as such in their individual documentation. + == Library Hacking === Reference diff --git a/collection_funcs.go b/collection_funcs.go index 21aa087..8b0d723 100644 --- a/collection_funcs.go +++ b/collection_funcs.go @@ -4,6 +4,7 @@ import ( "time" "github.com/godbus/dbus/v5" + "r00t2.io/goutils/multierr" ) /* @@ -146,13 +147,16 @@ func (c *Collection) Delete() (err error) { return } -// Items returns a slice of Item pointers in the Collection. +/* + Items returns a slice of Item pointers in the Collection. + err MAY be a *multierr.MultiError. +*/ func (c *Collection) Items() (items []*Item, err error) { var paths []dbus.ObjectPath var item *Item var variant dbus.Variant - var errs []error = make([]error, 0) + var errs *multierr.MultiError = multierr.NewMultiError() if variant, err = c.Dbus.GetProperty(DbusCollectionItems); err != nil { return @@ -165,13 +169,16 @@ func (c *Collection) Items() (items []*Item, err error) { for _, path := range paths { item = nil if item, err = NewItem(c, path); err != nil { - errs = append(errs, err) + errs.AddError(err) err = nil continue } items = append(items, item) } - err = NewErrors(err) + + if !errs.IsEmpty() { + err = errs + } return } @@ -248,18 +255,20 @@ func (c *Collection) Relabel(newLabel string) (err error) { } /* - SearchItems searches a Collection for a matching profile string. + SearchItems searches a Collection for a matching "profile" string. It's mostly a carry-over from go-libsecret, and is here for convenience. IT MAY BE REMOVED IN THE FUTURE. I promise it's not useful for any other implementation/storage of SecretService whatsoever. + err MAY be a *multierr.MultiError. + Deprecated: Use Service.SearchItems instead. */ func (c *Collection) SearchItems(profile string) (items []*Item, err error) { var call *dbus.Call var paths []dbus.ObjectPath - var errs []error = make([]error, 0) + var errs *multierr.MultiError = multierr.NewMultiError() var attrs map[string]string = make(map[string]string, 0) var item *Item @@ -280,13 +289,16 @@ func (c *Collection) SearchItems(profile string) (items []*Item, err error) { for _, path := range paths { item = nil if item, err = NewItem(c, path); err != nil { - errs = append(errs, err) + errs.AddError(err) err = nil continue } items = append(items, item) } - err = NewErrors(err) + + if !errs.IsEmpty() { + err = errs + } return } diff --git a/doc.go b/doc.go index 31915c6..8f12d08 100644 --- a/doc.go +++ b/doc.go @@ -84,5 +84,9 @@ Usage Full documentation can be found via inline documentation. Additionally, use either https://pkg.go.dev/r00t2.io/gosecret or https://pkg.go.dev/golang.org/x/tools/cmd/godoc (or `go doc`) in the source root. + +Note that many functions/methods may return a (r00t2.io/goutils/)multierr.MultiError (https://pkg.go.dev/r00t2.io/goutils/multierr#MultiError), +which you may attempt to typeswitch back to a *multierr.MultiErr to receive the original errors in their native error format (MultiError.Errors). +The functions/methods which may return a MultiError are noted as such in their individual documentation. */ package gosecret diff --git a/funcs.go b/funcs.go index f0ebd6a..0c86e52 100644 --- a/funcs.go +++ b/funcs.go @@ -4,6 +4,7 @@ import ( `strings` `github.com/godbus/dbus/v5` + `r00t2.io/goutils/multierr` ) // isPrompt returns a boolean that is true if path is/requires a prompt(ed path) and false if it is/does not. @@ -63,19 +64,27 @@ func pathIsValid(path interface{}) (ok bool, err error) { /* validConnPath condenses the checks for connIsValid and pathIsValid into one func due to how frequently this check is done. - err is a MultiError, which can be treated as an error.error. (See https://pkg.go.dev/builtin#error) + + If err is not nil, it IS a *multierr.MultiError. */ func validConnPath(conn *dbus.Conn, path interface{}) (cr *ConnPathCheckResult, err error) { - var connErr error - var pathErr error + var errs *multierr.MultiError = multierr.NewMultiError() cr = new(ConnPathCheckResult) - cr.ConnOK, connErr = connIsValid(conn) - cr.PathOK, pathErr = pathIsValid(path) + if cr.ConnOK, err = connIsValid(conn); err != nil { + errs.AddError(err) + err = nil + } + if cr.PathOK, err = pathIsValid(path); err != nil { + errs.AddError(err) + err = nil + } - err = NewErrors(connErr, pathErr) + if !errs.IsEmpty() { + err = errs + } return } diff --git a/go.mod b/go.mod index 0325efa..06930dd 100644 --- a/go.mod +++ b/go.mod @@ -5,4 +5,5 @@ go 1.17 require ( github.com/godbus/dbus/v5 v5.0.6 github.com/google/uuid v1.3.0 + r00t2.io/goutils v1.1.2 ) diff --git a/go.sum b/go.sum index 4c667d8..01d5029 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,9 @@ +github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/godbus/dbus/v5 v5.0.6 h1:mkgN1ofwASrYnJ5W6U/BxG15eXXXjirgZc7CLqkcaro= github.com/godbus/dbus/v5 v5.0.6/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA= github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +r00t2.io/goutils v1.1.2 h1:zOOqNHQ/HpJVggV5NTXBcd7FQtBP2C/sMLkHw3YvBzU= +r00t2.io/goutils v1.1.2/go.mod h1:9ObJI9S71wDLTOahwoOPs19DhZVYrOh4LEHmQ8SW4Lk= +r00t2.io/sysutils v1.1.1/go.mod h1:Wlfi1rrJpoKBOjWiYM9rw2FaiZqraD6VpXyiHgoDo/o= diff --git a/multierr_funcs.go b/multierr_funcs.go deleted file mode 100644 index 2bd86c4..0000000 --- a/multierr_funcs.go +++ /dev/null @@ -1,58 +0,0 @@ -package gosecret - -import ( - "fmt" -) - -/* - NewErrors returns a new MultiError based on a slice of error.Error (errs). - Any nil errors are trimmed. If there are no actual errors after trimming, err will be nil. -*/ -func NewErrors(errs ...error) (err error) { - - if errs == nil || len(errs) == 0 { - return - } - - var realErrs []error = make([]error, 0) - - for _, e := range errs { - if e == nil { - continue - } - realErrs = append(realErrs, e) - } - - if len(realErrs) == 0 { - return - } - - err = &MultiError{ - Errors: realErrs, - ErrorSep: "\n", - } - - return -} - -// Error makes a MultiError conform to the error interface. -func (e *MultiError) Error() (errStr string) { - - var numErrs int - - if e == nil || len(e.Errors) == 0 { - return - } else { - numErrs = len(e.Errors) - } - - for idx, err := range e.Errors { - if (idx + 1) < numErrs { - errStr += fmt.Sprintf("%v%v", err.Error(), e.ErrorSep) - } else { - errStr += err.Error() - } - } - - return -} diff --git a/service_funcs.go b/service_funcs.go index 0472506..e7cbe6f 100644 --- a/service_funcs.go +++ b/service_funcs.go @@ -8,6 +8,7 @@ import ( "time" "github.com/godbus/dbus/v5" + `r00t2.io/goutils/multierr` ) // NewService returns a pointer to a new Service connection. @@ -49,13 +50,17 @@ func (s *Service) Close() (err error) { return } -// Collections returns a slice of Collection items accessible to this Service. +/* + Collections returns a slice of Collection items accessible to this Service. + + err MAY be a *multierr.MultiError. +*/ func (s *Service) Collections() (collections []*Collection, err error) { var paths []dbus.ObjectPath var variant dbus.Variant var coll *Collection - var errs []error = make([]error, 0) + var errs *multierr.MultiError = multierr.NewMultiError() if variant, err = s.Dbus.GetProperty(DbusServiceCollections); err != nil { return @@ -68,14 +73,16 @@ func (s *Service) Collections() (collections []*Collection, err error) { for _, path := range paths { coll = nil if coll, err = NewCollection(s, path); err != nil { - // return - errs = append(errs, err) + errs.AddError(err) err = nil continue } collections = append(collections, coll) } - err = NewErrors(err) + + if !errs.IsEmpty() { + err = errs + } return } @@ -136,10 +143,12 @@ func (s *Service) CreateCollection(label string) (collection *Collection, err er /* GetCollection returns a single Collection based on the name (name can also be an alias). It's a helper function that avoids needing to make multiple calls in user code. + + err MAY be a *multierr.MultiError. */ func (s *Service) GetCollection(name string) (c *Collection, err error) { - var errs []error = make([]error, 0) + var errs *multierr.MultiError = multierr.NewMultiError() var colls []*Collection var pathName string @@ -160,7 +169,7 @@ func (s *Service) GetCollection(name string) (c *Collection, err error) { } for _, i := range colls { if pathName, err = NameFromPath(i.Dbus.Path()); err != nil { - errs = append(errs, err) + errs.AddError(err) err = nil continue } @@ -179,9 +188,8 @@ func (s *Service) GetCollection(name string) (c *Collection, err error) { } // Couldn't find it by the given name. - if errs != nil || len(errs) > 0 { - errs = append([]error{ErrDoesNotExist}, errs...) - err = NewErrors(errs...) + if !errs.IsEmpty() { + err = errs } else { err = ErrDoesNotExist } @@ -384,6 +392,8 @@ func (s *Service) RemoveAlias(alias string) (err error) { /* SearchItems searches all Collection objects and returns all matches based on the map of attributes. + + err MAY be a *multierr.MultiError. */ func (s *Service) SearchItems(attributes map[string]string) (unlockedItems []*Item, lockedItems []*Item, err error) { @@ -396,7 +406,7 @@ func (s *Service) SearchItems(attributes map[string]string) (unlockedItems []*It var c *Collection var cPath dbus.ObjectPath var item *Item - var errs []error = make([]error, 0) + var errs *multierr.MultiError = multierr.NewMultiError() if attributes == nil || len(attributes) == 0 { err = ErrMissingAttrs @@ -431,16 +441,17 @@ func (s *Service) SearchItems(attributes map[string]string) (unlockedItems []*It cPath = dbus.ObjectPath(filepath.Dir(string(i))) if c, ok = collections[cPath]; !ok { - errs = append(errs, errors.New(fmt.Sprintf( + errs.AddError(errors.New(fmt.Sprintf( "could not find matching Collection for locked item %v", string(i), ))) continue } if item, err = NewItem(c, i); err != nil { - errs = append(errs, errors.New(fmt.Sprintf( - "could not create Item for locked item %v", string(i), + errs.AddError(errors.New(fmt.Sprintf( + "could not create Item for locked item %v; error follows", string(i), ))) + errs.AddError(err) err = nil continue } @@ -454,24 +465,25 @@ func (s *Service) SearchItems(attributes map[string]string) (unlockedItems []*It cPath = dbus.ObjectPath(filepath.Dir(string(i))) if c, ok = collections[cPath]; !ok { - errs = append(errs, errors.New(fmt.Sprintf( + errs.AddError(errors.New(fmt.Sprintf( "could not find matching Collection for unlocked item %v", string(i), ))) continue } if item, err = NewItem(c, i); err != nil { - errs = append(errs, errors.New(fmt.Sprintf( - "could not create Item for unlocked item %v", string(i), + errs.AddError(errors.New(fmt.Sprintf( + "could not create Item for unlocked item %v; error follows", string(i), ))) + errs.AddError(err) err = nil continue } unlockedItems = append(unlockedItems, item) } - if errs != nil && len(errs) > 0 { - err = NewErrors(errs...) + if !errs.IsEmpty() { + err = errs } return diff --git a/types.go b/types.go index 8d81b2b..2560b82 100644 --- a/types.go +++ b/types.go @@ -6,16 +6,6 @@ import ( "github.com/godbus/dbus/v5" ) -/* - MultiError is a type of error.Error that can contain multiple error.Errors. Confused? Don't worry about it. -*/ -type MultiError struct { - // Errors is a slice of errors to combine/concatenate when .Error() is called. - Errors []error `json:"errors"` - // ErrorSep is a string to use to separate errors for .Error(). The default is "\n". - ErrorSep string `json:"separator"` -} - /* SecretServiceError is a translated error from SecretService API. See https://developer-old.gnome.org/libsecret/unstable/libsecret-SecretError.html#SecretError and @@ -75,14 +65,16 @@ type Service struct { // IsLocked indicates if the Service is locked or not. Status updated by Service.Locked. IsLocked bool `json:"locked"` /* - Legacy indicates that this SecretService implementation - breaks current spec by implementing the legacy/obsolete draft spec rather than current libsecret spec + Legacy indicates that this SecretService implementation breaks current spec + by implementing the legacy/obsolete draft spec rather than current libsecret spec for the Dbus API. - If you're using SecretService with KeePassXC, for instance, or a much older version of Gnome-Keyring *before* libsecret integration(?), - or if you are getting strange errors when performing a Service.SearchItems, you probably need to enable this field on the Service returned - by NewService. The coverage of this field may expand in the future, but currently it only prevents the (non-existent, in legacy spec) - Type property from being read or written on Items during NewItem and Collection.CreateItem. + If you're using SecretService with KeePassXC, for instance, or a much older version + of Gnome-Keyring *before* libsecret integration(?), or if you are getting strange errors + when performing a Service.SearchItems, you probably need to enable this field on the + Service returned by NewService. The coverage of this field may expand in the future, but + currently it only prevents/suppresses the (non-existent, in legacy spec) Type property + from being read or written on Items during NewItem and Collection.CreateItem respectively. */ Legacy bool `json:"is_legacy"` }