From dcf02f62569bdb0838a48e4e2049d3b810033514 Mon Sep 17 00:00:00 2001 From: ashmrtn <3891298+ashmrtn@users.noreply.github.com> Date: Wed, 28 Jun 2023 14:21:28 -0700 Subject: [PATCH] Minor refactoring of graph error checking code (#3686) Minor code cleanup to error checking and calling code: - Remove unused function - Update error check for folder not found - Remove duplicate check in SDK function --- #### Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [ ] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [x] :broom: Tech Debt/Cleanup #### Issue(s) * fixes #3684 #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/m365/graph/errors.go | 8 +++----- src/internal/m365/graph/errors_test.go | 20 +++++++++++++++++--- src/pkg/services/m365/m365.go | 4 ---- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/internal/m365/graph/errors.go b/src/internal/m365/graph/errors.go index 49bf5b0dd..4c3f8a3cb 100644 --- a/src/internal/m365/graph/errors.go +++ b/src/internal/m365/graph/errors.go @@ -131,7 +131,9 @@ func IsErrQuotaExceeded(err error) bool { } func IsErrExchangeMailFolderNotFound(err error) bool { - return hasErrorCode(err, ResourceNotFound, MailboxNotEnabledForRESTAPI) + // Not sure if we can actually see a resourceNotFound error here. I've only + // seen the latter two. + return hasErrorCode(err, ResourceNotFound, itemNotFound, MailboxNotEnabledForRESTAPI) } func IsErrUserNotFound(err error) bool { @@ -153,10 +155,6 @@ func IsErrUserNotFound(err error) bool { return false } -func IsErrResourceNotFound(err error) bool { - return hasErrorCode(err, ResourceNotFound) -} - func IsErrCannotOpenFileAttachment(err error) bool { return hasErrorCode(err, cannotOpenFileAttachment) } diff --git a/src/internal/m365/graph/errors_test.go b/src/internal/m365/graph/errors_test.go index 90ec23502..72daef7a4 100644 --- a/src/internal/m365/graph/errors_test.go +++ b/src/internal/m365/graph/errors_test.go @@ -255,13 +255,27 @@ func (suite *GraphErrorsUnitSuite) TestIsErrUserNotFound() { { name: "resource not found oDataErr", err: func() error { - res := odErr(string(ResourceNotFound)) - res.GetError().SetMessage(ptr.To("User not found")) - + res := odErrMsg(string(ResourceNotFound), "User not found") return res }(), expect: assert.True, }, + { + name: "resource not found oDataErr wrapped", + err: func() error { + res := odErrMsg(string(ResourceNotFound), "User not found") + return clues.Wrap(res, "getting mail folder") + }(), + expect: assert.True, + }, + { + name: "resource not found oDataErr stacked", + err: func() error { + res := odErrMsg(string(ResourceNotFound), "User not found") + return clues.Stack(res, assert.AnError) + }(), + expect: assert.True, + }, } for _, test := range table { suite.Run(test.name, func() { diff --git a/src/pkg/services/m365/m365.go b/src/pkg/services/m365/m365.go index ffffd3625..64846b81e 100644 --- a/src/pkg/services/m365/m365.go +++ b/src/pkg/services/m365/m365.go @@ -90,10 +90,6 @@ func UserHasMailbox(ctx context.Context, acct account.Account, userID string) (b return false, clues.Stack(graph.ErrResourceOwnerNotFound, err) } - if graph.IsErrExchangeMailFolderNotFound(err) { - return false, nil - } - return false, clues.Stack(err) }