Skip any attachment fetches that fail with OLE conversion error (#3607)

These files from what we can understand are not available and thus can't be fetched. This change ensures that we don't fail the backup just because of this error.

<!-- PR description-->

---

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

- [x]  Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ]  No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #<issue>

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x]  Unit test
- [ ] 💚 E2E
This commit is contained in:
Abin Simon 2023-06-14 10:39:07 +05:30 committed by GitHub
parent ce72acbcc1
commit 6e2d72509c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 62 additions and 0 deletions

View File

@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Fix Exchange folder cache population error when parent folder isn't found. - Fix Exchange folder cache population error when parent folder isn't found.
- Fix Exchange backup issue caused by incorrect json serialization - Fix Exchange backup issue caused by incorrect json serialization
- Fix issues with details model containing duplicate entry for api consumers - Fix issues with details model containing duplicate entry for api consumers
- Handle OLE conversion errors when trying to fetch attachments
### Changed ### Changed
- Do not display all the items that we restored at the end if there are more than 15. You can override this with `--verbose`. - Do not display all the items that we restored at the end if there are more than 15. You can override this with `--verbose`.

View File

@ -44,6 +44,10 @@ const (
// the same name as another folder in the same parent. Such duplicate folder // the same name as another folder in the same parent. Such duplicate folder
// names are not allowed by graph. // names are not allowed by graph.
folderExists errorCode = "ErrorFolderExists" folderExists errorCode = "ErrorFolderExists"
// cannotOpenFileAttachment happen when an attachment is
// inaccessible. The error message is usually "OLE conversion
// failed for an attachment."
cannotOpenFileAttachment errorCode = "ErrorCannotOpenFileAttachment"
) )
type errorMessage string type errorMessage string
@ -126,6 +130,10 @@ func IsErrResourceNotFound(err error) bool {
return hasErrorCode(err, resourceNotFound) return hasErrorCode(err, resourceNotFound)
} }
func IsErrCannotOpenFileAttachment(err error) bool {
return hasErrorCode(err, cannotOpenFileAttachment)
}
func IsErrAccessDenied(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))
} }

View File

@ -384,3 +384,42 @@ func (suite *GraphErrorsUnitSuite) TestIsErrFolderExists() {
}) })
} }
} }
func (suite *GraphErrorsUnitSuite) TestIsErrCannotOpenFileAttachment() {
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: ErrInvalidDelta,
expect: assert.False,
},
{
name: "non-matching oDataErr",
err: odErr("fnords"),
expect: assert.False,
},
{
name: "quota-exceeded oDataErr",
err: odErr(string(cannotOpenFileAttachment)),
expect: assert.True,
},
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(suite.T(), IsErrCannotOpenFileAttachment(test.err))
})
}
}

View File

@ -406,6 +406,20 @@ func (c Mail) GetItem(
ByAttachmentId(ptr.Val(a.GetId())). ByAttachmentId(ptr.Val(a.GetId())).
Get(ctx, attachConfig) Get(ctx, attachConfig)
if err != nil { if err != nil {
if graph.IsErrCannotOpenFileAttachment(err) {
logger.CtxErr(ctx, err).
With(
"skipped_reason", fault.SkipNotFound,
"attachment_id", ptr.Val(a.GetId()),
"attachment_size", ptr.Val(a.GetSize()),
).Info("attachment not found")
// TODO This should use a `AddSkip` once we have
// figured out the semantics for skipping
// subcomponents of an item
continue
}
return nil, nil, graph.Wrap(ctx, err, "getting mail attachment"). return nil, nil, graph.Wrap(ctx, err, "getting mail attachment").
With("attachment_id", ptr.Val(a.GetId()), "attachment_size", ptr.Val(a.GetSize())) With("attachment_id", ptr.Val(a.GetId()), "attachment_size", ptr.Val(a.GetSize()))
} }