From 1694581e2636d53f997c1823b151b7ea50458df4 Mon Sep 17 00:00:00 2001 From: Keepers Date: Thu, 11 Jan 2024 18:29:39 -0700 Subject: [PATCH] make a `not found` core error (#4975) make a canonical 'not found' error for our core sentinels. This replaces the graph.DeletedInFlight error, which acted as an interpretation of "not found". --- #### Does this PR need a docs update or release note? - [x] :no_entry: No #### Type of change - [x] :broom: Tech Debt/Cleanup #### Issue(s) * #4685 #### Test Plan - [x] :zap: Unit test - [x] :green_heart: E2E --- src/internal/m365/backup.go | 3 +- .../m365/collection/drive/collection.go | 8 +- .../m365/collection/drive/collection_test.go | 5 +- src/internal/m365/collection/drive/restore.go | 16 +- .../m365/collection/drive/restore_test.go | 13 +- .../m365/collection/exchange/backup.go | 4 +- .../m365/collection/exchange/backup_test.go | 3 +- .../m365/collection/exchange/collection.go | 6 +- .../collection/exchange/collection_test.go | 7 +- .../collection/exchange/contacts_restore.go | 7 +- .../exchange/contacts_restore_test.go | 6 +- .../collection/exchange/container_resolver.go | 4 +- .../exchange/container_resolver_test.go | 3 +- .../collection/exchange/events_restore.go | 7 +- .../exchange/events_restore_test.go | 6 +- .../m365/collection/exchange/mail_restore.go | 7 +- .../collection/exchange/mail_restore_test.go | 6 +- .../m365/collection/exchange/restore.go | 8 +- .../m365/collection/groups/backup_test.go | 3 +- .../m365/collection/groups/collection.go | 4 +- .../m365/collection/groups/collection_test.go | 6 +- .../m365/collection/site/collection.go | 4 +- .../m365/collection/site/collection_test.go | 6 +- src/internal/m365/controller.go | 2 +- .../m365/service/exchange/enabled_test.go | 4 +- src/pkg/errs/core/core.go | 20 ++- src/pkg/errs/errs.go | 28 +--- src/pkg/errs/errs_test.go | 67 +------- src/pkg/services/m365/api/contacts.go | 3 +- src/pkg/services/m365/api/drive.go | 8 +- src/pkg/services/m365/api/drive_test.go | 7 +- src/pkg/services/m365/api/graph/errors.go | 144 +++++++----------- .../services/m365/api/graph/errors_test.go | 77 ++-------- src/pkg/services/m365/api/graph/service.go | 4 +- .../services/m365/api/graph/service_test.go | 5 +- src/pkg/services/m365/api/groups.go | 4 +- src/pkg/services/m365/api/groups_test.go | 4 +- src/pkg/services/m365/api/lists.go | 4 - src/pkg/services/m365/api/lists_test.go | 4 +- src/pkg/services/m365/api/mail.go | 3 +- .../services/m365/api/pagers/pagers_test.go | 3 +- src/pkg/services/m365/api/sites.go | 15 -- src/pkg/services/m365/api/sites_test.go | 4 +- src/pkg/services/m365/api/users.go | 6 +- src/pkg/services/m365/api/users_test.go | 4 +- src/pkg/services/m365/groups_test.go | 4 +- src/pkg/services/m365/users_test.go | 13 +- 47 files changed, 222 insertions(+), 357 deletions(-) diff --git a/src/internal/m365/backup.go b/src/internal/m365/backup.go index c76d2e703..b6bab4120 100644 --- a/src/internal/m365/backup.go +++ b/src/internal/m365/backup.go @@ -171,8 +171,7 @@ func verifyBackupInputs(sels selectors.Selector, cachedIDs []string) error { } if !filters.Contains(ids).Compare(sels.ID()) { - return clues.Stack(core.ErrResourceOwnerNotFound). - With("selector_protected_resource", sels.DiscreteOwner) + return clues.Stack(core.ErrNotFound).With("selector_protected_resource", sels.DiscreteOwner) } return nil diff --git a/src/internal/m365/collection/drive/collection.go b/src/internal/m365/collection/drive/collection.go index 27f01ce44..aa196281a 100644 --- a/src/internal/m365/collection/drive/collection.go +++ b/src/internal/m365/collection/drive/collection.go @@ -10,6 +10,7 @@ import ( "time" "github.com/alcionai/clues" + "github.com/pkg/errors" "github.com/spatialcurrent/go-lazy/pkg/lazy" "github.com/alcionai/corso/src/internal/common/idname" @@ -21,6 +22,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/extensions" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" @@ -302,7 +304,7 @@ func (oc *Collection) getDriveItemContent( return nil, clues.Wrap(err, "malware item").Label(graph.LabelsSkippable) } - if clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) || graph.IsErrDeletedInFlight(err) { + if clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) || errors.Is(err, core.ErrNotFound) { logger.CtxErr(ctx, err).Info("item not found, probably deleted in flight") return nil, clues.Wrap(err, "deleted item").Label(graph.LabelsSkippable) } @@ -421,7 +423,7 @@ func readItemContents( // Handle newly deleted items if props.isDeleted { logger.Ctx(ctx).Info("item deleted in cache") - return nil, graph.ErrDeletedInFlight + return nil, core.ErrNotFound } rc, err := downloadFile(ctx, iaag, props.downloadURL) @@ -605,7 +607,7 @@ func (oc *Collection) streamDriveItem( itemMeta, itemMetaSize, err = downloadItemMeta(ctx, oc.handler, oc.driveID, item) if err != nil { // Skip deleted items - if !clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) && !graph.IsErrDeletedInFlight(err) { + if !clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) && !errors.Is(err, core.ErrNotFound) { errs.AddRecoverable(ctx, clues.Wrap(err, "getting item metadata").Label(fault.LabelForceNoBackupCreation)) } diff --git a/src/internal/m365/collection/drive/collection_test.go b/src/internal/m365/collection/drive/collection_test.go index ad9ac773c..58d7178cd 100644 --- a/src/internal/m365/collection/drive/collection_test.go +++ b/src/internal/m365/collection/drive/collection_test.go @@ -29,6 +29,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/extensions" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" @@ -767,10 +768,10 @@ func (suite *GetDriveItemUnitTestSuite) TestDownloadContent() { }, { name: "url refreshed from cache but item deleted", - mgi: getsItem{Item: itemWID, Err: graph.ErrDeletedInFlight}, + mgi: getsItem{Item: itemWID, Err: core.ErrNotFound}, itemInfo: details.ItemInfo{}, respBody: []io.ReadCloser{nil, nil, nil}, - getErr: []error{errUnauth, graph.ErrDeletedInFlight, graph.ErrDeletedInFlight}, + getErr: []error{errUnauth, core.ErrNotFound, core.ErrNotFound}, expectErr: require.Error, expect: require.Nil, muc: &mockURLCache{ diff --git a/src/internal/m365/collection/drive/restore.go b/src/internal/m365/collection/drive/restore.go index 8f7059491..626ed5a85 100644 --- a/src/internal/m365/collection/drive/restore.go +++ b/src/internal/m365/collection/drive/restore.go @@ -25,6 +25,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -281,7 +282,7 @@ func restoreItem( itemData, ctr) if err != nil { - if errors.Is(err, graph.ErrItemAlreadyExistsConflict) && rcc.RestoreConfig.OnCollision == control.Skip { + if errors.Is(err, core.ErrAlreadyExists) && rcc.RestoreConfig.OnCollision == control.Skip { return details.ItemInfo{}, true, nil } @@ -339,7 +340,7 @@ func restoreItem( ctr, errs) if err != nil { - if errors.Is(err, graph.ErrItemAlreadyExistsConflict) && rcc.RestoreConfig.OnCollision == control.Skip { + if errors.Is(err, core.ErrAlreadyExists) && rcc.RestoreConfig.OnCollision == control.Skip { return details.ItemInfo{}, true, nil } @@ -365,7 +366,7 @@ func restoreItem( ctr, errs) if err != nil { - if errors.Is(err, graph.ErrItemAlreadyExistsConflict) && rcc.RestoreConfig.OnCollision == control.Skip { + if errors.Is(err, core.ErrAlreadyExists) && rcc.RestoreConfig.OnCollision == control.Skip { return details.ItemInfo{}, true, nil } @@ -671,7 +672,7 @@ func createFolder( // ErrItemAlreadyExistsConflict can only occur for folders if the // item being replaced is a file, not another folder. - if err != nil && !errors.Is(err, graph.ErrItemAlreadyExistsConflict) { + if err != nil && !errors.Is(err, core.ErrAlreadyExists) { return nil, clues.Wrap(err, "creating folder") } @@ -742,7 +743,7 @@ func restoreFile( ctr.Inc(count.CollisionSkip) log.Debug("skipping item with collision") - return "", details.ItemInfo{}, graph.ErrItemAlreadyExistsConflict + return "", details.ItemInfo{}, core.ErrAlreadyExists } collision = dci @@ -757,7 +758,8 @@ func restoreFile( // risk failures in the middle, or we post w/ copy, then delete, then patch // the name, which could triple our graph calls in the worst case. if shouldDeleteOriginal { - if err := ir.DeleteItem(ctx, driveID, collision.ItemID); err != nil && !graph.IsErrDeletedInFlight(err) { + err := ir.DeleteItem(ctx, driveID, collision.ItemID) + if err != nil && !errors.Is(err, core.ErrNotFound) { return "", details.ItemInfo{}, clues.New("deleting colliding item") } } @@ -970,7 +972,7 @@ func ensureDriveExists( ictx := clues.Add(ctx, "new_drive_name", clues.Hide(nextDriveName)) newDrive, err = pdagrf.PostDrive(ictx, protectedResourceID, nextDriveName) - if err != nil && !errors.Is(err, graph.ErrItemAlreadyExistsConflict) { + if err != nil && !errors.Is(err, core.ErrAlreadyExists) { return driveInfo{}, clues.Wrap(err, "creating new drive") } diff --git a/src/internal/m365/collection/drive/restore_test.go b/src/internal/m365/collection/drive/restore_test.go index 82b84ee3d..026e7ff99 100644 --- a/src/internal/m365/collection/drive/restore_test.go +++ b/src/internal/m365/collection/drive/restore_test.go @@ -22,6 +22,7 @@ import ( "github.com/alcionai/corso/src/internal/version" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" @@ -122,7 +123,7 @@ func (suite *RestoreUnitSuite) TestRestoreItem_collisionHandling() { mock.DriveItemFileName: {ItemID: "smarf"}, }, onCollision: control.Replace, - deleteErr: graph.ErrDeletedInFlight, + deleteErr: core.ErrNotFound, expectSkipped: assert.False, expectMock: func(t *testing.T, rh *mockRestoreHandler) { assert.True(t, rh.CalledPostItem, "new item posted") @@ -298,7 +299,7 @@ func (suite *RestoreUnitSuite) TestCreateFolder() { { name: "good with copy", mock: &mockPIIC{ - errs: []error{graph.ErrItemAlreadyExistsConflict, nil}, + errs: []error{core.ErrAlreadyExists, nil}, items: []models.DriveItemable{nil, models.NewDriveItem()}, }, expectErr: assert.NoError, @@ -316,7 +317,7 @@ func (suite *RestoreUnitSuite) TestCreateFolder() { { name: "bad with copy", mock: &mockPIIC{ - errs: []error{graph.ErrItemAlreadyExistsConflict, assert.AnError}, + errs: []error{core.ErrAlreadyExists, assert.AnError}, items: []models.DriveItemable{nil, nil}, }, expectErr: assert.Error, @@ -759,7 +760,7 @@ func (suite *RestoreUnitSuite) TestEnsureDriveExists() { dp: dp, mock: &mockPDAGRF{ postResp: []models.Driveable{nil, makeMD()}, - postErr: []error{graph.ErrItemAlreadyExistsConflict, nil}, + postErr: []error{core.ErrAlreadyExists, nil}, grf: grf, }, rc: NewRestoreCaches(nil), @@ -773,7 +774,7 @@ func (suite *RestoreUnitSuite) TestEnsureDriveExists() { dp: oldDP, mock: &mockPDAGRF{ postResp: []models.Driveable{nil, makeMD()}, - postErr: []error{graph.ErrItemAlreadyExistsConflict, nil}, + postErr: []error{core.ErrAlreadyExists, nil}, grf: grf, }, rc: NewRestoreCaches(oldDriveIDNames), @@ -787,7 +788,7 @@ func (suite *RestoreUnitSuite) TestEnsureDriveExists() { dp: dp, mock: &mockPDAGRF{ postResp: []models.Driveable{nil, makeMD()}, - postErr: []error{graph.ErrItemAlreadyExistsConflict, nil}, + postErr: []error{core.ErrAlreadyExists, nil}, grf: grf, }, rc: populatedCache(driveID), diff --git a/src/internal/m365/collection/exchange/backup.go b/src/internal/m365/collection/exchange/backup.go index 240e2491b..6326099d7 100644 --- a/src/internal/m365/collection/exchange/backup.go +++ b/src/internal/m365/collection/exchange/backup.go @@ -2,6 +2,7 @@ package exchange import ( "context" + "errors" "fmt" "github.com/alcionai/clues" @@ -15,6 +16,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/metadata" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -265,7 +267,7 @@ func populateCollections( prevDelta, itemConfig) if err != nil { - if !graph.IsErrDeletedInFlight(err) { + if !errors.Is(err, core.ErrNotFound) { el.AddRecoverable(ctx, clues.Stack(err).Label(fault.LabelForceNoBackupCreation)) continue } diff --git a/src/internal/m365/collection/exchange/backup_test.go b/src/internal/m365/collection/exchange/backup_test.go index 8d1264144..61720157a 100644 --- a/src/internal/m365/collection/exchange/backup_test.go +++ b/src/internal/m365/collection/exchange/backup_test.go @@ -31,6 +31,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/metadata" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" @@ -1010,7 +1011,7 @@ func (suite *CollectionPopulationSuite) TestPopulateCollections() { added: []string{"a1", "a2", "a3"}, removed: []string{"r1", "r2", "r3"}, newDelta: pagers.DeltaUpdate{URL: "delta_url"}, - err: graph.ErrDeletedInFlight, + err: core.ErrNotFound, } container1 = mockContainer{ id: strPtr("1"), diff --git a/src/internal/m365/collection/exchange/collection.go b/src/internal/m365/collection/exchange/collection.go index b05764219..43412f78b 100644 --- a/src/internal/m365/collection/exchange/collection.go +++ b/src/internal/m365/collection/exchange/collection.go @@ -6,6 +6,7 @@ package exchange import ( "bytes" "context" + "errors" "io" "sync" "sync/atomic" @@ -19,6 +20,7 @@ import ( "github.com/alcionai/corso/src/internal/observe" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -278,7 +280,7 @@ func (col *prefetchCollection) streamItems( if err != nil { // Handle known error cases switch { - case graph.IsErrDeletedInFlight(err): + case errors.Is(err, core.ErrNotFound): // Don't report errors for deleted items as there's no way for us to // back up data that is gone. Record it as a "success", since there's // nothing else we can do, and not reporting it will make the status @@ -490,7 +492,7 @@ func (lig *lazyItemGetter) GetData( // // The item will be deleted from kopia on the next backup when the // delta token shows it's removed. - if graph.IsErrDeletedInFlight(err) { + if errors.Is(err, core.ErrNotFound) { logger.CtxErr(ctx, err).Info("item not found") return nil, nil, true, nil } diff --git a/src/internal/m365/collection/exchange/collection_test.go b/src/internal/m365/collection/exchange/collection_test.go index 90aaf28a5..6f9743bdc 100644 --- a/src/internal/m365/collection/exchange/collection_test.go +++ b/src/internal/m365/collection/exchange/collection_test.go @@ -25,6 +25,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" @@ -200,10 +201,10 @@ func (suite *CollectionUnitSuite) TestGetItemWithRetries() { { name: "deleted in flight", items: &mock.ItemGetSerialize{ - GetErr: graph.ErrDeletedInFlight, + GetErr: core.ErrNotFound, }, expectErr: func(t *testing.T, err error) { - assert.True(t, graph.IsErrDeletedInFlight(err), "is ErrDeletedInFlight") + assert.ErrorIs(t, err, core.ErrNotFound, "is ErrItemNotFound") }, expectGetCalls: 1, }, @@ -682,7 +683,7 @@ func (suite *CollectionUnitSuite) TestLazyItem_ReturnsEmptyReaderOnDeletedInFlig ctx, flush := tester.NewContext(t) defer flush() - getter := &mock.ItemGetSerialize{GetErr: graph.ErrDeletedInFlight} + getter := &mock.ItemGetSerialize{GetErr: core.ErrNotFound} li := data.NewLazyItemWithInfo( ctx, diff --git a/src/internal/m365/collection/exchange/contacts_restore.go b/src/internal/m365/collection/exchange/contacts_restore.go index bf887dc4d..40fccb037 100644 --- a/src/internal/m365/collection/exchange/contacts_restore.go +++ b/src/internal/m365/collection/exchange/contacts_restore.go @@ -2,6 +2,7 @@ package exchange import ( "context" + "errors" "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" @@ -10,6 +11,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -120,7 +122,7 @@ func restoreContact( ctr.Inc(count.CollisionSkip) log.Debug("skipping item with collision") - return nil, graph.ErrItemAlreadyExistsConflict + return nil, core.ErrAlreadyExists } collisionID = id @@ -138,7 +140,8 @@ func restoreContact( // at least we'll have accidentally over-produced data instead of deleting // the user's data. if shouldDeleteOriginal { - if err := cr.DeleteItem(ctx, userID, collisionID); err != nil && !graph.IsErrDeletedInFlight(err) { + err := cr.DeleteItem(ctx, userID, collisionID) + if err != nil && !errors.Is(err, core.ErrNotFound) { return nil, graph.Wrap(ctx, err, "deleting colliding contact") } } diff --git a/src/internal/m365/collection/exchange/contacts_restore_test.go b/src/internal/m365/collection/exchange/contacts_restore_test.go index 9ac34b995..914c5020d 100644 --- a/src/internal/m365/collection/exchange/contacts_restore_test.go +++ b/src/internal/m365/collection/exchange/contacts_restore_test.go @@ -16,10 +16,10 @@ import ( "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/testdata" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" - "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) var _ contactRestorer = &contactRestoreMock{} @@ -153,7 +153,7 @@ func (suite *ContactsRestoreIntgSuite) TestRestoreContact() { collisionMap: map[string]string{collisionKey: "smarf"}, onCollision: control.Skip, expectErr: func(t *testing.T, err error) { - assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrAlreadyExists, clues.ToCore(err)) }, expectMock: func(t *testing.T, m *contactRestoreMock) { assert.False(t, m.calledPost, "new item posted") @@ -191,7 +191,7 @@ func (suite *ContactsRestoreIntgSuite) TestRestoreContact() { }, { name: "collision: replace - err already deleted", - apiMock: &contactRestoreMock{deleteItemErr: graph.ErrDeletedInFlight}, + apiMock: &contactRestoreMock{deleteItemErr: core.ErrNotFound}, collisionMap: map[string]string{collisionKey: "smarf"}, onCollision: control.Replace, expectErr: func(t *testing.T, err error) { diff --git a/src/internal/m365/collection/exchange/container_resolver.go b/src/internal/m365/collection/exchange/container_resolver.go index d8e31fcc1..dcf6ee528 100644 --- a/src/internal/m365/collection/exchange/container_resolver.go +++ b/src/internal/m365/collection/exchange/container_resolver.go @@ -2,11 +2,13 @@ package exchange import ( "context" + "errors" "github.com/alcionai/clues" "golang.org/x/exp/slices" "github.com/alcionai/corso/src/internal/common/ptr" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -95,7 +97,7 @@ func (cr *containerResolver) refreshContainer( } c, err := cr.refresher.refreshContainer(ctx, id) - if err != nil && graph.IsErrDeletedInFlight(err) { + if err != nil && errors.Is(err, core.ErrNotFound) { logger.Ctx(ctx).Debug("container deleted") return nil, true, nil } else if err != nil { diff --git a/src/internal/m365/collection/exchange/container_resolver_test.go b/src/internal/m365/collection/exchange/container_resolver_test.go index e0cbe9fe4..c78fb0383 100644 --- a/src/internal/m365/collection/exchange/container_resolver_test.go +++ b/src/internal/m365/collection/exchange/container_resolver_test.go @@ -20,6 +20,7 @@ import ( "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" @@ -670,7 +671,7 @@ func (r mockContainerRefresher) refreshContainer( rr, ok := r.entries[id] if !ok { // May not be this precise error, but it's easy to get a handle on. - return nil, graph.ErrDeletedInFlight + return nil, core.ErrNotFound } if rr.err != nil { diff --git a/src/internal/m365/collection/exchange/events_restore.go b/src/internal/m365/collection/exchange/events_restore.go index 2758583fd..c53f66987 100644 --- a/src/internal/m365/collection/exchange/events_restore.go +++ b/src/internal/m365/collection/exchange/events_restore.go @@ -3,6 +3,7 @@ package exchange import ( "bytes" "context" + "errors" "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" @@ -11,6 +12,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -126,7 +128,7 @@ func restoreEvent( ctr.Inc(count.CollisionSkip) log.Debug("skipping item with collision") - return nil, graph.ErrItemAlreadyExistsConflict + return nil, core.ErrAlreadyExists } collisionID = id @@ -155,7 +157,8 @@ func restoreEvent( // at least we'll have accidentally over-produced data instead of deleting // the user's data. if shouldDeleteOriginal { - if err := er.DeleteItem(ctx, userID, collisionID); err != nil && !graph.IsErrDeletedInFlight(err) { + err := er.DeleteItem(ctx, userID, collisionID) + if err != nil && !errors.Is(err, core.ErrNotFound) { return nil, graph.Wrap(ctx, err, "deleting colliding event") } } diff --git a/src/internal/m365/collection/exchange/events_restore_test.go b/src/internal/m365/collection/exchange/events_restore_test.go index 41409808e..0d3157529 100644 --- a/src/internal/m365/collection/exchange/events_restore_test.go +++ b/src/internal/m365/collection/exchange/events_restore_test.go @@ -17,10 +17,10 @@ import ( "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/testdata" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" - "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) var _ eventRestorer = &eventRestoreMock{} @@ -201,7 +201,7 @@ func (suite *EventsRestoreIntgSuite) TestRestoreEvent() { collisionMap: map[string]string{collisionKey: "smarf"}, onCollision: control.Skip, expectErr: func(t *testing.T, err error) { - assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrAlreadyExists, clues.ToCore(err)) }, expectMock: func(t *testing.T, m *eventRestoreMock) { assert.False(t, m.calledPost, "new item posted") @@ -239,7 +239,7 @@ func (suite *EventsRestoreIntgSuite) TestRestoreEvent() { }, { name: "collision: replace - err already deleted", - apiMock: &eventRestoreMock{deleteItemErr: graph.ErrDeletedInFlight}, + apiMock: &eventRestoreMock{deleteItemErr: core.ErrNotFound}, collisionMap: map[string]string{collisionKey: "smarf"}, onCollision: control.Replace, expectErr: func(t *testing.T, err error) { diff --git a/src/internal/m365/collection/exchange/mail_restore.go b/src/internal/m365/collection/exchange/mail_restore.go index 0c3646114..a50b733ad 100644 --- a/src/internal/m365/collection/exchange/mail_restore.go +++ b/src/internal/m365/collection/exchange/mail_restore.go @@ -2,6 +2,7 @@ package exchange import ( "context" + "errors" "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" @@ -11,6 +12,7 @@ import ( "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" "github.com/alcionai/corso/src/pkg/dttm" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -126,7 +128,7 @@ func restoreMail( ctr.Inc(count.CollisionSkip) log.Debug("skipping item with collision") - return nil, graph.ErrItemAlreadyExistsConflict + return nil, core.ErrAlreadyExists } collisionID = id @@ -150,7 +152,8 @@ func restoreMail( // at least we'll have accidentally over-produced data instead of deleting // the user's data. if shouldDeleteOriginal { - if err := mr.DeleteItem(ctx, userID, collisionID); err != nil && !graph.IsErrDeletedInFlight(err) { + err := mr.DeleteItem(ctx, userID, collisionID) + if err != nil && !errors.Is(err, core.ErrNotFound) { return nil, graph.Wrap(ctx, err, "deleting colliding mail message") } } diff --git a/src/internal/m365/collection/exchange/mail_restore_test.go b/src/internal/m365/collection/exchange/mail_restore_test.go index c48511259..9e22d1a08 100644 --- a/src/internal/m365/collection/exchange/mail_restore_test.go +++ b/src/internal/m365/collection/exchange/mail_restore_test.go @@ -17,10 +17,10 @@ import ( "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/testdata" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/services/m365/api" - "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) var _ mailRestorer = &mailRestoreMock{} @@ -170,7 +170,7 @@ func (suite *MailRestoreIntgSuite) TestRestoreMail() { collisionMap: map[string]string{collisionKey: "smarf"}, onCollision: control.Skip, expectErr: func(t *testing.T, err error) { - assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrAlreadyExists, clues.ToCore(err)) }, expectMock: func(t *testing.T, m *mailRestoreMock) { assert.False(t, m.calledPost, "new item posted") @@ -208,7 +208,7 @@ func (suite *MailRestoreIntgSuite) TestRestoreMail() { }, { name: "collision: replace - err already deleted", - apiMock: &mailRestoreMock{deleteItemErr: graph.ErrDeletedInFlight}, + apiMock: &mailRestoreMock{deleteItemErr: core.ErrNotFound}, collisionMap: map[string]string{collisionKey: "smarf"}, onCollision: control.Replace, expectErr: func(t *testing.T, err error) { diff --git a/src/internal/m365/collection/exchange/restore.go b/src/internal/m365/collection/exchange/restore.go index dd4bb372b..a71c3ba13 100644 --- a/src/internal/m365/collection/exchange/restore.go +++ b/src/internal/m365/collection/exchange/restore.go @@ -3,6 +3,7 @@ package exchange import ( "bytes" "context" + "errors" "runtime/trace" "time" @@ -18,6 +19,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -87,7 +89,7 @@ func RestoreCollection( errs, ctr) if err != nil { - if !graph.IsErrItemAlreadyExistsConflict(err) { + if !errors.Is(err, core.ErrAlreadyExists) { el.AddRecoverable(ictx, clues.Wrap(err, "restoring item")) } @@ -256,7 +258,7 @@ func uploadAttachments( retryBackoff.Reset() - for (retry == 0 || graph.IsErrItemNotFound(err)) && retry <= maxRetries { + for (retry == 0 || errors.Is(err, core.ErrNotFound)) && retry <= maxRetries { retry++ ictx := clues.Add(actx, "attempt_num", retry) @@ -272,7 +274,7 @@ func uploadAttachments( // Sometimes graph returns a 404 when we try to post the attachment. // We're not sure why, but maybe it has to do with attaching many items. // In any case, wait a little while and try again. - if graph.IsErrItemNotFound(err) && retry <= maxRetries { + if errors.Is(err, core.ErrNotFound) && retry <= maxRetries { waitTime := retryBackoff.NextBackOff() logger.Ctx(ictx).Infow( diff --git a/src/internal/m365/collection/groups/backup_test.go b/src/internal/m365/collection/groups/backup_test.go index 2afab0cef..442904fc2 100644 --- a/src/internal/m365/collection/groups/backup_test.go +++ b/src/internal/m365/collection/groups/backup_test.go @@ -24,6 +24,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/metadata" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" @@ -239,7 +240,7 @@ func (suite *BackupUnitSuite) TestPopulateCollections() { name: "err: deleted in flight", mock: mockBackupHandler{ channels: testdata.StubChannels("one"), - messagesErr: graph.ErrDeletedInFlight, + messagesErr: core.ErrNotFound, }, expectErr: require.Error, expectColls: 1, diff --git a/src/internal/m365/collection/groups/collection.go b/src/internal/m365/collection/groups/collection.go index 9c3914b18..fdcfe72ec 100644 --- a/src/internal/m365/collection/groups/collection.go +++ b/src/internal/m365/collection/groups/collection.go @@ -3,6 +3,7 @@ package groups import ( "bytes" "context" + "errors" "io" "net/http" "sync" @@ -17,6 +18,7 @@ import ( "github.com/alcionai/corso/src/internal/observe" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -420,7 +422,7 @@ func (lig *lazyItemGetter[C, I]) GetData( if err != nil { // For items that were deleted in flight, add the skip label so that // they don't lead to recoverable failures during backup. - if clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) || graph.IsErrDeletedInFlight(err) { + if clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) || errors.Is(err, core.ErrNotFound) { logger.CtxErr(ctx, err).Info("item deleted in flight. skipping") // Returning delInFlight as true here for correctness, although the caller is going diff --git a/src/internal/m365/collection/groups/collection_test.go b/src/internal/m365/collection/groups/collection_test.go index 860b2ee9f..ea2ae47f6 100644 --- a/src/internal/m365/collection/groups/collection_test.go +++ b/src/internal/m365/collection/groups/collection_test.go @@ -22,9 +22,9 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" - "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) type CollectionUnitSuite struct { @@ -533,7 +533,7 @@ func (suite *CollectionUnitSuite) TestLazyItem_ReturnsEmptyReaderOnDeletedInFlig defer flush() m := getAndAugmentConversation{ - GetItemErr: graph.ErrDeletedInFlight, + GetItemErr: core.ErrNotFound, } li := data.NewLazyItemWithInfo( @@ -558,7 +558,7 @@ func (suite *CollectionUnitSuite) TestLazyItem_ReturnsEmptyReaderOnDeletedInFlig "item mod time") _, err := readers.NewVersionedRestoreReader(li.ToReader()) - assert.ErrorIs(t, err, graph.ErrDeletedInFlight, "item should be marked deleted in flight") + assert.ErrorIs(t, err, core.ErrNotFound, "item should be marked deleted in flight") } func (suite *CollectionUnitSuite) TestLazyItem() { diff --git a/src/internal/m365/collection/site/collection.go b/src/internal/m365/collection/site/collection.go index 39e5fd27c..5d33a3122 100644 --- a/src/internal/m365/collection/site/collection.go +++ b/src/internal/m365/collection/site/collection.go @@ -3,6 +3,7 @@ package site import ( "bytes" "context" + "errors" "io" "net/http" "sync" @@ -23,6 +24,7 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -501,7 +503,7 @@ func (lig *lazyItemGetter) GetData( ) (io.ReadCloser, *details.ItemInfo, bool, error) { list, info, err := lig.getter.GetItemByID(ctx, lig.itemID) if err != nil { - if clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) || graph.IsErrDeletedInFlight(err) { + if clues.HasLabel(err, graph.LabelStatus(http.StatusNotFound)) || errors.Is(err, core.ErrNotFound) { logger.CtxErr(ctx, err).Info("item deleted in flight. skipping") // Returning delInFlight as true here for correctness, although the caller is going diff --git a/src/internal/m365/collection/site/collection_test.go b/src/internal/m365/collection/site/collection_test.go index a268d80d3..44ea713c8 100644 --- a/src/internal/m365/collection/site/collection_test.go +++ b/src/internal/m365/collection/site/collection_test.go @@ -25,11 +25,11 @@ import ( "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/selectors" "github.com/alcionai/corso/src/pkg/services/m365/api" - "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) type SharePointCollectionUnitSuite struct { @@ -469,7 +469,7 @@ func (suite *SharePointCollectionSuite) TestLazyItem_ReturnsEmptyReaderOnDeleted ctx, flush := tester.NewContext(t) defer flush() - lh := mock.NewListHandler(nil, "", graph.ErrDeletedInFlight) + lh := mock.NewListHandler(nil, "", core.ErrNotFound) li := data.NewLazyItemWithInfo( ctx, @@ -491,6 +491,6 @@ func (suite *SharePointCollectionSuite) TestLazyItem_ReturnsEmptyReaderOnDeleted "item mod time") r, err := readers.NewVersionedRestoreReader(li.ToReader()) - assert.ErrorIs(t, err, graph.ErrDeletedInFlight, "item should be marked deleted in flight") + assert.ErrorIs(t, err, core.ErrNotFound, "item should be marked deleted in flight") assert.Nil(t, r) } diff --git a/src/internal/m365/controller.go b/src/internal/m365/controller.go index 8da8ec072..4057b2424 100644 --- a/src/internal/m365/controller.go +++ b/src/internal/m365/controller.go @@ -259,7 +259,7 @@ func (r resourceGetter) GetResourceIDAndNameFrom( } if len(id) == 0 || len(name) == 0 { - return nil, clues.Stack(core.ErrResourceOwnerNotFound) + return nil, clues.Stack(core.ErrNotFound) } return idname.NewProvider(id, name), nil diff --git a/src/internal/m365/service/exchange/enabled_test.go b/src/internal/m365/service/exchange/enabled_test.go index c37c61148..95d91ad71 100644 --- a/src/internal/m365/service/exchange/enabled_test.go +++ b/src/internal/m365/service/exchange/enabled_test.go @@ -92,14 +92,14 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() { name: "overlapping resourcenotfound", mock: func(ctx context.Context) getMailInboxer { odErr := graphTD.ODataErrWithMsg(string(graph.ResourceNotFound), "User not found") - err := clues.Stack(core.ErrResourceOwnerNotFound, odErr) + err := clues.Stack(core.ErrNotFound, odErr) return mockGMB{ mailboxErr: graph.Stack(ctx, err), } }, expect: assert.False, - expectErr: assert.Error, + expectErr: assert.NoError, }, { name: "arbitrary error", diff --git a/src/pkg/errs/core/core.go b/src/pkg/errs/core/core.go index 16a55e4a7..fd8b86587 100644 --- a/src/pkg/errs/core/core.go +++ b/src/pkg/errs/core/core.go @@ -51,16 +51,32 @@ func (e Err) Error() string { } var ( + // occurs when creation of an entity (usually by restful POST or PUT) errors + // because some other entity already already exists with a conflicting identifier. + // The identifier is not always the id. For example: duplicate filenames + // in the same directory will cause conflicts, even with different IDs. + ErrAlreadyExists = &Err{msg: "conflict: already exists"} // currently we have no internal throttling controls. We only try to match // external throttling requirements. This sentinel assumes that an external // server has returned one or more throttling errors which has stopped // operation progress. ErrApplicationThrottled = &Err{msg: "application throttled"} + // for use when a short-lived auth token (a jwt or something similar) expires. + ErrAuthTokenExpired = &Err{msg: "auth token expired"} // about what it sounds like: we tried to look for a backup by ID, but the // storage layer couldn't find anything for that ID. ErrBackupNotFound = &Err{msg: "backup not found"} // a catch-all for downstream api auth issues. doesn't matter which api. ErrInsufficientAuthorization = &Err{msg: "insufficient authorization"} + // happens when we look up something using an identifier other than a canonical ID + // (ex: filtering, searching, etc). This error should only be returned if a unique + // result is an expected constraint of the behavior. If it's possible to + // opportunistically select one of the many results, no error should get returned. + ErrMultipleResultsMatchIdentifier = &Err{msg: "multiple results match the identifier"} + // basically what it sounds like: we went looking for something by ID and + // it wasn't found. This might be because it was deleted in flight, or + // was never created, or some other reason. + ErrNotFound = &Err{msg: "not found"} // specifically for repository creation: if we tried to create a repo and // it already exists with those credentials, we return this error. ErrRepoAlreadyExists = &Err{msg: "repository already exists"} @@ -72,10 +88,6 @@ var ( // it, but are told by the external system that the resource is somehow // unusable. ErrResourceNotAccessible = &Err{msg: "resource not accesible"} - // use this when a resource (user, etc; whatever owner is used to own the - // data in the given backup) cannot be found in the system by the ID that - // the end user provided. - ErrResourceOwnerNotFound = &Err{msg: "resource owner not found"} // a service is the set of application data within a given provider. eg: // if m365 is the provider, then exchange is a service, so is oneDrive. // this sentinel is used to indicate that the service in question is not diff --git a/src/pkg/errs/errs.go b/src/pkg/errs/errs.go index 3f48b317f..baf35f5af 100644 --- a/src/pkg/errs/errs.go +++ b/src/pkg/errs/errs.go @@ -5,32 +5,21 @@ import ( "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/repository" - "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) // map of enums to errors. We might want to re-use an enum for multiple // internal errors. var externalToInternal = map[*core.Err][]error{ - core.ErrBackupNotFound: {repository.ErrorBackupNotFound}, - core.ErrRepoAlreadyExists: {repository.ErrorRepoAlreadyExists}, - core.ErrResourceNotAccessible: {graph.ErrResourceLocked}, + core.ErrBackupNotFound: {repository.ErrorBackupNotFound}, + core.ErrRepoAlreadyExists: {repository.ErrorRepoAlreadyExists}, } type ErrCheck func(error) bool -// map of enums to error comparators. The above map assumes that we -// always stack or wrap the sentinel error in the returned error. But in -// many places of error handling, we primarily rely on error comparison -// checks. This allows us to apply those comparison checks instead of relying -// only on sentinels. -var externalToInternalCheck = map[*core.Err][]ErrCheck{ - core.ErrResourceOwnerNotFound: {graph.IsErrItemNotFound}, -} - // Internal returns the internal errors and error checking functions which // match to the public error enum. -func Internal(ce *core.Err) ([]error, []ErrCheck) { - return externalToInternal[ce], externalToInternalCheck[ce] +func Internal(ce *core.Err) []error { + return externalToInternal[ce] } // Is checks if the provided error contains an internal error that matches @@ -49,14 +38,5 @@ func Is(err error, ce *core.Err) bool { } } - internalChecks, ok := externalToInternalCheck[ce] - if ok { - for _, check := range internalChecks { - if check(err) { - return true - } - } - } - return false } diff --git a/src/pkg/errs/errs_test.go b/src/pkg/errs/errs_test.go index 507ddc8ee..5cca1cc04 100644 --- a/src/pkg/errs/errs_test.go +++ b/src/pkg/errs/errs_test.go @@ -10,8 +10,6 @@ import ( "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/repository" - "github.com/alcionai/corso/src/pkg/services/m365/api/graph" - graphTD "github.com/alcionai/corso/src/pkg/services/m365/api/graph/testdata" ) type ErrUnitSuite struct { @@ -35,70 +33,11 @@ func (suite *ErrUnitSuite) TestInternal_errs() { get: core.ErrBackupNotFound, expect: []error{repository.ErrorBackupNotFound}, }, - { - get: core.ErrResourceNotAccessible, - expect: []error{graph.ErrResourceLocked}, - }, } for _, test := range table { suite.Run(test.get.Error(), func() { // can't compare func signatures - errs, _ := Internal(test.get) - assert.ElementsMatch(suite.T(), test.expect, errs) - }) - } -} - -func (suite *ErrUnitSuite) TestInternal_checks() { - table := []struct { - get *core.Err - err error - expectHasChecks assert.ValueAssertionFunc - expect assert.BoolAssertionFunc - }{ - { - get: core.ErrRepoAlreadyExists, - err: graphTD.ODataErr(string(graph.ApplicationThrottled)), - expectHasChecks: assert.Empty, - expect: assert.False, - }, - { - get: core.ErrBackupNotFound, - err: repository.ErrorBackupNotFound, - expectHasChecks: assert.Empty, - expect: assert.False, - }, - { - get: core.ErrResourceOwnerNotFound, - err: graphTD.ODataErr(string(graph.ItemNotFound)), - expectHasChecks: assert.NotEmpty, - expect: assert.True, - }, - { - get: core.ErrResourceOwnerNotFound, - err: graphTD.ODataErr(string(graph.ErrorItemNotFound)), - expectHasChecks: assert.NotEmpty, - expect: assert.True, - }, - } - for _, test := range table { - suite.Run(test.get.Error(), func() { - t := suite.T() - - _, checks := Internal(test.get) - - test.expectHasChecks(t, checks) - - var result bool - - for _, check := range checks { - if check(test.err) { - result = true - break - } - } - - test.expect(t, result) + assert.ElementsMatch(suite.T(), test.expect, Internal(test.get)) }) } } @@ -116,10 +55,6 @@ func (suite *ErrUnitSuite) TestIs() { target: core.ErrBackupNotFound, err: repository.ErrorBackupNotFound, }, - { - target: core.ErrResourceNotAccessible, - err: graph.ErrResourceLocked, - }, } for _, test := range table { suite.Run(test.target.Error(), func() { diff --git a/src/pkg/services/m365/api/contacts.go b/src/pkg/services/m365/api/contacts.go index 16041447e..e3e395472 100644 --- a/src/pkg/services/m365/api/contacts.go +++ b/src/pkg/services/m365/api/contacts.go @@ -14,6 +14,7 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/common/sanitize" "github.com/alcionai/corso/src/pkg/backup/details" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) @@ -143,7 +144,7 @@ func (c Contacts) GetContainerByName( // Return an error if multiple container exist (unlikely) or if no container // is found. if len(gv) != 1 { - return nil, clues.StackWC(ctx, graph.ErrMultipleResultsMatchIdentifier). + return nil, clues.StackWC(ctx, core.ErrMultipleResultsMatchIdentifier). With("returned_container_count", len(gv)) } diff --git a/src/pkg/services/m365/api/drive.go b/src/pkg/services/m365/api/drive.go index 934dfcc7c..7d3d40591 100644 --- a/src/pkg/services/m365/api/drive.go +++ b/src/pkg/services/m365/api/drive.go @@ -7,9 +7,11 @@ import ( "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/drives" "github.com/microsoftgraph/msgraph-sdk-go/models" + "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/pkg/control" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) @@ -52,7 +54,7 @@ func (c Drives) GetFolderByName( foundItem, err := builder.Get(ctx, nil) if err != nil { - if graph.IsErrDeletedInFlight(err) { + if errors.Is(err, core.ErrNotFound) { return nil, graph.Stack(ctx, clues.Stack(ErrFolderNotFound, err)) } @@ -182,10 +184,6 @@ func (c Drives) PostItemInContainer( newItem, err := builder.Post(ctx, newItem, nil) if err != nil { - if graph.IsErrItemAlreadyExistsConflict(err) { - return nil, clues.Stack(graph.ErrItemAlreadyExistsConflict, err) - } - return nil, graph.Wrap(ctx, err, "creating item in folder") } diff --git a/src/pkg/services/m365/api/drive_test.go b/src/pkg/services/m365/api/drive_test.go index 99ebb4e95..7ae5d11c5 100644 --- a/src/pkg/services/m365/api/drive_test.go +++ b/src/pkg/services/m365/api/drive_test.go @@ -17,6 +17,7 @@ import ( "github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/pkg/control" "github.com/alcionai/corso/src/pkg/control/testdata" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) @@ -114,7 +115,7 @@ func (suite *DriveAPIIntgSuite) TestDrives_PostItemInContainer() { onCollision: control.Skip, postItem: folder, expectErr: func(t *testing.T, err error) { - assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrAlreadyExists, clues.ToCore(err)) }, expectItem: func(t *testing.T, i models.DriveItemable) { assert.Nil(t, i) @@ -165,7 +166,7 @@ func (suite *DriveAPIIntgSuite) TestDrives_PostItemInContainer() { onCollision: control.Skip, postItem: file, expectErr: func(t *testing.T, err error) { - assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrAlreadyExists, clues.ToCore(err)) }, expectItem: func(t *testing.T, i models.DriveItemable) { assert.Nil(t, i) @@ -201,7 +202,7 @@ func (suite *DriveAPIIntgSuite) TestDrives_PostItemInContainer() { // onCollision: control.Replace, // postItem: file, // expectErr: func(t *testing.T, err error) { - // assert.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + // assert.ErrorIs(t, err, core.ErrConflictAlreadyExists, clues.ToCore(err)) // }, // expectItem: func(t *testing.T, i models.DriveItemable) { // assert.Nil(t, i) diff --git a/src/pkg/services/m365/api/graph/errors.go b/src/pkg/services/m365/api/graph/errors.go index 0c6543bba..3c0674963 100644 --- a/src/pkg/services/m365/api/graph/errors.go +++ b/src/pkg/services/m365/api/graph/errors.go @@ -107,87 +107,89 @@ const ( ) // --------------------------------------------------------------------------- -// error sentinels & categorization +// error categorization // --------------------------------------------------------------------------- -// These errors are graph specific. That means they don't have a clear parallel in -// pkg/errs/core. If these errors need to trickle outward to non-m365 layers, we -// need to find a sufficiently coarse errs/core sentinel to use as transformation. -var ( - // The folder or item was deleted between the time we identified - // it and when we tried to fetch data for it. - ErrDeletedInFlight = clues.New("deleted in flight") - - // ErrItemAlreadyExistsConflict denotes that a post or put attempted to create - // an item which already exists by some unique identifier. The identifier is - // not always the id. For example, in onedrive, this error can be produced - // when filenames collide in a @microsoft.graph.conflictBehavior=fail request. - ErrItemAlreadyExistsConflict = clues.New("item already exists") - - // ErrMultipleResultsMatchIdentifier describes a situation where we're doing a lookup - // in some way other than by canonical url ID (ex: filtering, searching, etc). - // This error should only be returned if a unique result is an expected constraint - // of the call results. If it's possible to opportunistically select one of the many - // replies, no error should get returned. - ErrMultipleResultsMatchIdentifier = clues.New("multiple results match the identifier") - - // ErrResourceLocked occurs when a resource has had its access locked. - // Example case: https://learn.microsoft.com/en-us/sharepoint/manage-lock-status - // This makes the resource inaccessible for any Corso operations. - ErrResourceLocked = clues.New("resource has been locked and must be unlocked by an administrator") - - ErrTokenExpired = clues.New("jwt token expired") -) - func stackWithCoreErr(ctx context.Context, err error, traceDepth int) error { if err == nil { return nil } + ode := parseODataErr(err) + switch { - case isErrApplicationThrottled(err): + case isErrBadJWTToken(ode, err): + err = clues.Stack(core.ErrAuthTokenExpired) + case isErrApplicationThrottled(ode, err): err = clues.Stack(core.ErrApplicationThrottled, err) - case isErrResourceLocked(err): + case isErrUserNotFound(ode, err): + err = clues.Stack(core.ErrNotFound, err) + case isErrResourceLocked(ode, err): err = clues.Stack(core.ErrResourceNotAccessible, err) - case isErrUserNotFound(err): - err = clues.Stack(core.ErrResourceOwnerNotFound, err) - case isErrInsufficientAuthorization(err): + case isErrInsufficientAuthorization(ode, err): err = clues.Stack(core.ErrInsufficientAuthorization, err) + case isErrNotFound(ode, err): + err = clues.Stack(core.ErrNotFound, err) + case isErrItemAlreadyExists(ode, err): + err = clues.Stack(core.ErrAlreadyExists, err) } return stackWithDepth(ctx, err, 1+traceDepth) } -func isErrApplicationThrottled(err error) bool { - return parseODataErr(err).hasErrorCode(err, ApplicationThrottled) +// unexported categorizers, for use with stackWithCoreErr + +func isErrApplicationThrottled(ode oDataErr, err error) bool { + return ode.hasErrorCode(err, ApplicationThrottled) } -func IsErrAuthenticationError(err error) bool { - return parseODataErr(err).hasErrorCode(err, AuthenticationError) +func isErrInsufficientAuthorization(ode oDataErr, err error) bool { + return ode.hasErrorCode(err, AuthorizationRequestDenied) } -func isErrInsufficientAuthorization(err error) bool { - return parseODataErr(err).hasErrorCode(err, AuthorizationRequestDenied) +func isErrNotFound(ode oDataErr, err error) bool { + return clues.HasLabel(err, LabelStatus(http.StatusNotFound)) || + ode.hasErrorCode( + err, + ErrorItemNotFound, + ItemNotFound, + syncFolderNotFound) } -func IsErrDeletedInFlight(err error) bool { - if errors.Is(err, ErrDeletedInFlight) { +func isErrUserNotFound(ode oDataErr, err error) bool { + if ode.hasErrorCode(err, RequestResourceNotFound, invalidUser) { return true } - if parseODataErr(err).hasErrorCode( - err, - ErrorItemNotFound, - ItemNotFound, - syncFolderNotFound) { - return true + if ode.hasErrorCode(err, ResourceNotFound) { + return strings.Contains(strings.ToLower(ode.Main.Message), "user") } return false } -func IsErrItemNotFound(err error) bool { - return parseODataErr(err).hasErrorCode(err, ItemNotFound, ErrorItemNotFound) +func isErrBadJWTToken(ode oDataErr, err error) bool { + return ode.hasErrorCode(err, invalidAuthenticationToken) +} + +func isErrItemAlreadyExists(ode oDataErr, err error) bool { + return ode.hasErrorCode(err, nameAlreadyExists) +} + +func isErrResourceLocked(ode oDataErr, err error) bool { + return ode.hasInnerErrorCode(err, ResourceLocked) || + ode.hasErrorCode(err, NotAllowed) || + ode.errMessageMatchesAllFilters( + err, + filters.In([]string{"the service principal for resource"}), + filters.In([]string{"this indicate that a subscription within the tenant has lapsed"}), + filters.In([]string{"preventing tokens from being issued for it"})) +} + +// exported categorizers + +func IsErrAuthenticationError(err error) bool { + return parseODataErr(err).hasErrorCode(err, AuthenticationError) } func IsErrInvalidDelta(err error) bool { @@ -213,20 +215,6 @@ func IsErrExchangeMailFolderNotFound(err error) bool { return parseODataErr(err).hasErrorCode(err, ResourceNotFound, ErrorItemNotFound, MailboxNotEnabledForRESTAPI) } -func isErrUserNotFound(err error) bool { - ode := parseODataErr(err) - - if ode.hasErrorCode(err, RequestResourceNotFound, invalidUser) { - return true - } - - if ode.hasErrorCode(err, ResourceNotFound) { - return strings.Contains(strings.ToLower(ode.Main.Message), "user") - } - - return false -} - func IsErrInvalidRecipients(err error) bool { return parseODataErr(err).hasErrorCode(err, ErrorInvalidRecipients) } @@ -259,16 +247,7 @@ func IsErrConnectionReset(err error) bool { func IsErrUnauthorizedOrBadToken(err error) bool { return clues.HasLabel(err, LabelStatus(http.StatusUnauthorized)) || parseODataErr(err).hasErrorCode(err, invalidAuthenticationToken) || - errors.Is(err, ErrTokenExpired) -} - -func IsErrBadJWTToken(err error) bool { - return parseODataErr(err).hasErrorCode(err, invalidAuthenticationToken) -} - -func IsErrItemAlreadyExistsConflict(err error) bool { - return errors.Is(err, ErrItemAlreadyExistsConflict) || - parseODataErr(err).hasErrorCode(err, nameAlreadyExists) + errors.Is(err, core.ErrAuthTokenExpired) } // LabelStatus transforms the provided statusCode into @@ -305,19 +284,6 @@ func IsErrSiteNotFound(err error) bool { return parseODataErr(err).hasErrorMessage(err, requestedSiteCouldNotBeFound) } -func isErrResourceLocked(err error) bool { - ode := parseODataErr(err) - - return errors.Is(err, ErrResourceLocked) || - ode.hasInnerErrorCode(err, ResourceLocked) || - ode.hasErrorCode(err, NotAllowed) || - ode.errMessageMatchesAllFilters( - err, - filters.In([]string{"the service principal for resource"}), - filters.In([]string{"this indicate that a subscription within the tenant has lapsed"}), - filters.In([]string{"preventing tokens from being issued for it"})) -} - func IsErrSharingDisabled(err error) bool { return parseODataErr(err).hasInnerErrorCode(err, sharingDisabled) } @@ -679,7 +645,7 @@ func IsURLExpired( } if expired { - return clues.StackWC(ctx, ErrTokenExpired), nil + return clues.StackWC(ctx, core.ErrAuthTokenExpired), nil } return nil, nil diff --git a/src/pkg/services/m365/api/graph/errors_test.go b/src/pkg/services/m365/api/graph/errors_test.go index 8e2c4ff5d..f33271994 100644 --- a/src/pkg/services/m365/api/graph/errors_test.go +++ b/src/pkg/services/m365/api/graph/errors_test.go @@ -14,6 +14,7 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" graphTD "github.com/alcionai/corso/src/pkg/services/m365/api/graph/testdata" "github.com/alcionai/corso/src/pkg/services/m365/custom" @@ -85,7 +86,8 @@ func (suite *GraphErrorsUnitSuite) TestIsErrApplicationThrottled() { } for _, test := range table { suite.Run(test.name, func() { - test.expect(suite.T(), isErrApplicationThrottled(test.err)) + ode := parseODataErr(test.err) + test.expect(suite.T(), isErrApplicationThrottled(ode, test.err)) }) } } @@ -153,12 +155,13 @@ func (suite *GraphErrorsUnitSuite) TestIsErrInsufficientAuthorization() { } for _, test := range table { suite.Run(test.name, func() { - test.expect(suite.T(), isErrInsufficientAuthorization(test.err)) + ode := parseODataErr(test.err) + test.expect(suite.T(), isErrInsufficientAuthorization(ode, test.err)) }) } } -func (suite *GraphErrorsUnitSuite) TestIsErrDeletedInFlight() { +func (suite *GraphErrorsUnitSuite) TestIsErrNotFound() { table := []struct { name string err error @@ -174,11 +177,6 @@ func (suite *GraphErrorsUnitSuite) TestIsErrDeletedInFlight() { err: assert.AnError, expect: assert.False, }, - { - name: "as", - err: ErrDeletedInFlight, - expect: assert.True, - }, { name: "non-matching oDataErr", err: graphTD.ODataErr("fnords"), @@ -197,7 +195,8 @@ func (suite *GraphErrorsUnitSuite) TestIsErrDeletedInFlight() { } for _, test := range table { suite.Run(test.name, func() { - test.expect(suite.T(), IsErrDeletedInFlight(test.err)) + ode := parseODataErr(test.err) + test.expect(suite.T(), isErrNotFound(ode, test.err)) }) } } @@ -481,7 +480,8 @@ func (suite *GraphErrorsUnitSuite) TestIsErrUserNotFound() { } for _, test := range table { suite.Run(test.name, func() { - test.expect(suite.T(), isErrUserNotFound(test.err)) + ode := parseODataErr(test.err) + test.expect(suite.T(), isErrUserNotFound(ode, test.err)) }) } } @@ -544,7 +544,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrUnauthorizedOrBadToken() { }, { name: "err token expired", - err: clues.Stack(assert.AnError, ErrTokenExpired), + err: clues.Stack(assert.AnError, core.ErrAuthTokenExpired), expect: assert.True, }, { @@ -587,11 +587,6 @@ func (suite *GraphErrorsUnitSuite) TestIsErrIsErrBadJWTToken() { Label(LabelStatus(http.StatusUnauthorized)), expect: assert.False, }, - { - name: "err token expired", - err: clues.Stack(assert.AnError, ErrTokenExpired), - expect: assert.False, - }, { name: "oDataErr code invalid auth token ", err: graphTD.ODataErr(string(invalidAuthenticationToken)), @@ -600,7 +595,8 @@ func (suite *GraphErrorsUnitSuite) TestIsErrIsErrBadJWTToken() { } for _, test := range table { suite.Run(test.name, func() { - test.expect(suite.T(), IsErrBadJWTToken(test.err)) + ode := parseODataErr(test.err) + test.expect(suite.T(), isErrBadJWTToken(ode, test.err)) }) } } @@ -879,45 +875,6 @@ func (suite *GraphErrorsUnitSuite) TestGraphStack_labels() { } } -func (suite *GraphErrorsUnitSuite) TestIsErrItemNotFound() { - table := []struct { - name string - err error - expect assert.BoolAssertionFunc - }{ - { - name: "nil", - err: nil, - expect: assert.False, - }, - { - name: "non-matching", - err: assert.AnError, - expect: assert.False, - }, - { - name: "non-matching oDataErr", - err: graphTD.ODataErr("fnords"), - expect: assert.False, - }, - { - name: "item not found oDataErr", - err: graphTD.ODataErr(string(ItemNotFound)), - expect: assert.True, - }, - { - name: "error item not found oDataErr", - err: graphTD.ODataErr(string(ErrorItemNotFound)), - expect: assert.True, - }, - } - for _, test := range table { - suite.Run(test.name, func() { - test.expect(suite.T(), IsErrItemNotFound(test.err)) - }) - } -} - func (suite *GraphErrorsUnitSuite) TestIsErrResourceLocked() { table := []struct { name string @@ -960,15 +917,11 @@ func (suite *GraphErrorsUnitSuite) TestIsErrResourceLocked() { "deadbeef-7f1e-4578-8215-36004a2c935c Timestamp: 2023-12-05 19:31:01Z"), expect: assert.True, }, - { - name: "matching err sentinel", - err: ErrResourceLocked, - expect: assert.True, - }, } for _, test := range table { suite.Run(test.name, func() { - test.expect(suite.T(), isErrResourceLocked(test.err)) + ode := parseODataErr(test.err) + test.expect(suite.T(), isErrResourceLocked(ode, test.err)) }) } } diff --git a/src/pkg/services/m365/api/graph/service.go b/src/pkg/services/m365/api/graph/service.go index a656b89ad..ed8ccff7b 100644 --- a/src/pkg/services/m365/api/graph/service.go +++ b/src/pkg/services/m365/api/graph/service.go @@ -13,11 +13,13 @@ import ( khttp "github.com/microsoft/kiota-http-go" msgraphsdkgo "github.com/microsoftgraph/msgraph-sdk-go" msgraphgocore "github.com/microsoftgraph/msgraph-sdk-go-core" + "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/common/crash" "github.com/alcionai/corso/src/internal/common/idname" "github.com/alcionai/corso/src/internal/events" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/filters" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/path" @@ -413,7 +415,7 @@ func (aw *adapterWrap) Send( if IsErrConnectionReset(err) || connectionEnded.Compare(err.Error()) { logger.Ctx(ictx).Debug("http connection error") events.Inc(events.APICall, "connectionerror") - } else if IsErrBadJWTToken(err) { + } else if errors.Is(err, core.ErrAuthTokenExpired) { logger.Ctx(ictx).Debug("bad jwt token") events.Inc(events.APICall, "badjwttoken") } else if requestInfo.Method.String() == http.MethodGet && IsErrInvalidRequest(err) { diff --git a/src/pkg/services/m365/api/graph/service_test.go b/src/pkg/services/m365/api/graph/service_test.go index c15ec1be3..56cab3890 100644 --- a/src/pkg/services/m365/api/graph/service_test.go +++ b/src/pkg/services/m365/api/graph/service_test.go @@ -22,6 +22,7 @@ import ( "github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/count" + "github.com/alcionai/corso/src/pkg/errs/core" graphTD "github.com/alcionai/corso/src/pkg/services/m365/api/graph/testdata" ) @@ -313,14 +314,14 @@ func (suite *GraphIntgSuite) TestAdapterWrap_retriesBadJWTToken() { _, err = users. NewItemCalendarsItemEventsDeltaRequestBuilder("https://graph.microsoft.com/fnords/beaux/regard", adpt). Get(ctx, nil) - assert.True(t, IsErrBadJWTToken(err), clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrAuthTokenExpired, clues.ToCore(err)) assert.Equal(t, 4, retryInc, "number of retries") retryInc = 0 // the query doesn't matter _, err = NewService(adpt).Client().Users().Get(ctx, nil) - assert.True(t, IsErrBadJWTToken(err), clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrAuthTokenExpired, clues.ToCore(err)) assert.Equal(t, 4, retryInc, "number of retries") } diff --git a/src/pkg/services/m365/api/groups.go b/src/pkg/services/m365/api/groups.go index 9bdedcb94..992cd104a 100644 --- a/src/pkg/services/m365/api/groups.go +++ b/src/pkg/services/m365/api/groups.go @@ -218,9 +218,9 @@ func getGroupFromResponse(ctx context.Context, resp models.GroupCollectionRespon vs := resp.GetValue() if len(vs) == 0 { - return nil, clues.StackWC(ctx, core.ErrResourceOwnerNotFound) + return nil, clues.StackWC(ctx, core.ErrNotFound) } else if len(vs) > 1 { - return nil, clues.StackWC(ctx, graph.ErrMultipleResultsMatchIdentifier) + return nil, clues.StackWC(ctx, core.ErrMultipleResultsMatchIdentifier) } return vs[0], nil diff --git a/src/pkg/services/m365/api/groups_test.go b/src/pkg/services/m365/api/groups_test.go index 54871bdb3..2cf9da79e 100644 --- a/src/pkg/services/m365/api/groups_test.go +++ b/src/pkg/services/m365/api/groups_test.go @@ -203,7 +203,7 @@ func (suite *GroupsIntgSuite) TestGroups_GetByID() { name: "invalid id", id: uuid.NewString(), expectErr: func(t *testing.T, err error) { - assert.ErrorIs(t, err, core.ErrResourceOwnerNotFound, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrNotFound, clues.ToCore(err)) }, }, { @@ -217,7 +217,7 @@ func (suite *GroupsIntgSuite) TestGroups_GetByID() { name: "invalid displayName", id: "jabberwocky", expectErr: func(t *testing.T, err error) { - assert.ErrorIs(t, err, core.ErrResourceOwnerNotFound, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrNotFound, clues.ToCore(err)) }, }, } diff --git a/src/pkg/services/m365/api/lists.go b/src/pkg/services/m365/api/lists.go index c9a1d18a0..9cdb3e03d 100644 --- a/src/pkg/services/m365/api/lists.go +++ b/src/pkg/services/m365/api/lists.go @@ -105,10 +105,6 @@ func (c Lists) PostDrive( Lists() newList, err := builder.Post(ctx, list, nil) - if graph.IsErrItemAlreadyExistsConflict(err) { - return nil, clues.StackWC(ctx, graph.ErrItemAlreadyExistsConflict, err) - } - if err != nil { return nil, graph.Wrap(ctx, err, "creating documentLibrary list") } diff --git a/src/pkg/services/m365/api/lists_test.go b/src/pkg/services/m365/api/lists_test.go index 040c085c1..6f8c6c71b 100644 --- a/src/pkg/services/m365/api/lists_test.go +++ b/src/pkg/services/m365/api/lists_test.go @@ -18,8 +18,8 @@ import ( "github.com/alcionai/corso/src/internal/tester/tconfig" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/control/testdata" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" - "github.com/alcionai/corso/src/pkg/services/m365/api/graph" graphTD "github.com/alcionai/corso/src/pkg/services/m365/api/graph/testdata" ) @@ -553,7 +553,7 @@ func (suite *ListsAPIIntgSuite) TestLists_PostDrive() { // second post, same name, should error on name conflict] _, err = acl.PostDrive(ctx, siteID, driveName) - require.ErrorIs(t, err, graph.ErrItemAlreadyExistsConflict, clues.ToCore(err)) + require.ErrorIs(t, err, core.ErrAlreadyExists, clues.ToCore(err)) } func (suite *ListsAPIIntgSuite) TestLists_GetListByID() { diff --git a/src/pkg/services/m365/api/mail.go b/src/pkg/services/m365/api/mail.go index fe98200ee..4b20016ec 100644 --- a/src/pkg/services/m365/api/mail.go +++ b/src/pkg/services/m365/api/mail.go @@ -17,6 +17,7 @@ import ( "github.com/alcionai/corso/src/internal/common/sanitize" "github.com/alcionai/corso/src/pkg/backup/details" "github.com/alcionai/corso/src/pkg/dttm" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" @@ -171,7 +172,7 @@ func (c Mail) GetContainerByName( // Return an error if multiple container exist (unlikely) or if no container // is found. if len(gv) != 1 { - return nil, clues.StackWC(ctx, graph.ErrMultipleResultsMatchIdentifier). + return nil, clues.StackWC(ctx, core.ErrMultipleResultsMatchIdentifier). With("returned_container_count", len(gv)) } diff --git a/src/pkg/services/m365/api/pagers/pagers_test.go b/src/pkg/services/m365/api/pagers/pagers_test.go index e8ea0db87..0cc85bc62 100644 --- a/src/pkg/services/m365/api/pagers/pagers_test.go +++ b/src/pkg/services/m365/api/pagers/pagers_test.go @@ -14,6 +14,7 @@ import ( "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/tester" + "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" graphTD "github.com/alcionai/corso/src/pkg/services/m365/api/graph/testdata" ) @@ -635,7 +636,7 @@ func (suite *PagerUnitSuite) TestGetAddedAndRemovedItemIDs() { t: t, pages: []pageResult{ { - err: graph.ErrDeletedInFlight, + err: core.ErrNotFound, }, }, validModTimes: validModTimes, diff --git a/src/pkg/services/m365/api/sites.go b/src/pkg/services/m365/api/sites.go index 6f4501656..b5af1114a 100644 --- a/src/pkg/services/m365/api/sites.go +++ b/src/pkg/services/m365/api/sites.go @@ -13,7 +13,6 @@ import ( "github.com/microsoftgraph/msgraph-sdk-go/sites" "github.com/alcionai/corso/src/internal/common/ptr" - "github.com/alcionai/corso/src/pkg/errs/core" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/services/m365/api/graph" ) @@ -150,13 +149,6 @@ func (c Sites) GetByID( Get(ctx, options) if err != nil { err := graph.Wrap(ctx, err, "getting site by id") - - // a 404 when getting sites by ID returns an itemNotFound - // error code, instead of something more sensible. - if graph.IsErrItemNotFound(err) { - err = clues.Stack(core.ErrResourceOwnerNotFound, err) - } - return nil, err } @@ -192,13 +184,6 @@ func (c Sites) GetByID( Get(ctx, nil) if err != nil { err := graph.Wrap(ctx, err, "getting site by weburl") - - // a 404 when getting sites by ID returns an itemNotFound - // error code, instead of something more sensible. - if graph.IsErrItemNotFound(err) { - err = clues.Stack(core.ErrResourceOwnerNotFound, err) - } - return nil, err } diff --git a/src/pkg/services/m365/api/sites_test.go b/src/pkg/services/m365/api/sites_test.go index aa42c60c3..8f823a8ea 100644 --- a/src/pkg/services/m365/api/sites_test.go +++ b/src/pkg/services/m365/api/sites_test.go @@ -181,7 +181,7 @@ func (suite *SitesIntgSuite) TestSites_GetByID() { name: "random id", id: uuid.NewString() + "," + uuid.NewString(), expectErr: func(t *testing.T, err error) bool { - assert.ErrorIs(t, err, core.ErrResourceOwnerNotFound, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrNotFound, clues.ToCore(err)) return true }, }, @@ -213,7 +213,7 @@ func (suite *SitesIntgSuite) TestSites_GetByID() { name: "well formed url, no sites match", id: modifiedSiteURL, expectErr: func(t *testing.T, err error) bool { - assert.ErrorIs(t, err, core.ErrResourceOwnerNotFound, clues.ToCore(err)) + assert.ErrorIs(t, err, core.ErrNotFound, clues.ToCore(err)) return true }, }, diff --git a/src/pkg/services/m365/api/users.go b/src/pkg/services/m365/api/users.go index 41cce7f93..cf23a18c1 100644 --- a/src/pkg/services/m365/api/users.go +++ b/src/pkg/services/m365/api/users.go @@ -194,11 +194,13 @@ func EvaluateMailboxError(err error) error { } // must occur before MailFolderNotFound, due to overlapping cases. - if errors.Is(err, core.ErrResourceOwnerNotFound) || errors.Is(err, core.ErrResourceNotAccessible) { + if errors.Is(err, core.ErrResourceNotAccessible) { return err } - if graph.IsErrExchangeMailFolderNotFound(err) || graph.IsErrAuthenticationError(err) { + if errors.Is(err, core.ErrNotFound) || + graph.IsErrExchangeMailFolderNotFound(err) || + graph.IsErrAuthenticationError(err) { return nil } diff --git a/src/pkg/services/m365/api/users_test.go b/src/pkg/services/m365/api/users_test.go index d35eb78e5..20c9d9b6d 100644 --- a/src/pkg/services/m365/api/users_test.go +++ b/src/pkg/services/m365/api/users_test.go @@ -81,9 +81,9 @@ func (suite *UsersUnitSuite) TestEvaluateMailboxError() { }, { name: "mail inbox err - user not found", - err: core.ErrResourceOwnerNotFound, + err: core.ErrNotFound, expect: func(t *testing.T, err error) { - assert.ErrorIs(t, err, core.ErrResourceOwnerNotFound, clues.ToCore(err)) + assert.NoError(t, err, clues.ToCore(err)) }, }, { diff --git a/src/pkg/services/m365/groups_test.go b/src/pkg/services/m365/groups_test.go index 643ea4dec..1e0eb5e21 100644 --- a/src/pkg/services/m365/groups_test.go +++ b/src/pkg/services/m365/groups_test.go @@ -108,8 +108,8 @@ func (suite *GroupsIntgSuite) TestGroupByID_notFound() { group, err := suite.cli.GroupByID(ctx, uuid.NewString()) require.Nil(t, group) - require.ErrorIs(t, err, core.ErrResourceOwnerNotFound, clues.ToCore(err)) - require.True(t, errs.Is(err, core.ErrResourceOwnerNotFound)) + require.ErrorIs(t, err, core.ErrNotFound, clues.ToCore(err)) + require.True(t, errs.Is(err, core.ErrNotFound)) } func (suite *GroupsIntgSuite) TestGroups() { diff --git a/src/pkg/services/m365/users_test.go b/src/pkg/services/m365/users_test.go index 2d0d3ab53..6d17a999a 100644 --- a/src/pkg/services/m365/users_test.go +++ b/src/pkg/services/m365/users_test.go @@ -175,13 +175,14 @@ func (suite *userIntegrationSuite) TestUserGetMailboxInfo() { name: "invalid user", user: uuid.NewString(), expect: func(t *testing.T, info api.MailboxInfo) { - mi := api.MailboxInfo{ - ErrGetMailBoxSetting: []error{}, - } - - require.Equal(t, mi, info) + require.NotNil(t, info) + assert.Contains(t, info.ErrGetMailBoxSetting, api.ErrMailBoxNotFound) }, - expectErr: require.Error, + // may seem odd, but we assume the user themselves + // has already been vetted, which turns this into a + // notFound error in the same way a mailboxNotFound + // is handled. + expectErr: require.NoError, }, } for _, test := range table {