fix up naming, comments

This commit is contained in:
ryanfkeepers 2024-01-30 17:28:59 -07:00
parent 5b7d1559ca
commit 9369bc36c8
6 changed files with 21 additions and 16 deletions

View File

@ -29,7 +29,7 @@ func IsServiceEnabled(
return false, clues.Stack(err) 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 return false, nil
} }
@ -61,7 +61,7 @@ func GetMailboxInfo(
return mi, clues.Stack(err) 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 = append(
mi.ErrGetMailBoxSetting, mi.ErrGetMailBoxSetting,

View File

@ -16,8 +16,8 @@ import "errors"
// //
// 2. Maintain coarseness. // 2. Maintain coarseness.
// We won't need a core.Err version of every lower-level error. Try, where possible, // 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 // to group concepts into broad categories. Ex: prefer "not found" over "resource not
// "user not found" or "site not found". // found", and "resource not found" over "user not found".
// //
// 3. Always Stack/Wrap core.Errs. Only once. // 3. Always Stack/Wrap core.Errs. Only once.
// `return core.ErrFoo` should be avoided. Also, if you're handling a error returned // `return core.ErrFoo` should be avoided. Also, if you're handling a error returned

View File

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

View File

@ -549,7 +549,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrUserNotFound() {
for _, test := range table { for _, test := range table {
suite.Run(test.name, func() { suite.Run(test.name, func() {
ode := parseODataErr(test.err) 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", name: "user not found",
err: graphTD.ODataErrWithMsg(string(ResourceNotFound), "User not found"), err: graphTD.ODataErrWithMsg(string(ResourceNotFound), "User not found"),
expect: []error{ErrUserNotFound, core.ErrNotFound}, expect: []error{ErrResourceNotFound, core.ErrNotFound},
}, },
{ {
name: "resource locked", name: "resource locked",

View File

@ -185,12 +185,13 @@ func appendIfErr(errs []error, err error) []error {
return append(errs, err) return append(errs, err)
} }
// IsMailboxErrorIgnorable checks whether the provided error can be interpreted // IsMailboxErrorIgnorable checks whether the provided error (which is assumed to
// as "user does not have a mailbox", or whether it is some other error. If // have originated from a call to retrieve a user's mailbox) can be safely ignored
// the former (no mailbox), returns nil, otherwise returns an error. // 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 { func IsMailboxErrorIgnorable(err error) bool {
// must occur before MailFolderNotFound, due to overlapping cases. // 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 return false
} }

View File

@ -79,7 +79,7 @@ func (suite *UsersUnitSuite) TestEvaluateMailboxError() {
}, },
{ {
name: "user not found - corso sentinel", name: "user not found - corso sentinel",
err: graph.ErrUserNotFound, err: graph.ErrResourceNotFound,
expect: assert.False, expect: assert.False,
}, },
{ {