clean up graph errors (#2286)

## Description

The current interface makes no sense.  This
change refactors the funcs to standard boolean
comparators.

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

- [x]  No 

## Type of change

- [x] 🧹 Tech Debt/Cleanup
This commit is contained in:
Keepers 2023-01-26 21:24:06 -07:00 committed by GitHub
parent 06efad7028
commit ce2c1ffbb3
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 44 additions and 101 deletions

View File

@ -215,7 +215,7 @@ func (c Contacts) GetAddedAndRemovedItemIDs(
}
// only return on error if it is NOT a delta issue.
// on bad deltas we retry the call with the regular builder
if graph.IsErrInvalidDelta(err) == nil {
if !graph.IsErrInvalidDelta(err) {
return nil, nil, DeltaUpdate{}, err
}

View File

@ -216,7 +216,7 @@ func (c Events) GetAddedAndRemovedItemIDs(
}
// only return on error if it is NOT a delta issue.
// on bad deltas we retry the call with the regular builder
if graph.IsErrInvalidDelta(err) == nil {
if !graph.IsErrInvalidDelta(err) {
return nil, nil, DeltaUpdate{}, err
}

View File

@ -215,7 +215,7 @@ func (c Mail) GetAddedAndRemovedItemIDs(
}
// only return on error if it is NOT a delta issue.
// on bad deltas we retry the call with the regular builder
if graph.IsErrInvalidDelta(err) == nil {
if !graph.IsErrInvalidDelta(err) {
return nil, nil, DeltaUpdate{}, err
}

View File

@ -72,11 +72,7 @@ func getItemsAddedAndRemovedFromContainer(
// get the next page of data, check for standard errors
resp, err := pager.getPage(ctx)
if err != nil {
if err := graph.IsErrDeletedInFlight(err); err != nil {
return nil, nil, deltaURL, err
}
if err := graph.IsErrInvalidDelta(err); err != nil {
if graph.IsErrDeletedInFlight(err) || graph.IsErrInvalidDelta(err) {
return nil, nil, deltaURL, err
}

View File

@ -260,17 +260,13 @@ func (col *Collection) streamItems(ctx context.Context) {
// If the data is no longer available just return here and chalk it up
// as a success. There's no reason to retry and no way we can backup up
// enough information to restore the item anyway.
if e := graph.IsErrDeletedInFlight(err); e != nil {
if graph.IsErrDeletedInFlight(err) {
atomic.AddInt64(&success, 1)
logger.Ctx(ctx).Infow(
"Graph reported item not found",
"error",
e,
"service",
path.ExchangeService.String(),
"category",
col.category.String,
)
"error", err,
"service", path.ExchangeService.String(),
"category", col.category.String)
return
}
@ -286,17 +282,13 @@ func (col *Collection) streamItems(ctx context.Context) {
// there's really nothing we can do and not reporting it will make the
// status code upset cause we won't have the same number of results as
// attempted items.
if e := graph.IsErrDeletedInFlight(err); e != nil {
if graph.IsErrDeletedInFlight(err) {
atomic.AddInt64(&success, 1)
logger.Ctx(ctx).Infow(
"Graph reported item not found",
"error",
e,
"service",
path.ExchangeService.String(),
"category",
col.category.String,
)
"error", err,
"service", path.ExchangeService.String(),
"category", col.category.String)
return
}

View File

@ -93,8 +93,7 @@ func filterContainersAndFillCollections(
added, removed, newDelta, err := getter.GetAddedAndRemovedItemIDs(ctx, qp.ResourceOwner, cID, prevDelta)
if err != nil {
// note == nil check; only catches non-inFlight error cases.
if graph.IsErrDeletedInFlight(err) == nil {
if !graph.IsErrDeletedInFlight(err) {
errs = support.WrapAndAppend(qp.ResourceOwner, err, errs)
continue
}

View File

@ -41,21 +41,17 @@ type ErrDeletedInFlight struct {
common.Err
}
func IsErrDeletedInFlight(err error) error {
if asDeletedInFlight(err) {
return err
func IsErrDeletedInFlight(err error) bool {
e := ErrDeletedInFlight{}
if errors.As(err, &e) {
return true
}
if hasErrorCode(err, errCodeItemNotFound, errCodeSyncFolderNotFound) {
return ErrDeletedInFlight{*common.EncapsulateError(err)}
return true
}
return nil
}
func asDeletedInFlight(err error) bool {
e := ErrDeletedInFlight{}
return errors.As(err, &e)
return false
}
// Delta tokens can be desycned or expired. In either case, the token
@ -65,21 +61,17 @@ type ErrInvalidDelta struct {
common.Err
}
func IsErrInvalidDelta(err error) error {
if asInvalidDelta(err) {
return err
func IsErrInvalidDelta(err error) bool {
e := ErrInvalidDelta{}
if errors.As(err, &e) {
return true
}
if hasErrorCode(err, errCodeSyncStateNotFound, errCodeResyncRequired) {
return ErrInvalidDelta{*common.EncapsulateError(err)}
return true
}
return nil
}
func asInvalidDelta(err error) bool {
e := ErrInvalidDelta{}
return errors.As(err, &e)
return false
}
func IsErrExchangeMailFolderNotFound(err error) bool {
@ -94,28 +86,12 @@ type ErrTimeout struct {
common.Err
}
func IsErrTimeout(err error) error {
if asTimeout(err) {
return err
}
if isTimeoutErr(err) {
return ErrTimeout{*common.EncapsulateError(err)}
}
return nil
}
func asTimeout(err error) bool {
func IsErrTimeout(err error) bool {
e := ErrTimeout{}
return errors.As(err, &e)
}
if errors.As(err, &e) {
return true
}
// isTimeoutErr is used to determine if the Graph error returned is
// because of Timeout. This is used to restrict retries to just
// timeouts as other errors are handled within a middleware in the
// client.
func isTimeoutErr(err error) bool {
if errors.Is(err, context.DeadlineExceeded) || os.IsTimeout(err) {
return true
}
@ -132,20 +108,13 @@ type ErrThrottled struct {
common.Err
}
func IsErrThrottled(err error) error {
func IsErrThrottled(err error) bool {
if errors.Is(err, Err429TooManyRequests) {
return err
return true
}
if asThrottled(err) {
return err
}
return nil
}
func asThrottled(err error) bool {
e := ErrThrottled{}
return errors.As(err, &e)
}
@ -153,23 +122,16 @@ type ErrUnauthorized struct {
common.Err
}
func IsErrUnauthorized(err error) error {
func IsErrUnauthorized(err error) bool {
// TODO: refine this investigation. We don't currently know if
// a specific item download url expired, or if the full connection
// auth expired.
if errors.Is(err, Err401Unauthorized) {
return err
return true
}
if asUnauthorized(err) {
return err
}
return nil
}
func asUnauthorized(err error) bool {
e := ErrUnauthorized{}
return errors.As(err, &e)
}
@ -177,21 +139,17 @@ type ErrServiceUnavailable struct {
common.Err
}
func IsSericeUnavailable(err error) error {
func IsSericeUnavailable(err error) bool {
if errors.Is(err, Err503ServiceUnavailable) {
return err
return true
}
if asServiceUnavailable(err) {
return err
}
return nil
}
func asServiceUnavailable(err error) bool {
e := ErrUnauthorized{}
return errors.As(err, &e)
if errors.As(err, &e) {
return true
}
return true
}
// ---------------------------------------------------------------------------

View File

@ -256,7 +256,7 @@ func (oc *Collection) populateItems(ctx context.Context) {
break
}
if graph.IsErrUnauthorized(err) != nil {
if graph.IsErrUnauthorized(err) {
// assume unauthorized requests are a sign of an expired
// jwt token, and that we've overrun the available window
// to download the actual file. Re-downloading the item
@ -271,9 +271,7 @@ func (oc *Collection) populateItems(ctx context.Context) {
continue
} else if graph.IsErrTimeout(err) == nil &&
graph.IsErrThrottled(err) == nil &&
graph.IsSericeUnavailable(err) == nil {
} else if !graph.IsErrTimeout(err) && !graph.IsErrThrottled(err) && !graph.IsSericeUnavailable(err) {
// TODO: graphAPI will provides headers that state the duration to wait
// in order to succeed again. The one second sleep won't cut it here.
//