separate delta not supported from invalid (#4375)

groups backup has a failing case where delta
queries are not supported, but we aren't distinguishing
that case from the invalid delta case as expected. This
results in backup failures, instead of success with no data.

---

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

- [x]  Yes, it's included

#### Type of change

- [x] 🐛 Bugfix

#### Issue(s)

* #3988

#### Test Plan

- [x] 💪 Manual
- [x]  Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2023-09-26 12:37:15 -06:00 committed by GitHub
parent 8791c59c17
commit 0474e5c957
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 76 additions and 15 deletions

View File

@ -13,6 +13,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Increase Exchange backup performance by lazily fetching data only for items whose content changed.
- Added `--backups` flag to delete multiple backups in `corso backup delete` command.
## Fixed
- Teams Channels that cannot support delta tokens (those without messages) fall back to non-delta enumeration and no longer fail a backup.
## [v0.13.0] (beta) - 2023-09-18
### Added

View File

@ -95,6 +95,10 @@ var (
// https://learn.microsoft.com/en-us/graph/errors#code-property
ErrInvalidDelta = clues.New("invalid delta token")
// Not all systems support delta queries. This must be handled separately
// from invalid delta token cases.
ErrDeltaNotSupported = clues.New("delta not supported")
// ErrItemAlreadyExistsConflict denotes that a post or put attempted to create
// an item which already exists by some unique identifier. The identifier is
// not always the id. For example, in onedrive, this error can be produced
@ -122,8 +126,8 @@ var (
)
func IsErrApplicationThrottled(err error) bool {
return hasErrorCode(err, applicationThrottled) ||
errors.Is(err, ErrApplicationThrottled)
return errors.Is(err, ErrApplicationThrottled) ||
hasErrorCode(err, applicationThrottled)
}
func IsErrAuthenticationError(err error) bool {
@ -151,9 +155,13 @@ func IsErrItemNotFound(err error) bool {
}
func IsErrInvalidDelta(err error) bool {
return hasErrorCode(err, syncStateNotFound, resyncRequired, syncStateInvalid) ||
hasErrorMessage(err, parameterDeltaTokenNotSupported) ||
errors.Is(err, ErrInvalidDelta)
return errors.Is(err, ErrInvalidDelta) ||
hasErrorCode(err, syncStateNotFound, resyncRequired, syncStateInvalid)
}
func IsErrDeltaNotSupported(err error) bool {
return errors.Is(err, ErrDeltaNotSupported) ||
hasErrorMessage(err, parameterDeltaTokenNotSupported)
}
func IsErrQuotaExceeded(err error) bool {
@ -190,7 +198,8 @@ func IsErrCannotOpenFileAttachment(err error) bool {
}
func IsErrAccessDenied(err error) bool {
return hasErrorCode(err, ErrorAccessDenied) || clues.HasLabel(err, LabelStatus(http.StatusForbidden))
return hasErrorCode(err, ErrorAccessDenied) ||
clues.HasLabel(err, LabelStatus(http.StatusForbidden))
}
func IsErrTimeout(err error) bool {
@ -218,8 +227,8 @@ func IsErrUnauthorized(err error) bool {
}
func IsErrItemAlreadyExistsConflict(err error) bool {
return hasErrorCode(err, nameAlreadyExists) ||
errors.Is(err, ErrItemAlreadyExistsConflict)
return errors.Is(err, ErrItemAlreadyExistsConflict) ||
hasErrorCode(err, nameAlreadyExists)
}
// LabelStatus transforms the provided statusCode into
@ -298,7 +307,7 @@ func hasErrorMessage(err error, msgs ...errorMessage) bool {
cs[i] = string(c)
}
return filters.Contains(cs).Compare(msg)
return filters.In(cs).Compare(msg)
}
// Wrap is a helper function that extracts ODataError metadata from

View File

@ -247,11 +247,6 @@ func (suite *GraphErrorsUnitSuite) TestIsErrInvalidDelta() {
err: odErr(string(syncStateInvalid)),
expect: assert.True,
},
{
name: "deltatoken not supported oDataErrMsg",
err: odErrMsg("fnords", string(parameterDeltaTokenNotSupported)),
expect: assert.True,
},
// next two tests are to make sure the checks are case insensitive
{
name: "resync-required oDataErr camelcase",
@ -271,6 +266,55 @@ func (suite *GraphErrorsUnitSuite) TestIsErrInvalidDelta() {
}
}
func (suite *GraphErrorsUnitSuite) TestIsErrDeltaNotSupported() {
table := []struct {
name string
err error
expect assert.BoolAssertionFunc
}{
{
name: "nil",
err: nil,
expect: assert.False,
},
{
name: "non-matching",
err: assert.AnError,
expect: assert.False,
},
{
name: "as",
err: ErrDeltaNotSupported,
expect: assert.True,
},
{
name: "non-matching oDataErr",
err: odErr("fnords"),
expect: assert.False,
},
{
name: "non-matching oDataErrMsg",
err: odErrMsg("fnords", "deltatoken not supported"),
expect: assert.False,
},
{
name: "deltatoken not supported oDataErrMsg",
err: odErrMsg("fnords", string(parameterDeltaTokenNotSupported)),
expect: assert.True,
},
{
name: "deltatoken not supported oDataErrMsg with punctuation",
err: odErrMsg("fnords", string(parameterDeltaTokenNotSupported)+"."),
expect: assert.True,
},
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(suite.T(), IsErrDeltaNotSupported(test.err))
})
}
}
func (suite *GraphErrorsUnitSuite) TestIsErrQuotaExceeded() {
table := []struct {
name string

View File

@ -138,6 +138,11 @@ func deltaEnumerateItems[T any](
// Loop through all pages returned by Graph API.
for len(nextLink) > 0 {
page, err := pager.GetPage(graph.ConsumeNTokens(ctx, graph.SingleGetOrDeltaLC))
if graph.IsErrDeltaNotSupported(err) {
logger.Ctx(ctx).Infow("delta queries not supported")
return nil, DeltaUpdate{}, clues.Stack(graph.ErrDeltaNotSupported, err)
}
if graph.IsErrInvalidDelta(err) {
logger.Ctx(ctx).Infow("invalid previous delta", "delta_link", prevDeltaLink)
@ -191,7 +196,7 @@ func getAddedAndRemovedItemIDs[T any](
) (map[string]time.Time, bool, []string, DeltaUpdate, error) {
if canMakeDeltaQueries {
ts, du, err := deltaEnumerateItems[T](ctx, deltaPager, prevDeltaLink)
if err != nil && (!graph.IsErrInvalidDelta(err) || len(prevDeltaLink) == 0) {
if err != nil && !graph.IsErrInvalidDelta(err) && !graph.IsErrDeltaNotSupported(err) {
return nil, false, nil, DeltaUpdate{}, graph.Stack(ctx, err)
}