catch basic notFound graph errors (#5052)

make the NotFound error comparison more lenient so that it catches the broader set of not-found error responses from graph.

---

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

- [x]  No

#### Type of change

- [x] 🐛 Bugfix

#### Test Plan

- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2024-01-17 15:57:15 -07:00 committed by GitHub
parent b8b1299514
commit 232ebba13a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 57 additions and 21 deletions

View File

@ -66,6 +66,7 @@ const (
// @microsoft.graph.conflictBehavior=fail finds a conflicting file.
nameAlreadyExists errorCode = "nameAlreadyExists"
NotAllowed errorCode = "notAllowed"
notFound errorCode = "NotFound"
noResolvedUsers errorCode = "noResolvedUsers"
QuotaExceeded errorCode = "ErrorQuotaExceeded"
RequestResourceNotFound errorCode = "Request_ResourceNotFound"
@ -128,10 +129,10 @@ func stackWithCoreErr(ctx context.Context, err error, traceDepth int) error {
err = clues.Stack(core.ErrResourceNotAccessible, 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)
case isErrNotFound(ode, err):
err = clues.Stack(core.ErrNotFound, err)
}
return stackWithDepth(ctx, err, 1+traceDepth)
@ -140,7 +141,8 @@ func stackWithCoreErr(ctx context.Context, err error, traceDepth int) error {
// unexported categorizers, for use with stackWithCoreErr
func isErrApplicationThrottled(ode oDataErr, err error) bool {
return ode.hasErrorCode(err, ApplicationThrottled)
return ode.hasErrorCode(err, ApplicationThrottled) ||
ode.hasResponseCode(err, http.StatusTooManyRequests)
}
func isErrInsufficientAuthorization(ode oDataErr, err error) bool {
@ -149,11 +151,13 @@ func isErrInsufficientAuthorization(ode oDataErr, err error) bool {
func isErrNotFound(ode oDataErr, err error) bool {
return clues.HasLabel(err, LabelStatus(http.StatusNotFound)) ||
ode.hasResponseCode(err, http.StatusNotFound) ||
ode.hasErrorCode(
err,
ErrorItemNotFound,
ItemNotFound,
syncFolderNotFound)
syncFolderNotFound,
notFound)
}
func isErrUserNotFound(ode oDataErr, err error) bool {
@ -177,8 +181,7 @@ func isErrItemAlreadyExists(ode oDataErr, err error) bool {
}
func isErrResourceLocked(ode oDataErr, err error) bool {
return ode.hasInnerErrorCode(err, ResourceLocked) ||
ode.hasErrorCode(err, NotAllowed) ||
return ode.hasErrorCode(err, ResourceLocked, NotAllowed) ||
ode.errMessageMatchesAllFilters(
err,
filters.In([]string{"the service principal for resource"}),
@ -285,7 +288,7 @@ func IsErrSiteNotFound(err error) bool {
}
func IsErrSharingDisabled(err error) bool {
return parseODataErr(err).hasInnerErrorCode(err, sharingDisabled)
return parseODataErr(err).hasErrorCode(err, sharingDisabled)
}
// ---------------------------------------------------------------------------
@ -577,20 +580,9 @@ func (ode oDataErr) hasErrorCode(err error, codes ...errorCode) bool {
cs[i] = string(c)
}
return filters.Equal(cs).Compare(ode.Main.Code)
}
eq := filters.Equal(cs)
func (ode oDataErr) hasInnerErrorCode(err error, codes ...errorCode) bool {
if !ode.isODataErr {
return false
}
cs := make([]string, len(codes))
for i, c := range codes {
cs[i] = string(c)
}
return filters.Equal(cs).Compare(ode.Inner.Code)
return eq.CompareAny(ode.Main.Code, ode.Inner.Code)
}
// only use this as a last resort. Prefer the code or statuscode if possible.

View File

@ -83,6 +83,11 @@ func (suite *GraphErrorsUnitSuite) TestIsErrApplicationThrottled() {
err: graphTD.ODataErr(string(ApplicationThrottled)),
expect: assert.True,
},
{
name: "too many requests resp status",
err: graphTD.ODataErrWithStatus(http.StatusTooManyRequests, "err"),
expect: assert.True,
},
}
for _, test := range table {
suite.Run(test.name, func() {
@ -165,6 +170,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrNotFound() {
table := []struct {
name string
err error
inner error
expect assert.BoolAssertionFunc
}{
{
@ -180,23 +186,49 @@ func (suite *GraphErrorsUnitSuite) TestIsErrNotFound() {
{
name: "non-matching oDataErr",
err: graphTD.ODataErr("fnords"),
inner: graphTD.ODataInner("fnords"),
expect: assert.False,
},
{
name: "not-found oDataErr",
err: graphTD.ODataErr(string(notFound)),
inner: graphTD.ODataInner(string(notFound)),
expect: assert.True,
},
{
name: "error item not-found oDataErr",
err: graphTD.ODataErr(string(ErrorItemNotFound)),
inner: graphTD.ODataInner(string(ErrorItemNotFound)),
expect: assert.True,
},
{
name: "item not-found oDataErr",
err: graphTD.ODataErr(string(ItemNotFound)),
inner: graphTD.ODataInner(string(ItemNotFound)),
expect: assert.True,
},
{
name: "sync-not-found oDataErr",
err: graphTD.ODataErr(string(syncFolderNotFound)),
inner: graphTD.ODataInner(string(syncFolderNotFound)),
expect: assert.True,
},
{
name: "not found resp status",
err: graphTD.ODataErrWithStatus(http.StatusNotFound, "err"),
expect: assert.True,
},
}
for _, test := range table {
suite.Run(test.name, func() {
t := suite.T()
ode := parseODataErr(test.err)
test.expect(suite.T(), isErrNotFound(ode, test.err))
test.expect(t, isErrNotFound(ode, test.err))
if test.inner != nil {
test.expect(t, isErrNotFound(ode, test.inner))
}
})
}
}

View File

@ -36,6 +36,18 @@ func ODataErrWithMsg(code, message string) *odataerrors.ODataError {
return odErr
}
func ODataErrWithStatus(status int, code string) *odataerrors.ODataError {
odErr := odataerrors.NewODataError()
merr := odataerrors.NewMainError()
merr.SetCode(&code)
// graph sdk expects the message to be available
merr.SetMessage(&code)
odErr.SetErrorEscaped(merr)
odErr.SetStatusCode(status)
return odErr
}
func ODataInner(innerCode string) *odataerrors.ODataError {
odErr := odataerrors.NewODataError()
inerr := odataerrors.NewInnerError()