Handle broader set of user not found error codes (#3678)
graph has been inconsistent about what error code is returned when the user doesn't exist. Expand the set of things we check so that hopefully we'll get them all Always return an error if the user doesn't exist as determined by checking for exchange folders during GetInfo calls Also add a little more stack trace info to returned errors --- #### Does this PR need a docs update or release note? - [ ] ✅ Yes, it's included - [ ] 🕐 Yes, but in a later PR - [x] ⛔ No #### Type of change - [ ] 🌻 Feature - [x] 🐛 Bugfix - [ ] 🗺️ Documentation - [ ] 🤖 Supportability/Tests - [ ] 💻 CI/Deployment - [ ] 🧹 Tech Debt/Cleanup #### Issue(s) * fixes #3672 #### Test Plan - [x] 💪 Manual - [x] ⚡ Unit test - [ ] 💚 E2E
This commit is contained in:
parent
65ce91592b
commit
4f9793a824
@ -214,7 +214,7 @@ func (suite *DiscoveryIntgSuite) TestUserInfo() {
|
||||
ServicesEnabled: map[path.ServiceType]struct{}{},
|
||||
Mailbox: api.MailboxInfo{},
|
||||
},
|
||||
expectErr: require.NoError,
|
||||
expectErr: require.Error,
|
||||
},
|
||||
}
|
||||
for _, test := range table {
|
||||
|
||||
@ -38,11 +38,15 @@ const (
|
||||
nameAlreadyExists errorCode = "nameAlreadyExists"
|
||||
quotaExceeded errorCode = "ErrorQuotaExceeded"
|
||||
RequestResourceNotFound errorCode = "Request_ResourceNotFound"
|
||||
resourceNotFound errorCode = "ResourceNotFound"
|
||||
resyncRequired errorCode = "ResyncRequired" // alt: resyncRequired
|
||||
syncFolderNotFound errorCode = "ErrorSyncFolderNotFound"
|
||||
syncStateInvalid errorCode = "SyncStateInvalid"
|
||||
syncStateNotFound errorCode = "SyncStateNotFound"
|
||||
// Returned when we try to get the inbox of a user that doesn't exist.
|
||||
resourceNotFound errorCode = "ResourceNotFound"
|
||||
// Some datacenters are returning this when we try to get the inbox of a user
|
||||
// that doesn't exist.
|
||||
invalidUser errorCode = "ErrorInvalidUser"
|
||||
resyncRequired errorCode = "ResyncRequired"
|
||||
syncFolderNotFound errorCode = "ErrorSyncFolderNotFound"
|
||||
syncStateInvalid errorCode = "SyncStateInvalid"
|
||||
syncStateNotFound errorCode = "SyncStateNotFound"
|
||||
// This error occurs when an attempt is made to create a folder that has
|
||||
// the same name as another folder in the same parent. Such duplicate folder
|
||||
// names are not allowed by graph.
|
||||
@ -131,7 +135,22 @@ func IsErrExchangeMailFolderNotFound(err error) bool {
|
||||
}
|
||||
|
||||
func IsErrUserNotFound(err error) bool {
|
||||
return hasErrorCode(err, RequestResourceNotFound)
|
||||
if hasErrorCode(err, RequestResourceNotFound, invalidUser) {
|
||||
return true
|
||||
}
|
||||
|
||||
if hasErrorCode(err, resourceNotFound) {
|
||||
var odErr odataerrors.ODataErrorable
|
||||
if !errors.As(err, &odErr) {
|
||||
return false
|
||||
}
|
||||
|
||||
mainMsg, _, _ := errData(odErr)
|
||||
|
||||
return strings.Contains(strings.ToLower(mainMsg), "user")
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
func IsErrResourceNotFound(err error) bool {
|
||||
|
||||
@ -12,6 +12,7 @@ import (
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/suite"
|
||||
|
||||
"github.com/alcionai/corso/src/internal/common/ptr"
|
||||
"github.com/alcionai/corso/src/internal/tester"
|
||||
"github.com/alcionai/corso/src/pkg/fault"
|
||||
)
|
||||
@ -231,11 +232,36 @@ func (suite *GraphErrorsUnitSuite) TestIsErrUserNotFound() {
|
||||
err: odErr("fnords"),
|
||||
expect: assert.False,
|
||||
},
|
||||
{
|
||||
name: "non-matching resource not found",
|
||||
err: func() error {
|
||||
res := odErr(string(resourceNotFound))
|
||||
res.GetError().SetMessage(ptr.To("Calendar not found"))
|
||||
|
||||
return res
|
||||
}(),
|
||||
expect: assert.False,
|
||||
},
|
||||
{
|
||||
name: "request resource not found oDataErr",
|
||||
err: odErr(string(RequestResourceNotFound)),
|
||||
expect: assert.True,
|
||||
},
|
||||
{
|
||||
name: "invalid user oDataErr",
|
||||
err: odErr(string(invalidUser)),
|
||||
expect: assert.True,
|
||||
},
|
||||
{
|
||||
name: "resource not found oDataErr",
|
||||
err: func() error {
|
||||
res := odErr(string(resourceNotFound))
|
||||
res.GetError().SetMessage(ptr.To("User not found"))
|
||||
|
||||
return res
|
||||
}(),
|
||||
expect: assert.True,
|
||||
},
|
||||
}
|
||||
for _, test := range table {
|
||||
suite.Run(test.name, func() {
|
||||
|
||||
@ -200,12 +200,14 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
|
||||
|
||||
if graph.IsErrUserNotFound(err) {
|
||||
logger.CtxErr(ctx, err).Error("user not found")
|
||||
return nil, clues.Stack(graph.ErrResourceOwnerNotFound, err)
|
||||
// FIXME(ashmrtn): This shouldn't need clues.Wrap.
|
||||
return nil, clues.Wrap(clues.Stack(graph.ErrResourceOwnerNotFound, err), "")
|
||||
}
|
||||
|
||||
if !graph.IsErrExchangeMailFolderNotFound(err) {
|
||||
logger.CtxErr(ctx, err).Error("getting user's mail folder")
|
||||
return nil, err
|
||||
// FIXME(ashmrtn): This should use clues.Stack.
|
||||
return nil, clues.Wrap(err, "")
|
||||
}
|
||||
|
||||
logger.Ctx(ctx).Info("resource owner does not have a mailbox enabled")
|
||||
@ -243,7 +245,8 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
|
||||
err = c.getFirstInboxMessage(ctx, userID, ptr.Val(inbx.GetId()))
|
||||
if err != nil {
|
||||
if !graph.IsErrQuotaExceeded(err) {
|
||||
return nil, err
|
||||
// FIXME(ashmrtn): This should use clues.Stack.
|
||||
return nil, clues.Wrap(err, "")
|
||||
}
|
||||
|
||||
userInfo.Mailbox.QuotaExceeded = graph.IsErrQuotaExceeded(err)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user