From 77e9c0fad2f2cb737dc54cba7eb39f5877901fd8 Mon Sep 17 00:00:00 2001 From: Abin Simon Date: Tue, 7 Feb 2023 23:59:44 +0530 Subject: [PATCH] Update retry handling for permissions (#2420) ## Description Previous retry check logic was incorrect and was never retrying. This switches it to using `graph.RunWithRetry`. Sample failures: - https://github.com/alcionai/corso/actions/runs/4109625295/jobs/7091735297 - https://github.com/alcionai/corso/actions/runs/4110739264/jobs/7093919589 ## Does this PR need a docs update or release note? - [ ] :white_check_mark: Yes, it's included - [ ] :clock1: Yes, but in a later PR - [x] :no_entry: No ## Type of change - [ ] :sunflower: Feature - [x] :bug: Bugfix - [ ] :world_map: Documentation - [ ] :robot: Test - [ ] :computer: CI/Deployment - [ ] :broom: Tech Debt/Cleanup ## Issue(s) * # ## Test Plan - [x] :muscle: Manual - [ ] :zap: Unit test - [ ] :green_heart: E2E --- src/internal/connector/onedrive/collection.go | 42 +++++++------------ 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go index d56114746..e0a328ef7 100644 --- a/src/internal/connector/onedrive/collection.go +++ b/src/internal/connector/onedrive/collection.go @@ -280,35 +280,23 @@ func (oc *Collection) populateItems(ctx context.Context) { if oc.source == OneDriveSource { // Fetch metadata for the file - for i := 1; i <= maxRetries; i++ { - if !oc.ctrl.ToggleFeatures.EnablePermissionsBackup { - // We are still writing the metadata file but with - // empty permissions as we don't have a way to - // signify that the permissions was explicitly - // not added. - itemMeta = io.NopCloser(strings.NewReader("{}")) - itemMetaSize = 2 + if !oc.ctrl.ToggleFeatures.EnablePermissionsBackup { + // We are still writing the metadata file but with + // empty permissions as we don't have a way to + // signify that the permissions was explicitly + // not added. + itemMeta = io.NopCloser(strings.NewReader("{}")) + itemMetaSize = 2 + } else { + err = graph.RunWithRetry(func() error { + itemMeta, itemMetaSize, err = oc.itemMetaReader(ctx, oc.service, oc.driveID, item) + return err + }) - break + if err != nil { + errUpdater(*item.GetId(), errors.Wrap(err, "failed to get item permissions")) + return } - - itemMeta, itemMetaSize, err = oc.itemMetaReader(ctx, oc.service, oc.driveID, item) - - // retry on Timeout type errors, break otherwise. - if err == nil || - !graph.IsErrTimeout(err) || - !graph.IsInternalServerError(err) { - break - } - - if i < maxRetries { - time.Sleep(1 * time.Second) - } - } - - if err != nil { - errUpdater(*item.GetId(), errors.Wrap(err, "failed to get item permissions")) - return } }