diff --git a/src/internal/connector/discovery/api/users.go b/src/internal/connector/discovery/api/users.go index b1a37b683..f59737168 100644 --- a/src/internal/connector/discovery/api/users.go +++ b/src/internal/connector/discovery/api/users.go @@ -114,7 +114,7 @@ func (c Users) GetAll(ctx context.Context, errs *fault.Errors) ([]models.Userabl us := make([]models.Userable, 0) iterator := func(item any) bool { - if errs.Failed() { + if errs.Err() != nil { return false } diff --git a/src/internal/connector/exchange/contact_folder_cache.go b/src/internal/connector/exchange/contact_folder_cache.go index 5ba03172c..e7ef18292 100644 --- a/src/internal/connector/exchange/contact_folder_cache.go +++ b/src/internal/connector/exchange/contact_folder_cache.go @@ -3,10 +3,10 @@ package exchange import ( "context" + "github.com/alcionai/clues" "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/connector/graph" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/pkg/path" ) @@ -26,7 +26,7 @@ func (cfc *contactFolderCache) populateContactRoot( ) error { f, err := cfc.getter.GetContainerByID(ctx, cfc.userID, directoryID) if err != nil { - return support.ConnectorStackErrorTraceWrap(err, "fetching root folder") + return clues.Wrap(err, "fetching root folder") } temp := graph.NewCacheFolder( @@ -34,7 +34,7 @@ func (cfc *contactFolderCache) populateContactRoot( path.Builder{}.Append(baseContainerPath...), // storage path path.Builder{}.Append(baseContainerPath...)) // display location if err := cfc.addFolder(temp); err != nil { - return errors.Wrap(err, "adding resolver dir") + return clues.Wrap(err, "adding resolver dir").WithClues(ctx) } return nil @@ -71,7 +71,7 @@ func (cfc *contactFolderCache) init( baseContainerPath []string, ) error { if len(baseNode) == 0 { - return errors.New("m365 folderID required for base folder") + return clues.New("m365 folderID required for base folder").WithClues(ctx) } if cfc.containerResolver == nil { diff --git a/src/internal/connector/exchange/container_resolver.go b/src/internal/connector/exchange/container_resolver.go index 248f7cd4f..3270940ed 100644 --- a/src/internal/connector/exchange/container_resolver.go +++ b/src/internal/connector/exchange/container_resolver.go @@ -3,6 +3,7 @@ package exchange import ( "context" + "github.com/alcionai/clues" "github.com/hashicorp/go-multierror" "github.com/pkg/errors" @@ -62,13 +63,15 @@ func (cr *containerResolver) idToPath( depth int, useIDInPath bool, ) (*path.Builder, *path.Builder, error) { + ctx = clues.Add(ctx, "container_id", folderID) + if depth >= maxIterations { - return nil, nil, errors.New("path contains cycle or is too tall") + return nil, nil, clues.New("path contains cycle or is too tall").WithClues(ctx) } c, ok := cr.cache[folderID] if !ok { - return nil, nil, errors.Errorf("folder %s not cached", folderID) + return nil, nil, clues.New("folder not cached").WithClues(ctx) } p := c.Path() @@ -164,7 +167,7 @@ func (cr *containerResolver) AddToCache( Container: f, } if err := cr.addFolder(temp); err != nil { - return errors.Wrap(err, "adding cache folder") + return clues.Wrap(err, "adding cache folder").WithClues(ctx) } // Populate the path for this entry so calls to PathInCache succeed no matter diff --git a/src/internal/connector/exchange/data_collections.go b/src/internal/connector/exchange/data_collections.go index 7fd180281..fd41418f8 100644 --- a/src/internal/connector/exchange/data_collections.go +++ b/src/internal/connector/exchange/data_collections.go @@ -185,7 +185,7 @@ func DataCollections( } for _, scope := range eb.Scopes() { - if errs.Failed() { + if errs.Err() != nil { break } @@ -196,7 +196,8 @@ func DataCollections( scope, cdps[scope.Category().PathType()], ctrlOpts, - su) + su, + errs) if err != nil { errs.Add(err) continue @@ -217,7 +218,7 @@ func getterByType(ac api.Client, category path.CategoryType) (addedAndRemovedIte case path.ContactsCategory: return ac.Contacts(), nil default: - return nil, clues.Wrap(clues.New(category.String()), "category not supported") + return nil, clues.New("no api client registered for category") } } @@ -232,6 +233,7 @@ func createCollections( dps DeltaPaths, ctrlOpts control.Options, su support.StatusUpdater, + errs *fault.Errors, ) ([]data.BackupCollection, error) { var ( allCollections = make([]data.BackupCollection, 0) @@ -239,6 +241,8 @@ func createCollections( category = scope.Category().PathType() ) + ctx = clues.Add(ctx, "category", category) + getter, err := getterByType(ac, category) if err != nil { return nil, clues.Stack(err).WithClues(ctx) @@ -274,7 +278,8 @@ func createCollections( resolver, scope, dps, - ctrlOpts) + ctrlOpts, + errs) if err != nil { return nil, errors.Wrap(err, "filling collections") } diff --git a/src/internal/connector/exchange/data_collections_test.go b/src/internal/connector/exchange/data_collections_test.go index d337b9d32..8eea6bcba 100644 --- a/src/internal/connector/exchange/data_collections_test.go +++ b/src/internal/connector/exchange/data_collections_test.go @@ -15,6 +15,7 @@ import ( "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/control" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -267,7 +268,8 @@ func (suite *DataCollectionsIntegrationSuite) TestMailFetch() { test.scope, DeltaPaths{}, control.Options{}, - func(status *support.ConnectorOperationStatus) {}) + func(status *support.ConnectorOperationStatus) {}, + fault.New(true)) require.NoError(t, err) for _, c := range collections { @@ -334,7 +336,8 @@ func (suite *DataCollectionsIntegrationSuite) TestDelta() { test.scope, DeltaPaths{}, control.Options{}, - func(status *support.ConnectorOperationStatus) {}) + func(status *support.ConnectorOperationStatus) {}, + fault.New(true)) require.NoError(t, err) assert.Less(t, 1, len(collections), "retrieved metadata and data collections") @@ -364,7 +367,8 @@ func (suite *DataCollectionsIntegrationSuite) TestDelta() { test.scope, dps, control.Options{}, - func(status *support.ConnectorOperationStatus) {}) + func(status *support.ConnectorOperationStatus) {}, + fault.New(true)) require.NoError(t, err) // TODO(keepers): this isn't a very useful test at the moment. It needs to @@ -409,7 +413,8 @@ func (suite *DataCollectionsIntegrationSuite) TestMailSerializationRegression() sel.Scopes()[0], DeltaPaths{}, control.Options{}, - newStatusUpdater(t, &wg)) + newStatusUpdater(t, &wg), + fault.New(true)) require.NoError(t, err) wg.Add(len(collections)) @@ -476,7 +481,8 @@ func (suite *DataCollectionsIntegrationSuite) TestContactSerializationRegression test.scope, DeltaPaths{}, control.Options{}, - newStatusUpdater(t, &wg)) + newStatusUpdater(t, &wg), + fault.New(true)) require.NoError(t, err) wg.Add(len(edcs)) @@ -583,7 +589,8 @@ func (suite *DataCollectionsIntegrationSuite) TestEventsSerializationRegression( test.scope, DeltaPaths{}, control.Options{}, - newStatusUpdater(t, &wg)) + newStatusUpdater(t, &wg), + fault.New(true)) require.NoError(t, err) require.Len(t, collections, 2) diff --git a/src/internal/connector/exchange/event_calendar_cache.go b/src/internal/connector/exchange/event_calendar_cache.go index c7736970e..c49e19e14 100644 --- a/src/internal/connector/exchange/event_calendar_cache.go +++ b/src/internal/connector/exchange/event_calendar_cache.go @@ -3,10 +3,10 @@ package exchange import ( "context" + "github.com/alcionai/clues" "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/connector/graph" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/pkg/path" ) @@ -42,7 +42,7 @@ func (ecc *eventCalendarCache) populateEventRoot(ctx context.Context) error { f, err := ecc.getter.GetContainerByID(ctx, ecc.userID, container) if err != nil { - return errors.Wrap(err, "fetching calendar "+support.ConnectorStackErrorTrace(err)) + return errors.Wrap(err, "fetching calendar") } temp := graph.NewCacheFolder( @@ -50,7 +50,7 @@ func (ecc *eventCalendarCache) populateEventRoot(ctx context.Context) error { path.Builder{}.Append(*f.GetId()), // storage path path.Builder{}.Append(*f.GetDisplayName())) // display location if err := ecc.addFolder(temp); err != nil { - return errors.Wrap(err, "initializing calendar resolver") + return clues.Wrap(err, "initializing calendar resolver").WithClues(ctx) } return nil @@ -88,7 +88,7 @@ func (ecc *eventCalendarCache) Populate( // @returns error iff the required values are not accessible. func (ecc *eventCalendarCache) AddToCache(ctx context.Context, f graph.Container, useIDInPath bool) error { if err := checkIDAndName(f); err != nil { - return errors.Wrap(err, "validating container") + return clues.Wrap(err, "validating container").WithClues(ctx) } temp := graph.NewCacheFolder( @@ -104,7 +104,7 @@ func (ecc *eventCalendarCache) AddToCache(ctx context.Context, f graph.Container if err := ecc.addFolder(temp); err != nil { delete(ecc.newAdditions, *f.GetDisplayName()) - return errors.Wrap(err, "adding container") + return clues.Wrap(err, "adding container").WithClues(ctx) } // Populate the path for this entry so calls to PathInCache succeed no matter diff --git a/src/internal/connector/exchange/mail_folder_cache.go b/src/internal/connector/exchange/mail_folder_cache.go index f9f4cf687..4bf31460c 100644 --- a/src/internal/connector/exchange/mail_folder_cache.go +++ b/src/internal/connector/exchange/mail_folder_cache.go @@ -3,10 +3,10 @@ package exchange import ( "context" + "github.com/alcionai/clues" "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/connector/graph" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/pkg/path" ) @@ -46,7 +46,7 @@ func (mc *mailFolderCache) populateMailRoot(ctx context.Context) error { f, err := mc.getter.GetContainerByID(ctx, mc.userID, fldr) if err != nil { - return support.ConnectorStackErrorTraceWrap(err, "fetching root folder") + return clues.Wrap(err, "fetching root folder") } if fldr == DefaultMailFolder { @@ -57,7 +57,7 @@ func (mc *mailFolderCache) populateMailRoot(ctx context.Context) error { path.Builder{}.Append(directory), // storage path path.Builder{}.Append(directory)) // display location if err := mc.addFolder(temp); err != nil { - return errors.Wrap(err, "adding resolver dir") + return clues.Wrap(err, "adding resolver dir").WithClues(ctx) } } diff --git a/src/internal/connector/exchange/service_functions.go b/src/internal/connector/exchange/service_functions.go index 16b81c8a8..f3fb69462 100644 --- a/src/internal/connector/exchange/service_functions.go +++ b/src/internal/connector/exchange/service_functions.go @@ -2,8 +2,8 @@ package exchange import ( "context" - "fmt" + "github.com/alcionai/clues" "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/connector/exchange/api" @@ -75,11 +75,11 @@ func PopulateExchangeContainerResolver( cacheRoot = DefaultCalendar default: - return nil, fmt.Errorf("ContainerResolver not present for %s type", qp.Category) + return nil, clues.New("no container resolver registered for category").WithClues(ctx) } if err := res.Populate(ctx, cacheRoot); err != nil { - return nil, errors.Wrap(err, "populating directory resolver") + return nil, clues.Wrap(err, "populating directory resolver").WithClues(ctx) } return res, nil diff --git a/src/internal/connector/exchange/service_iterators.go b/src/internal/connector/exchange/service_iterators.go index f5ba34b53..49a167437 100644 --- a/src/internal/connector/exchange/service_iterators.go +++ b/src/internal/connector/exchange/service_iterators.go @@ -2,8 +2,8 @@ package exchange import ( "context" - "fmt" + "github.com/alcionai/clues" "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/connector/exchange/api" @@ -11,6 +11,7 @@ import ( "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/pkg/control" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" @@ -38,9 +39,9 @@ func filterContainersAndFillCollections( scope selectors.ExchangeScope, dps DeltaPaths, ctrlOpts control.Options, + errs *fault.Errors, ) error { var ( - errs error // folder ID -> delta url or folder path lookups deltaURLs = map[string]string{} currPaths = map[string]string{} @@ -63,8 +64,8 @@ func filterContainersAndFillCollections( } for _, c := range resolver.Items() { - if ctrlOpts.FailFast && errs != nil { - return errs + if errs.Err() != nil { + return errs.Err() } cID := *c.GetId() @@ -85,7 +86,7 @@ func filterContainersAndFillCollections( if len(prevPathStr) > 0 { if prevPath, err = pathFromPrevString(prevPathStr); err != nil { - logger.Ctx(ctx).Error(err) + logger.Ctx(ctx).With("err", err).Errorw("parsing prev path", clues.InErr(err).Slice()...) // if the previous path is unusable, then the delta must be, too. prevDelta = "" } @@ -94,7 +95,7 @@ func filterContainersAndFillCollections( added, removed, newDelta, err := getter.GetAddedAndRemovedItemIDs(ctx, qp.ResourceOwner, cID, prevDelta) if err != nil { if !graph.IsErrDeletedInFlight(err) { - errs = support.WrapAndAppend(qp.ResourceOwner, err, errs) + errs.Add(err) continue } @@ -150,7 +151,7 @@ func filterContainersAndFillCollections( // resolver (which contains all the resource owners' current containers). for id, p := range tombstones { if collections[id] != nil { - errs = support.WrapAndAppend(p, errors.New("conflict: tombstone exists for a live collection"), errs) + errs.Add(clues.Wrap(err, "conflict: tombstone exists for a live collection").WithClues(ctx)) continue } @@ -162,9 +163,8 @@ func filterContainersAndFillCollections( prevPath, err := pathFromPrevString(p) if err != nil { - // technically shouldn't ever happen. But just in case, we need to catch - // it for protection. - logger.Ctx(ctx).Errorw("parsing tombstone path", "err", err) + // technically shouldn't ever happen. But just in case... + logger.Ctx(ctx).With("err", err).Errorw("parsing tombstone prev path", clues.InErr(err).Slice()...) continue } @@ -189,20 +189,21 @@ func filterContainersAndFillCollections( entries = append(entries, graph.NewMetadataEntry(graph.DeltaURLsFileName, deltaURLs)) } - if col, err := graph.MakeMetadataCollection( + col, err := graph.MakeMetadataCollection( qp.Credentials.AzureTenantID, qp.ResourceOwner, path.ExchangeService, qp.Category, entries, statusUpdater, - ); err != nil { - errs = support.WrapAndAppend("making metadata collection", err, errs) - } else if col != nil { - collections["metadata"] = col + ) + if err != nil { + return clues.Wrap(err, "making metadata collection") } - return errs + collections["metadata"] = col + + return errs.Err() } // produces a set of id:path pairs from the deltapaths map. @@ -236,6 +237,6 @@ func itemerByType(ac api.Client, category path.CategoryType) (itemer, error) { case path.ContactsCategory: return ac.Contacts(), nil default: - return nil, fmt.Errorf("category %s not supported by getFetchIDFunc", category) + return nil, clues.New("category not registered in getFetchIDFunc") } } diff --git a/src/internal/connector/exchange/service_iterators_test.go b/src/internal/connector/exchange/service_iterators_test.go index cd1237b2e..81d9ebd99 100644 --- a/src/internal/connector/exchange/service_iterators_test.go +++ b/src/internal/connector/exchange/service_iterators_test.go @@ -17,6 +17,7 @@ import ( "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/control" + "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" ) @@ -230,7 +231,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() { }, resolver: newMockResolver(container1), scope: allScope, - expectErr: assert.Error, + expectErr: assert.NoError, expectNewColls: 0, expectMetadataColls: 1, }, @@ -255,7 +256,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() { }, resolver: newMockResolver(container1, container2), scope: allScope, - expectErr: assert.Error, + expectErr: assert.NoError, expectNewColls: 1, expectMetadataColls: 1, }, @@ -304,7 +305,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections() { test.scope, dps, control.Options{FailFast: test.failFast}, - ) + fault.New(test.failFast)) test.expectErr(t, err) // collection assertions @@ -457,7 +458,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_repea allScope, dps, control.Options{FailFast: true}, - ) + fault.New(true)) require.NoError(t, err) // collection assertions @@ -809,7 +810,7 @@ func (suite *ServiceIteratorsSuite) TestFilterContainersAndFillCollections_incre allScope, test.dps, control.Options{}, - ) + fault.New(true)) assert.NoError(t, err) metadatas := 0 diff --git a/src/internal/connector/graph_connector.go b/src/internal/connector/graph_connector.go index 45461e6d1..a60e76e0c 100644 --- a/src/internal/connector/graph_connector.go +++ b/src/internal/connector/graph_connector.go @@ -331,7 +331,7 @@ func getResources( } callbackFunc := func(item any) bool { - if errs.Failed() { + if errs.Err() != nil { return false } diff --git a/src/internal/operations/manifests.go b/src/internal/operations/manifests.go index 38ef60e0c..ebe40f8a9 100644 --- a/src/internal/operations/manifests.go +++ b/src/internal/operations/manifests.go @@ -142,7 +142,7 @@ func verifyDistinctBases(ctx context.Context, mans []*kopia.ManifestEntry, errs ) for _, man := range mans { - if errs.Failed() { + if errs.Err() != nil { break } diff --git a/src/pkg/fault/fault.go b/src/pkg/fault/fault.go index ee560965b..ea26757ee 100644 --- a/src/pkg/fault/fault.go +++ b/src/pkg/fault/fault.go @@ -84,11 +84,6 @@ func (e *Errors) Fail(err error) *Errors { return e.setErr(err) } -// Failed returns true if e.err != nil, signifying a catastrophic failure. -func (e *Errors) Failed() bool { - return e.err != nil -} - // setErr handles setting errors.err. Sync locking gets // handled upstream of this call. func (e *Errors) setErr(err error) *Errors {