add extra catches for resource locked (#4499)

Adds two layers of extra catches for resoruceLocked errors.  First, in pkg/errs, adds error check funcs to ensure the source comparators are checked even if
they somehow got skipped in the lower layer packages.

Second, adds checks for resource locked throughout sites, users, and groups in the api layer.

---

#### Does this PR need a docs update or release note?

- [x] ⛔ No

#### Type of change

- [x] 🧹 Tech Debt/Cleanup

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

src/pkg/errs/errs.go

@@ -24,7 +24,7 @@ const (
 // map of enums to errors.  We might want to re-use an enum for multiple
 // internal errors (ex: "ServiceNotEnabled" may exist in both graph and
 // non-graph producers).
-var internalToExternal = map[errEnum][]error{
+var externalToInternal = map[errEnum][]error{
 	ApplicationThrottled:  {graph.ErrApplicationThrottled},
 	BackupNotFound:        {repository.ErrorBackupNotFound},
 	RepoAlreadyExists:     {repository.ErrorRepoAlreadyExists},
@@ -33,22 +33,43 @@ var internalToExternal = map[errEnum][]error{
 	ServiceNotEnabled:     {graph.ErrServiceNotEnabled},
 }
 
-// Internal returns the internal errors which match to the public error category.
-func Internal(enum errEnum) []error {
-	return internalToExternal[enum]
+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[errEnum][]ErrCheck{
+	ApplicationThrottled:  {graph.IsErrApplicationThrottled},
+	ResourceNotAccessible: {graph.IsErrResourceLocked},
+	ResourceOwnerNotFound: {graph.IsErrItemNotFound},
+}
+
+// Internal returns the internal errors and error checking functions which
+// match to the public error enum.
+func Internal(enum errEnum) ([]error, []ErrCheck) {
+	return externalToInternal[enum], externalToInternalCheck[enum]
 }
 
 // Is checks if the provided error contains an internal error that matches
 // the public error category.
 func Is(err error, enum errEnum) bool {
-	internalErrs, ok := internalToExternal[enum]
-	if !ok {
-		return false
+	internalErrs, ok := externalToInternal[enum]
+	if ok {
+		for _, target := range internalErrs {
+			if errors.Is(err, target) {
+				return true
+			}
+		}
 	}
 
-	for _, target := range internalErrs {
-		if errors.Is(err, target) {
-			return true
+	internalChecks, ok := externalToInternalCheck[enum]
+	if ok {
+		for _, check := range internalChecks {
+			if check(err) {
+				return true
+			}
 		}
 	}
 

src/pkg/errs/errs_test.go

@@ -20,20 +20,108 @@ func TestErrUnitSuite(t *testing.T) {
 	suite.Run(t, &ErrUnitSuite{Suite: tester.NewUnitSuite(t)})
 }
 
-func (suite *ErrUnitSuite) TestInternal() {
+func (suite *ErrUnitSuite) TestInternal_errs() {
 	table := []struct {
 		get    errEnum
 		expect []error
 	}{
-		{RepoAlreadyExists, []error{repository.ErrorRepoAlreadyExists}},
-		{BackupNotFound, []error{repository.ErrorBackupNotFound}},
-		{ServiceNotEnabled, []error{graph.ErrServiceNotEnabled}},
-		{ResourceOwnerNotFound, []error{graph.ErrResourceOwnerNotFound}},
-		{ResourceNotAccessible, []error{graph.ErrResourceLocked}},
+		{
+			get:    ApplicationThrottled,
+			expect: []error{graph.ErrApplicationThrottled},
+		},
+		{
+			get:    RepoAlreadyExists,
+			expect: []error{repository.ErrorRepoAlreadyExists},
+		},
+		{
+			get:    BackupNotFound,
+			expect: []error{repository.ErrorBackupNotFound},
+		},
+		{
+			get:    ServiceNotEnabled,
+			expect: []error{graph.ErrServiceNotEnabled},
+		},
+		{
+			get:    ResourceOwnerNotFound,
+			expect: []error{graph.ErrResourceOwnerNotFound},
+		},
+		{
+			get:    ResourceNotAccessible,
+			expect: []error{graph.ErrResourceLocked},
+		},
 	}
 	for _, test := range table {
 		suite.Run(string(test.get), func() {
-			assert.ElementsMatch(suite.T(), test.expect, Internal(test.get))
+			// can't compare func signatures
+			errs, _ := Internal(test.get)
+			assert.ElementsMatch(suite.T(), test.expect, errs)
+		})
+	}
+}
+
+func (suite *ErrUnitSuite) TestInternal_checks() {
+	table := []struct {
+		get             errEnum
+		err             error
+		expectHasChecks assert.ValueAssertionFunc
+		expect          assert.BoolAssertionFunc
+	}{
+		{
+			get:             ApplicationThrottled,
+			err:             graph.ErrApplicationThrottled,
+			expectHasChecks: assert.NotEmpty,
+			expect:          assert.True,
+		},
+		{
+			get:             RepoAlreadyExists,
+			err:             graph.ErrApplicationThrottled,
+			expectHasChecks: assert.Empty,
+			expect:          assert.False,
+		},
+		{
+			get:             BackupNotFound,
+			err:             repository.ErrorBackupNotFound,
+			expectHasChecks: assert.Empty,
+			expect:          assert.False,
+		},
+		{
+			get:             ServiceNotEnabled,
+			err:             graph.ErrServiceNotEnabled,
+			expectHasChecks: assert.Empty,
+			expect:          assert.False,
+		},
+		{
+			get: ResourceOwnerNotFound,
+			// won't match, checks itemNotFound, which isn't an error enum
+			err:             graph.ErrResourceOwnerNotFound,
+			expectHasChecks: assert.NotEmpty,
+			expect:          assert.False,
+		},
+		{
+			get:             ResourceNotAccessible,
+			err:             graph.ErrResourceLocked,
+			expectHasChecks: assert.NotEmpty,
+			expect:          assert.True,
+		},
+	}
+	for _, test := range table {
+		suite.Run(string(test.get), 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)
 		})
 	}
 }
@@ -43,11 +131,30 @@ func (suite *ErrUnitSuite) TestIs() {
 		target errEnum
 		err    error
 	}{
-		{RepoAlreadyExists, repository.ErrorRepoAlreadyExists},
-		{BackupNotFound, repository.ErrorBackupNotFound},
-		{ServiceNotEnabled, graph.ErrServiceNotEnabled},
-		{ResourceOwnerNotFound, graph.ErrResourceOwnerNotFound},
-		{ResourceNotAccessible, graph.ErrResourceLocked},
+		{
+			target: ApplicationThrottled,
+			err:    graph.ErrApplicationThrottled,
+		},
+		{
+			target: RepoAlreadyExists,
+			err:    repository.ErrorRepoAlreadyExists,
+		},
+		{
+			target: BackupNotFound,
+			err:    repository.ErrorBackupNotFound,
+		},
+		{
+			target: ServiceNotEnabled,
+			err:    graph.ErrServiceNotEnabled,
+		},
+		{
+			target: ResourceOwnerNotFound,
+			err:    graph.ErrResourceOwnerNotFound,
+		},
+		{
+			target: ResourceNotAccessible,
+			err:    graph.ErrResourceLocked,
+		},
 	}
 	for _, test := range table {
 		suite.Run(string(test.target), func() {

src/pkg/services/m365/api/groups.go

@@ -128,6 +128,10 @@ func (c Groups) GetByID(
 			return group, nil
 		}
 
+		if graph.IsErrResourceLocked(err) {
+			return nil, graph.Stack(ctx, clues.Stack(graph.ErrResourceLocked, err))
+		}
+
 		logger.CtxErr(ctx, err).Info("finding group by id, falling back to display name")
 	}
 
@@ -140,6 +144,10 @@ func (c Groups) GetByID(
 
 	resp, err := service.Client().Groups().Get(ctx, opts)
 	if err != nil {
+		if graph.IsErrResourceLocked(err) {
+			err = clues.Stack(graph.ErrResourceLocked, err)
+		}
+
 		return nil, graph.Wrap(ctx, err, "finding group by display name")
 	}
 

src/pkg/services/m365/api/sites.go

@@ -158,6 +158,10 @@ func (c Sites) GetByID(
 				err = clues.Stack(graph.ErrResourceOwnerNotFound, err)
 			}
 
+			if graph.IsErrResourceLocked(err) {
+				err = clues.Stack(graph.ErrResourceLocked, err)
+			}
+
 			return nil, err
 		}
 
@@ -200,6 +204,10 @@ func (c Sites) GetByID(
 			err = clues.Stack(graph.ErrResourceOwnerNotFound, err)
 		}
 
+		if graph.IsErrResourceLocked(err) {
+			return nil, graph.Stack(ctx, clues.Stack(graph.ErrResourceLocked, err))
+		}
+
 		return nil, err
 	}
 

src/pkg/services/m365/api/users.go

@@ -121,9 +121,12 @@ func (c Users) GetByID(ctx context.Context, identifier string) (models.Userable,
 	)
 
 	resp, err = c.Stable.Client().Users().ByUserId(identifier).Get(ctx, nil)
-
 	if err != nil {
-		return nil, graph.Wrap(ctx, err, "getting user")
+		if graph.IsErrResourceLocked(err) {
+			err = clues.Stack(graph.ErrResourceLocked, err)
+		}
+
+		return nil, graph.Stack(ctx, err)
 	}
 
 	return resp, err