Skip to content
This repository has been archived by the owner on Oct 11, 2024. It is now read-only.

Commit

Permalink
fix up naming, comments
Browse files Browse the repository at this point in the history
ryanfkeepers committed Feb 15, 2024
1 parent 5b7d155 commit 9369bc3
Showing 6 changed files with 21 additions and 16 deletions.
4 changes: 2 additions & 2 deletions src/internal/m365/service/exchange/enabled.go
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ func IsServiceEnabled(
return false, clues.Stack(err)
}

logger.Ctx(ctx).Info("resource owner does not have a mailbox enabled")
logger.CtxErr(ctx, err).Info("resource owner does not have a mailbox enabled")

return false, nil
}
@@ -61,7 +61,7 @@ func GetMailboxInfo(
return mi, clues.Stack(err)
}

logger.Ctx(ctx).Info("resource owner does not have a mailbox enabled")
logger.CtxErr(ctx, err).Info("resource owner does not have a mailbox enabled")

mi.ErrGetMailBoxSetting = append(
mi.ErrGetMailBoxSetting,
4 changes: 2 additions & 2 deletions src/pkg/errs/core/core.go
Original file line number Diff line number Diff line change
@@ -16,8 +16,8 @@ import "errors"
//
// 2. Maintain coarseness.
// We won't need a core.Err version of every lower-level error. Try, where possible,
// to group concepts into broad categories. Ex: prefer "resource not found" over
// "user not found" or "site not found".
// to group concepts into broad categories. Ex: prefer "not found" over "resource not
// found", and "resource not found" over "user not found".
//
// 3. Always Stack/Wrap core.Errs. Only once.
// `return core.ErrFoo` should be avoided. Also, if you're handling a error returned
14 changes: 9 additions & 5 deletions src/pkg/services/m365/api/graph/errors.go
Original file line number Diff line number Diff line change
@@ -127,7 +127,11 @@ var (
// error categorization
// ---------------------------------------------------------------------------

var ErrUserNotFound = clues.New("user not found")
// ErrResourceNotFound is a special case handler to differentiate from the
// more generic core.ErrNotFound. Two rules for usage, to preserve sanity:
// 1. it should get stacked on top of a clues.NotFound error.
// 2. it should not be investigated above the api layer.
var ErrResourceNotFound = clues.New("resource not found")

func stackWithCoreErr(ctx context.Context, err error, traceDepth int) error {
if err == nil {
@@ -142,11 +146,11 @@ func stackWithCoreErr(ctx context.Context, err error, traceDepth int) error {
err = clues.Stack(core.ErrAuthTokenExpired)
case isErrApplicationThrottled(ode, err):
err = clues.Stack(core.ErrApplicationThrottled, err)
case isErrUserNotFound(ode, err):
// stack both userNotFound and notFound to ensure some graph api
case isErrResourceNotFound(ode, err):
// stack both resourceNotFound and notFound to ensure some graph api
// internals can distinguish between the two cases (where possible).
// layers above the api should still handle only the core notFound.
err = clues.Stack(ErrUserNotFound, core.ErrNotFound, err)
err = clues.Stack(ErrResourceNotFound, core.ErrNotFound, err)
case isErrResourceLocked(ode, err):
err = clues.Stack(core.ErrResourceNotAccessible, err)
case isErrInsufficientAuthorization(ode, err):
@@ -209,7 +213,7 @@ func isErrNotFound(ode oDataErr, err error) bool {
notFound)
}

func isErrUserNotFound(ode oDataErr, err error) bool {
func isErrResourceNotFound(ode oDataErr, err error) bool {
if ode.hasErrorCode(err, RequestResourceNotFound, invalidUser) {
return true
}
4 changes: 2 additions & 2 deletions src/pkg/services/m365/api/graph/errors_test.go
Original file line number Diff line number Diff line change
@@ -549,7 +549,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrUserNotFound() {
for _, test := range table {
suite.Run(test.name, func() {
ode := parseODataErr(test.err)
test.expect(suite.T(), isErrUserNotFound(ode, test.err))
test.expect(suite.T(), isErrResourceNotFound(ode, test.err))
})
}
}
@@ -1176,7 +1176,7 @@ func (suite *GraphErrorsUnitSuite) TestStackWithCoreErr() {
{
name: "user not found",
err: graphTD.ODataErrWithMsg(string(ResourceNotFound), "User not found"),
expect: []error{ErrUserNotFound, core.ErrNotFound},
expect: []error{ErrResourceNotFound, core.ErrNotFound},
},
{
name: "resource locked",
9 changes: 5 additions & 4 deletions src/pkg/services/m365/api/users.go
Original file line number Diff line number Diff line change
@@ -185,12 +185,13 @@ func appendIfErr(errs []error, err error) []error {
return append(errs, err)
}

// IsMailboxErrorIgnorable checks whether the provided error can be interpreted
// as "user does not have a mailbox", or whether it is some other error. If
// the former (no mailbox), returns nil, otherwise returns an error.
// IsMailboxErrorIgnorable checks whether the provided error (which is assumed to
// have originated from a call to retrieve a user's mailbox) can be safely ignored
// or not. In particular, the igorable cases are when the mail folder is not found
// and when an authentication issue occurs.
func IsMailboxErrorIgnorable(err error) bool {
// must occur before MailFolderNotFound, due to overlapping cases.
if errors.Is(err, core.ErrResourceNotAccessible) || errors.Is(err, graph.ErrUserNotFound) {
if errors.Is(err, core.ErrResourceNotAccessible) || errors.Is(err, graph.ErrResourceNotFound) {
return false
}

2 changes: 1 addition & 1 deletion src/pkg/services/m365/api/users_test.go
Original file line number Diff line number Diff line change
@@ -79,7 +79,7 @@ func (suite *UsersUnitSuite) TestEvaluateMailboxError() {
},
{
name: "user not found - corso sentinel",
err: graph.ErrUserNotFound,
err: graph.ErrResourceNotFound,
expect: assert.False,
},
{

0 comments on commit 9369bc3

Please sign in to comment.