larryh/corso
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Adds more retries to OneDrive API calls (#2387)

Browse Source 
## Description

Adds more reties to handle timeout issues.

## 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
- [ ] 🤖 Test
- [ ] 💻 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.-->
- [x] 💪 Manual
- [ ] ⚡ Unit test
- [ ] 💚 E2E
This commit is contained in:
Abin Simon 2023-02-04 06:13:23 +05:30 committed by GitHub
parent 8d04957e5f
commit 5f3eaa0178
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 132 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed
- Backing up a calendar that has the same name as the default calendar
- Added additional backoff-retry to all OneDrive queries.
### Known Issues

31
src/internal/connector/discovery/api/users.go
View File

@ -77,7 +77,13 @@ func (c Users) GetAll(ctx context.Context) ([]models.Userable, error) {
return nil, err
}
resp, err := service.Client().Users().Get(ctx, userOptions(&userFilterNoGuests))
var resp models.UserCollectionResponseable
err = graph.RunWithRetry(func() error {
resp, err = service.Client().Users().Get(ctx, userOptions(&userFilterNoGuests))
return err
})
if err != nil {
return nil, support.ConnectorStackErrorTraceWrap(err, "getting all users")
}
@ -114,22 +120,37 @@ func (c Users) GetAll(ctx context.Context) ([]models.Userable, error) {
}
func (c Users) GetByID(ctx context.Context, userID string) (models.Userable, error) {
user, err := c.stable.Client().UsersById(userID).Get(ctx, nil)
var (
resp models.Userable
err error
)
err = graph.RunWithRetry(func() error {
resp, err = c.stable.Client().UsersById(userID).Get(ctx, nil)
return err
})
if err != nil {
return nil, support.ConnectorStackErrorTraceWrap(err, "getting user by id")
}
return user, nil
return resp, err
}
func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
// Assume all services are enabled
// then filter down to only services the user has enabled
userInfo := newUserInfo()
var (
err error
userInfo = newUserInfo()
)
// TODO: OneDrive
err = graph.RunWithRetry(func() error {
_, err = c.stable.Client().UsersById(userID).MailFolders().Get(ctx, nil)
return err
})
_, err := c.stable.Client().UsersById(userID).MailFolders().Get(ctx, nil)
if err != nil {
if !graph.IsErrExchangeMailFolderNotFound(err) {
return nil, support.ConnectorStackErrorTraceWrap(err, "getting user's exchange mailfolders")

24
src/internal/connector/exchange/api/api.go
View File

@ -10,7 +10,6 @@ import (
"github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/internal/connector/support"
"github.com/alcionai/corso/src/pkg/account"
)
@ -154,26 +153,3 @@ func HasAttachments(body models.ItemBodyable) bool {
return strings.Contains(content, "src=\"cid:")
}
// Run a function with retries
func runWithRetry(run func() error) error {
var err error
for i := 0; i < numberOfRetries; i++ {
err = run()
if err == nil {
return nil
}
// only retry on timeouts and 500-internal-errors.
if !(graph.IsErrTimeout(err) || graph.IsInternalServerError(err)) {
break
}
if i < numberOfRetries {
time.Sleep(time.Duration(3*(i+2)) * time.Second)
}
}
return support.ConnectorStackErrorTraceWrap(err, "maximum retries or unretryable")
}

10
src/internal/connector/exchange/api/contacts.go
View File

@ -68,7 +68,7 @@ func (c Contacts) GetItem(
err error
)
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
cont, err = c.stable.Client().UsersById(user).ContactsById(itemID).Get(ctx, nil)
return err
})
@ -94,7 +94,7 @@ func (c Contacts) GetAllContactFolderNamesForUser(
var resp models.ContactFolderCollectionResponseable
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = c.stable.Client().UsersById(user).ContactFolders().Get(ctx, options)
return err
})
@ -113,7 +113,7 @@ func (c Contacts) GetContainerByID(
var resp models.ContactFolderable
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = c.stable.Client().UsersById(userID).ContactFoldersById(dirID).Get(ctx, ofcf)
return err
})
@ -154,7 +154,7 @@ func (c Contacts) EnumerateContainers(
ChildFolders()
for {
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = builder.Get(ctx, ofcf)
return err
})
@ -206,7 +206,7 @@ func (p *contactPager) getPage(ctx context.Context) (api.DeltaPageLinker, error)
err error
)
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = p.builder.Get(ctx, p.options)
return err
})

10
src/internal/connector/exchange/api/events.go
View File

@ -77,7 +77,7 @@ func (c Events) GetContainerByID(
var cal models.Calendarable
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
cal, err = service.Client().UsersById(userID).CalendarsById(containerID).Get(ctx, ofc)
return err
})
@ -99,7 +99,7 @@ func (c Events) GetItem(
err error
)
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
event, err = c.stable.Client().UsersById(user).EventsById(itemID).Get(ctx, nil)
return err
})
@ -154,7 +154,7 @@ func (c Client) GetAllCalendarNamesForUser(
var resp models.CalendarCollectionResponseable
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = c.stable.Client().UsersById(user).Calendars().Get(ctx, options)
return err
})
@ -193,7 +193,7 @@ func (c Events) EnumerateContainers(
for {
var err error
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = builder.Get(ctx, ofc)
return err
})
@ -250,7 +250,7 @@ func (p *eventPager) getPage(ctx context.Context) (api.DeltaPageLinker, error) {
err error
)
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = p.builder.Get(ctx, p.options)
return err
})

8
src/internal/connector/exchange/api/mail.go
View File

@ -99,7 +99,7 @@ func (c Mail) GetContainerByID(
var resp graph.Container
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = service.Client().UsersById(userID).MailFoldersById(dirID).Get(ctx, ofmf)
return err
})
@ -118,7 +118,7 @@ func (c Mail) GetItem(
err error
)
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
mail, err = c.stable.Client().UsersById(user).MessagesById(itemID).Get(ctx, nil)
return err
})
@ -188,7 +188,7 @@ func (c Mail) EnumerateContainers(
for {
var err error
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
resp, err = builder.Get(ctx, nil)
return err
})
@ -235,7 +235,7 @@ func (p *mailPager) getPage(ctx context.Context) (api.DeltaPageLinker, error) {
err error
)
err = runWithRetry(func() error {
err = graph.RunWithRetry(func() error {
page, err = p.builder.Get(ctx, p.options)
return err
})

25
src/internal/connector/graph/service.go
View File

@ -8,6 +8,7 @@ import (
"time"
"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
"github.com/alcionai/corso/src/internal/connector/support"
"github.com/microsoft/kiota-abstractions-go/serialization"
ka "github.com/microsoft/kiota-authentication-azure-go"
khttp "github.com/microsoft/kiota-http-go"
@ -22,6 +23,7 @@ import (
const (
logGraphRequestsEnvKey = "LOG_GRAPH_REQUESTS"
numberOfRetries = 3
)
// AllMetadataFileNames produces the standard set of filenames used to store graph
@ -294,3 +296,26 @@ func (handler *LoggingMiddleware) Intercept(
return resp, err
}
// Run a function with retries
func RunWithRetry(run func() error) error {
var err error
for i := 0; i < numberOfRetries; i++ {
err = run()
if err == nil {
return nil
}
// only retry on timeouts and 500-internal-errors.
if !(IsErrTimeout(err) || IsInternalServerError(err)) {
break
}
if i < numberOfRetries {
time.Sleep(time.Duration(3*(i+2)) * time.Second)
}
}
return support.ConnectorStackErrorTraceWrap(err, "maximum retries or unretryable")
}

36
src/internal/connector/onedrive/api/drive.go
View File

@ -61,7 +61,17 @@ func NewItemPager(
}
func (p *driveItemPager) GetPage(ctx context.Context) (api.DeltaPageLinker, error) {
return p.builder.Get(ctx, p.options)
var (
resp api.DeltaPageLinker
err error
)
err = graph.RunWithRetry(func() error {
resp, err = p.builder.Get(ctx, p.options)
return err
})
return resp, err
}
func (p *driveItemPager) SetNext(link string) {
@ -99,7 +109,17 @@ func NewUserDrivePager(
}
func (p *userDrivePager) GetPage(ctx context.Context) (api.PageLinker, error) {
return p.builder.Get(ctx, p.options)
var (
resp api.PageLinker
err error
)
err = graph.RunWithRetry(func() error {
resp, err = p.builder.Get(ctx, p.options)
return err
})
return resp, err
}
func (p *userDrivePager) SetNext(link string) {
@ -137,7 +157,17 @@ func NewSiteDrivePager(
}
func (p *siteDrivePager) GetPage(ctx context.Context) (api.PageLinker, error) {
return p.builder.Get(ctx, p.options)
var (
resp api.PageLinker
err error
)
err = graph.RunWithRetry(func() error {
resp, err = p.builder.Get(ctx, p.options)
return err
})
return resp, err
}
func (p *siteDrivePager) SetNext(link string) {

13
src/internal/connector/onedrive/drive.go
View File

@ -80,7 +80,7 @@ func drives(
page, err = pager.GetPage(ctx)
if err != nil {
// Various error handling. May return an error or perform a retry.
detailedError := support.ConnectorStackErrorTrace(err)
detailedError := err.Error()
if strings.Contains(detailedError, userMysiteURLNotFound) ||
strings.Contains(detailedError, userMysiteNotFound) {
logger.Ctx(ctx).Infof("resource owner does not have a drive")
@ -236,7 +236,16 @@ func getFolder(
rawURL := fmt.Sprintf(itemByPathRawURLFmt, driveID, parentFolderID, folderName)
builder := msdrive.NewItemsDriveItemItemRequestBuilder(rawURL, service.Adapter())
foundItem, err := builder.Get(ctx, nil)
var (
foundItem models.DriveItemable
err error
)
err = graph.RunWithRetry(func() error {
foundItem, err = builder.Get(ctx, nil)
return err
})
if err != nil {
var oDataError *odataerrors.ODataError
if errors.As(err, &oDataError) &&

42
src/internal/connector/onedrive/drive_test.go
View File

@ -15,6 +15,7 @@ import (
"github.com/alcionai/corso/src/internal/common"
"github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/internal/connector/graph/api"
"github.com/alcionai/corso/src/internal/connector/support"
"github.com/alcionai/corso/src/internal/tester"
"github.com/alcionai/corso/src/pkg/control"
"github.com/alcionai/corso/src/pkg/logger"
@ -76,6 +77,15 @@ func TestOneDriveUnitSuite(t *testing.T) {
suite.Run(t, new(OneDriveUnitSuite))
}
func odErr(code string) *odataerrors.ODataError {
odErr := &odataerrors.ODataError{}
merr := odataerrors.MainError{}
merr.SetCode(&code)
odErr.SetError(&merr)
return odErr
}
func (suite *OneDriveUnitSuite) TestDrives() {
numDriveResults := 4
emptyLink := ""
@ -84,26 +94,18 @@ func (suite *OneDriveUnitSuite) TestDrives() {
// These errors won't be the "correct" format when compared to what graph
// returns, but they're close enough to have the same info when the inner
// details are extracted via support package.
tmp := userMysiteURLNotFound
tmpMySiteURLNotFound := odataerrors.NewMainError()
tmpMySiteURLNotFound.SetMessage(&tmp)
mySiteURLNotFound := odataerrors.NewODataError()
mySiteURLNotFound.SetError(tmpMySiteURLNotFound)
tmp2 := userMysiteNotFound
tmpMySiteNotFound := odataerrors.NewMainError()
tmpMySiteNotFound.SetMessage(&tmp2)
mySiteNotFound := odataerrors.NewODataError()
mySiteNotFound.SetError(tmpMySiteNotFound)
tmp3 := contextDeadlineExceeded
tmpDeadlineExceeded := odataerrors.NewMainError()
tmpDeadlineExceeded.SetMessage(&tmp3)
deadlineExceeded := odataerrors.NewODataError()
deadlineExceeded.SetError(tmpDeadlineExceeded)
mySiteURLNotFound := support.ConnectorStackErrorTraceWrap(
odErr(userMysiteURLNotFound),
"maximum retries or unretryable",
)
mySiteNotFound := support.ConnectorStackErrorTraceWrap(
odErr(userMysiteNotFound),
"maximum retries or unretryable",
)
deadlineExceeded := support.ConnectorStackErrorTraceWrap(
odErr(contextDeadlineExceeded),
"maximum retries or unretryable",
)
resultDrives := make([]models.Driveable, 0, numDriveResults)