From 313f05fcb6f832006f7fe26720f2cecd1d6110a9 Mon Sep 17 00:00:00 2001 From: Keepers Date: Mon, 6 Feb 2023 17:44:35 -0700 Subject: [PATCH] add clues, fault to small /internal pkgs (#2336) --- src/internal/common/errors.go | 2 ++ src/internal/common/slices.go | 1 + src/internal/common/time.go | 14 ++++++++----- src/internal/events/events.go | 4 ++-- src/internal/streamstore/streamstore.go | 27 +++++++++++++------------ src/pkg/backup/details/details.go | 6 +++--- src/pkg/path/onedrive.go | 2 +- 7 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/internal/common/errors.go b/src/internal/common/errors.go index 2eb3ba891..82dea7b38 100644 --- a/src/internal/common/errors.go +++ b/src/internal/common/errors.go @@ -5,6 +5,8 @@ import ( "io" ) +// TODO: Remove in favor of clues.Stack() + // Err provides boiler-plate functions that other types of errors can use // if they wish to be compared with `errors.As()`. This struct ensures that // stack traces are printed when requested (if present) and that Err diff --git a/src/internal/common/slices.go b/src/internal/common/slices.go index 7600400e9..73c7c951e 100644 --- a/src/internal/common/slices.go +++ b/src/internal/common/slices.go @@ -1,5 +1,6 @@ package common +// TODO: can be replaced with slices.Contains() func ContainsString(super []string, sub string) bool { for _, s := range super { if s == sub { diff --git a/src/internal/common/time.go b/src/internal/common/time.go index de430cf8b..f747bef4c 100644 --- a/src/internal/common/time.go +++ b/src/internal/common/time.go @@ -4,6 +4,7 @@ import ( "regexp" "time" + "github.com/alcionai/clues" "github.com/pkg/errors" ) @@ -85,7 +86,10 @@ var ( } ) -var ErrNoTimeString = errors.New("no substring contains a known time format") +var ( + ErrNoTimeString = errors.New("no substring contains a known time format") + errParsingStringToTime = errors.New("parsing string as time.Time") +) // Now produces the current time as a string in the standard format. func Now() string { @@ -132,7 +136,7 @@ func FormatLegacyTime(t time.Time) string { // the provided string. Always returns a UTC timezone value. func ParseTime(s string) (time.Time, error) { if len(s) == 0 { - return time.Time{}, errors.New("cannot interpret an empty string as time.Time") + return time.Time{}, clues.Stack(errParsingStringToTime, errors.New("empty string")) } for _, form := range formats { @@ -142,14 +146,14 @@ func ParseTime(s string) (time.Time, error) { } } - return time.Time{}, errors.New("unable to parse time string: " + s) + return time.Time{}, clues.Stack(errParsingStringToTime, errors.New(s)) } // ExtractTime greedily retrieves a timestamp substring from the provided string. // returns ErrNoTimeString if no match is found. func ExtractTime(s string) (time.Time, error) { if len(s) == 0 { - return time.Time{}, errors.New("cannot extract time.Time from an empty string") + return time.Time{}, clues.Stack(errParsingStringToTime, errors.New("empty string")) } for _, re := range regexes { @@ -159,5 +163,5 @@ func ExtractTime(s string) (time.Time, error) { } } - return time.Time{}, errors.Wrap(ErrNoTimeString, s) + return time.Time{}, clues.Stack(ErrNoTimeString, errors.New(s)) } diff --git a/src/internal/events/events.go b/src/internal/events/events.go index 58b57f63f..d91c734c9 100644 --- a/src/internal/events/events.go +++ b/src/internal/events/events.go @@ -7,7 +7,7 @@ import ( "os" "time" - "github.com/pkg/errors" + "github.com/alcionai/clues" analytics "github.com/rudderlabs/analytics-go" "github.com/alcionai/corso/src/internal/version" @@ -93,7 +93,7 @@ func NewBus(ctx context.Context, s storage.Storage, tenID string, opts control.O }) if err != nil { - return Bus{}, errors.Wrap(err, "configuring event bus") + return Bus{}, clues.Wrap(err, "configuring event bus").WithClues(ctx) } } diff --git a/src/internal/streamstore/streamstore.go b/src/internal/streamstore/streamstore.go index fb5dfccd9..d2fd4b654 100644 --- a/src/internal/streamstore/streamstore.go +++ b/src/internal/streamstore/streamstore.go @@ -8,6 +8,7 @@ import ( "encoding/json" "io" + "github.com/alcionai/clues" "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/data" @@ -52,17 +53,16 @@ func (ss *streamStore) WriteBackupDetails( ss.tenant, collectionPurposeDetails, ss.service, - false, - ) + false) if err != nil { - return "", err + return "", clues.Stack(err).WithClues(ctx) } // TODO: We could use an io.Pipe here to avoid a double copy but that // makes error handling a bit complicated dbytes, err := json.Marshal(backupDetails) if err != nil { - return "", errors.Wrap(err, "marshalling backup details") + return "", clues.Wrap(err, "marshalling backup details").WithClues(ctx) } dc := &streamCollection{ @@ -79,10 +79,9 @@ func (ss *streamStore) WriteBackupDetails( []data.Collection{dc}, nil, nil, - false, - ) + false) if err != nil { - return "", nil + return "", errors.Wrap(err, "storing details in repository") } return backupStats.SnapshotID, nil @@ -104,7 +103,7 @@ func (ss *streamStore) ReadBackupDetails( true, ) if err != nil { - return nil, err + return nil, clues.Stack(err).WithClues(ctx) } var bc stats.ByteCounter @@ -116,7 +115,9 @@ func (ss *streamStore) ReadBackupDetails( // Expect only 1 data collection if len(dcs) != 1 { - return nil, errors.Errorf("expected 1 details data collection: %d", len(dcs)) + return nil, clues.New("greater than 1 details data collection found"). + WithClues(ctx). + With("collection_count", len(dcs)) } dc := dcs[0] @@ -129,12 +130,12 @@ func (ss *streamStore) ReadBackupDetails( for { select { case <-ctx.Done(): - return nil, errors.New("context cancelled waiting for backup details data") + return nil, clues.New("context cancelled waiting for backup details data").WithClues(ctx) case itemData, ok := <-items: if !ok { if !found { - return nil, errors.New("no backup details found") + return nil, clues.New("no backup details found").WithClues(ctx) } return &d, nil @@ -142,7 +143,7 @@ func (ss *streamStore) ReadBackupDetails( err := json.NewDecoder(itemData.ToReader()).Decode(&d) if err != nil { - return nil, errors.Wrap(err, "failed to decode details data from repository") + return nil, clues.Wrap(err, "decoding details data").WithClues(ctx) } found = true @@ -157,7 +158,7 @@ func (ss *streamStore) DeleteBackupDetails( ) error { err := ss.kw.DeleteSnapshot(ctx, detailsID) if err != nil { - return errors.Wrap(err, "deleting backup details failed") + return errors.Wrap(err, "deleting backup details") } return nil diff --git a/src/pkg/backup/details/details.go b/src/pkg/backup/details/details.go index f244c72c9..bb392c223 100644 --- a/src/pkg/backup/details/details.go +++ b/src/pkg/backup/details/details.go @@ -6,8 +6,8 @@ import ( "sync" "time" + "github.com/alcionai/clues" "github.com/dustin/go-humanize" - "github.com/pkg/errors" "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/internal/common" @@ -509,7 +509,7 @@ func (i SharePointInfo) Values() []string { func (i *SharePointInfo) UpdateParentPath(newPath path.Path) error { newParent, err := path.GetDriveFolderPath(newPath) if err != nil { - return errors.Wrapf(err, "making sharepoint path from %s", newPath) + return clues.Wrap(err, "making sharePoint path").With("path", newPath) } i.ParentPath = newParent @@ -551,7 +551,7 @@ func (i OneDriveInfo) Values() []string { func (i *OneDriveInfo) UpdateParentPath(newPath path.Path) error { newParent, err := path.GetDriveFolderPath(newPath) if err != nil { - return errors.Wrapf(err, "making drive path from %s", newPath) + return clues.Wrap(err, "making oneDrive path").With("path", newPath) } i.ParentPath = newParent diff --git a/src/pkg/path/onedrive.go b/src/pkg/path/onedrive.go index 86d40c887..35738289c 100644 --- a/src/pkg/path/onedrive.go +++ b/src/pkg/path/onedrive.go @@ -20,7 +20,7 @@ func ToOneDrivePath(p Path) (*DrivePath, error) { if len(folders) < 3 { return nil, clues. New("folder path doesn't match expected format for OneDrive items"). - With("folders", p.Folder()) + With("path_folders", p.Folder()) } return &DrivePath{DriveID: folders[1], Folders: folders[3:]}, nil