From 0474e5c95717eb5923edcc425645e6105249b387 Mon Sep 17 00:00:00 2001 From: Keepers Date: Tue, 26 Sep 2023 12:37:15 -0600 Subject: [PATCH] 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] :white_check_mark: Yes, it's included #### Type of change - [x] :bug: Bugfix #### Issue(s) * #3988 #### Test Plan - [x] :muscle: Manual - [x] :zap: Unit test - [x] :green_heart: E2E --- CHANGELOG.md | 3 ++ src/internal/m365/graph/errors.go | 27 ++++++++----- src/internal/m365/graph/errors_test.go | 54 ++++++++++++++++++++++--- src/pkg/services/m365/api/item_pager.go | 7 +++- 4 files changed, 76 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d5ad8ce4..0e8c12dfd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/internal/m365/graph/errors.go b/src/internal/m365/graph/errors.go index 91dc01bb0..f5c7824ab 100644 --- a/src/internal/m365/graph/errors.go +++ b/src/internal/m365/graph/errors.go @@ -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 diff --git a/src/internal/m365/graph/errors_test.go b/src/internal/m365/graph/errors_test.go index 606eb23a5..cf9f2f99d 100644 --- a/src/internal/m365/graph/errors_test.go +++ b/src/internal/m365/graph/errors_test.go @@ -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 diff --git a/src/pkg/services/m365/api/item_pager.go b/src/pkg/services/m365/api/item_pager.go index d8fed9116..3cd1deb53 100644 --- a/src/pkg/services/m365/api/item_pager.go +++ b/src/pkg/services/m365/api/item_pager.go @@ -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) }