From c1ec1585a27d641235ce106e76c87dce0f2a5269 Mon Sep 17 00:00:00 2001 From: Abhishek Pandey Date: Tue, 6 Feb 2024 18:40:14 -0800 Subject: [PATCH] Skip exchange items which fail to download due to ErrorCorruptData error (#5191) * We are seeing 500 errors from graph during exchange(email) backup. * Error is `:{"code":"ErrorCorruptData","message":"Data is corrupt., Invalid global object ID: some ID`. * Catch the error and add a skip since these items cannot be downloaded from graph. --- #### Does this PR need a docs update or release note? - [x] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [ ] :no_entry: No #### Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Supportability/Tests - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup #### Issue(s) * # #### Test Plan - [ ] :muscle: Manual - [x] :zap: Unit test - [ ] :green_heart: E2E --- CHANGELOG.md | 3 ++ .../m365/collection/exchange/collection.go | 13 +++++++ .../collection/exchange/collection_test.go | 11 ++++++ src/pkg/fault/skipped.go | 4 +++ src/pkg/services/m365/api/graph/errors.go | 8 +++++ .../services/m365/api/graph/errors_test.go | 36 ++++++++++++++++++- 6 files changed, 74 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95e99d7eb..3c0c93c38 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] (beta) +### Fixed +- Handle the case where an email or event cannot be retrieved from Exchange due to an `ErrorCorruptData` error. Corso will skip over the item but report it in the backup summary. + ## [v0.19.0] (beta) - 2024-02-06 ### Added diff --git a/src/internal/m365/collection/exchange/collection.go b/src/internal/m365/collection/exchange/collection.go index 7cd6161de..efb6f7e94 100644 --- a/src/internal/m365/collection/exchange/collection.go +++ b/src/internal/m365/collection/exchange/collection.go @@ -300,6 +300,19 @@ func (col *prefetchCollection) streamItems( id, map[string]any{"parentPath": parentPath})) atomic.AddInt64(&success, 1) + case graph.IsErrCorruptData(err): + // These items cannot be downloaded, graph error indicates that the item + // data is corrupted. Add to skipped list. + logger. + CtxErr(ctx, err). + With("skipped_reason", fault.SkipCorruptData). + Info("inaccessible email") + errs.AddSkip(ctx, fault.EmailSkip( + fault.SkipCorruptData, + user, + id, + map[string]any{"parentPath": parentPath})) + atomic.AddInt64(&success, 1) default: col.Counter.Inc(count.StreamItemsErred) el.AddRecoverable(ctx, clues.Wrap(err, "fetching item").Label(fault.LabelForceNoBackupCreation)) diff --git a/src/internal/m365/collection/exchange/collection_test.go b/src/internal/m365/collection/exchange/collection_test.go index d2e8b86c8..1ec9e81f3 100644 --- a/src/internal/m365/collection/exchange/collection_test.go +++ b/src/internal/m365/collection/exchange/collection_test.go @@ -364,6 +364,17 @@ func (suite *CollectionUnitSuite) TestCollection_SkippedErrors() { }, expectedSkipError: fault.EmailSkip(fault.SkipInvalidRecipients, "", "fisher", nil), }, + { + name: "ErrorCorruptData", + added: map[string]time.Time{ + "fisher": {}, + }, + expectItemCount: 0, + itemGetter: &mock.ItemGetSerialize{ + GetErr: graphTD.ODataErr(string(graph.ErrorCorruptData)), + }, + expectedSkipError: fault.EmailSkip(fault.SkipCorruptData, "", "fisher", nil), + }, } for _, test := range table { diff --git a/src/pkg/fault/skipped.go b/src/pkg/fault/skipped.go index 3bb08af50..1754f3a06 100644 --- a/src/pkg/fault/skipped.go +++ b/src/pkg/fault/skipped.go @@ -36,6 +36,10 @@ const ( // SkipInvalidRecipients identifies that an email was skipped because Exchange // believes it is not valid and fails any attempt to read it. SkipInvalidRecipients skipCause = "invalid_recipients_email" + + // SkipCorruptData identifies that an email was skipped because graph reported + // that the email data was corrupt and failed all attempts to read it. + SkipCorruptData skipCause = "corrupt_data" ) var _ print.Printable = &Skipped{} diff --git a/src/pkg/services/m365/api/graph/errors.go b/src/pkg/services/m365/api/graph/errors.go index ab46c0ad9..6c99d3f21 100644 --- a/src/pkg/services/m365/api/graph/errors.go +++ b/src/pkg/services/m365/api/graph/errors.go @@ -50,6 +50,10 @@ const ( // are mailbox creation racing with email receipt or a similar issue triggered // due to on-prem->M365 mailbox migration. ErrorInvalidRecipients errorCode = "ErrorInvalidRecipients" + // We have seen this graph error with a 500 response while backing up email + // messages. It implies that the email message is corrupt and cannot be read. + // Associated error message goes like "Data is corrupt.,Invalid global object ID:" + ErrorCorruptData errorCode = "ErrorCorruptData" // This error occurs when an attempt is made to create a folder that has // the same name as another folder in the same parent. Such duplicate folder // names are not allowed by graph. @@ -249,6 +253,10 @@ func IsErrInvalidRecipients(err error) bool { return parseODataErr(err).hasErrorCode(err, ErrorInvalidRecipients) } +func IsErrCorruptData(err error) bool { + return parseODataErr(err).hasErrorCode(err, ErrorCorruptData) +} + func IsErrCannotOpenFileAttachment(err error) bool { return parseODataErr(err).hasErrorCode(err, cannotOpenFileAttachment) } diff --git a/src/pkg/services/m365/api/graph/errors_test.go b/src/pkg/services/m365/api/graph/errors_test.go index c312a47f1..19b113a80 100644 --- a/src/pkg/services/m365/api/graph/errors_test.go +++ b/src/pkg/services/m365/api/graph/errors_test.go @@ -235,7 +235,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrNotFound() { } } -func (suite *GraphErrorsUnitSuite) Test() { +func (suite *GraphErrorsUnitSuite) TestIsErrInvalidRecipients() { table := []struct { name string err error @@ -269,6 +269,40 @@ func (suite *GraphErrorsUnitSuite) Test() { } } +func (suite *GraphErrorsUnitSuite) TestIsErrCorruptData() { + 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: "non-matching oDataErr", + err: graphTD.ODataErr("fnords"), + expect: assert.False, + }, + { + name: "invalid receipient oDataErr", + err: graphTD.ODataErr(string(ErrorCorruptData)), + expect: assert.True, + }, + } + for _, test := range table { + suite.Run(test.name, func() { + test.expect(suite.T(), IsErrCorruptData(test.err)) + }) + } +} + func (suite *GraphErrorsUnitSuite) TestIsErrInvalidDelta() { table := []struct { name string