From 1d16355873ad830caa1673ce574dccb7d9aa0d8b Mon Sep 17 00:00:00 2001
From: Abin Simon <abinsimon10@gmail.com>
Date: Mon, 12 Dec 2022 13:50:02 +0530
Subject: [PATCH] Don't retry unless timeout err (#1738)

## Description

We have a retry handler within the client added as a middleware. So, don't retry here again if we still get any other err.

## Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] :sunflower: Feature
- [x] :bug: Bugfix
- [ ] :world_map: Documentation
- [ ] :robot: Test
- [ ] :computer: CI/Deployment
- [ ] :hamster: Trivial/Minor

## Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* fixes https://github.com/alcionai/corso/issues/1684

## Test Plan

<!-- How will this be tested prior to merging.-->
- [x] :muscle: Manual
- [ ] :zap: Unit test
- [x] :green_heart: E2E
---
 src/internal/connector/onedrive/collection.go | 29 +++++++++++++++----
 1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/internal/connector/onedrive/collection.go b/src/internal/connector/onedrive/collection.go
index 7733248ff..0d61988b5 100644
--- a/src/internal/connector/onedrive/collection.go
+++ b/src/internal/connector/onedrive/collection.go
@@ -4,6 +4,7 @@ package onedrive
 import (
 	"context"
 	"io"
+	"net/url"
 	"sync"
 	"sync/atomic"
 	"time"
@@ -38,7 +39,7 @@ var (
 	//_ data.StreamModTime = &Item{}
 )
 
-// Collection represents a set of OneDrive objects retreived from M365
+// Collection represents a set of OneDrive objects retrieved from M365
 type Collection struct {
 	// data is used to share data streams with the collection consumer
 	data chan data.Stream
@@ -148,6 +149,19 @@ func (od *Item) Info() details.ItemInfo {
 //	return od.info.Modified
 //}
 
+// isTimeoutErr is used to determine if the Graph error returned is
+// because of Timeout. This is used to restrict retries to just
+// timeouts as other errors are handled within a middleware in the
+// client.
+func isTimeoutErr(err error) bool {
+	switch err := err.(type) {
+	case *url.Error:
+		return err.Timeout()
+	default:
+		return false
+	}
+}
+
 // populateItems iterates through items added to the collection
 // and uses the collection `itemReader` to read the item
 func (oc *Collection) populateItems(ctx context.Context) {
@@ -204,16 +218,19 @@ func (oc *Collection) populateItems(ctx context.Context) {
 				err      error
 			)
 
-			// Retrying as we were hitting timeouts when we have multiple requests
-			// https://github.com/microsoftgraph/msgraph-sdk-go/issues/302
 			for i := 1; i <= maxRetries; i++ {
 				itemInfo, itemData, err = oc.itemReader(ctx, oc.service, oc.driveID, itemID)
-				if err == nil {
+
+				// We only retry if it is a timeout error. Other
+				// errors like throttling are already handled within
+				// the graph client via a retry middleware.
+				// https://github.com/microsoftgraph/msgraph-sdk-go/issues/302
+				if err == nil || !isTimeoutErr(err) {
 					break
 				}
-				// TODO: Tweak sleep times
+
 				if i < maxRetries {
-					time.Sleep(time.Duration(3*(i+1)) * time.Second)
+					time.Sleep(1 * time.Second)
 				}
 			}