[Bug]: Exchange swallows attachment serialization errors

ryanfkeepers commented 1 year ago

What happened?

https://github.com/alcionai/corso/blob/be4032aec9b19b0c12cdc603e97c9be69d6cc01a/src/internal/connector/exchange/service_restore.go#L300

Exchange item attachment processing swallowing errors instead of reporting them. When we attempted to remove this code, we started getting "could not deserialize payload" errors in our integration tests. This implies we might be defaulting on uploads in prod, but not tracking the failures.

Investigation into the case is needed. There's no explanation for skipping the error or why the code is there in the first place. Feels like something we should remove, and update the tests to match.

Corso Version?

Corso v0.8.0

Where are you running Corso?

any

Relevant log output

No response

vkamra commented 1 year ago

This was introduced in https://github.com/alcionai/corso/commit/a4b50a1ec05cd44bb361a861b0203dd8dc9a1c92

ashmrtn commented 10 months ago

this now seems to be causing failures for TestRestoreIntgSuite/TestRestoreExchangeObject/Test_Mail:_Item_Attachment_Event if the line in question is removed. I'm unable to create an attachment of the type in the test so I'm unable to manually reproduce the issue outside the test. The test does consistently fail with a 500 internal server error though

alcionai / corso