diff --git a/CHANGELOG.md b/CHANGELOG.md index 99d92b4fd..d528b18c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 backup issue caused by incorrect json serialization - Fix issues with details model containing duplicate entry for api consumers +- Handle OLE conversion errors when trying to fetch attachments ### 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`. diff --git a/src/internal/m365/graph/errors.go b/src/internal/m365/graph/errors.go index 2f2427b6c..65150868b 100644 --- a/src/internal/m365/graph/errors.go +++ b/src/internal/m365/graph/errors.go @@ -44,6 +44,10 @@ const ( // the same name as another folder in the same parent. Such duplicate folder // names are not allowed by graph. 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 @@ -126,6 +130,10 @@ func IsErrResourceNotFound(err error) bool { return hasErrorCode(err, resourceNotFound) } +func IsErrCannotOpenFileAttachment(err error) bool { + return hasErrorCode(err, cannotOpenFileAttachment) +} + func IsErrAccessDenied(err error) bool { return hasErrorCode(err, errorAccessDenied) || clues.HasLabel(err, LabelStatus(http.StatusForbidden)) } diff --git a/src/internal/m365/graph/errors_test.go b/src/internal/m365/graph/errors_test.go index a0095dc1e..714677179 100644 --- a/src/internal/m365/graph/errors_test.go +++ b/src/internal/m365/graph/errors_test.go @@ -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)) + }) + } +} diff --git a/src/pkg/services/m365/api/mail.go b/src/pkg/services/m365/api/mail.go index 441198c2a..4c608ad28 100644 --- a/src/pkg/services/m365/api/mail.go +++ b/src/pkg/services/m365/api/mail.go @@ -406,6 +406,20 @@ func (c Mail) GetItem( ByAttachmentId(ptr.Val(a.GetId())). Get(ctx, attachConfig) 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"). With("attachment_id", ptr.Val(a.GetId()), "attachment_size", ptr.Val(a.GetSize())) }