diff --git a/src/internal/connector/exchange/restore_test.go b/src/internal/connector/exchange/restore_test.go index 962964e8c..0c5a3e711 100644 --- a/src/internal/connector/exchange/restore_test.go +++ b/src/internal/connector/exchange/restore_test.go @@ -13,7 +13,6 @@ import ( "github.com/alcionai/corso/src/internal/connector/exchange/api" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/mockconnector" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/account" "github.com/alcionai/corso/src/pkg/control" @@ -88,7 +87,7 @@ func (suite *ExchangeRestoreSuite) TestRestoreContact() { control.Copy, folderID, userID) - assert.NoError(t, err, support.ConnectorStackErrorTrace(err)) + assert.NoError(t, err) assert.NotNil(t, info, "contact item info") } @@ -122,7 +121,7 @@ func (suite *ExchangeRestoreSuite) TestRestoreEvent() { calendarID, userID, fault.New(true)) - assert.NoError(t, err, support.ConnectorStackErrorTrace(err)) + assert.NoError(t, err) assert.NotNil(t, info, "event item info") } @@ -349,7 +348,7 @@ func (suite *ExchangeRestoreSuite) TestRestoreExchangeObject() { destination, userID, fault.New(true)) - assert.NoError(t, err, support.ConnectorStackErrorTrace(err)) + assert.NoError(t, err) assert.NotNil(t, info, "item info was not populated") assert.NotNil(t, deleters) assert.NoError(t, deleters[test.category].DeleteContainer(ctx, userID, destination)) diff --git a/src/internal/connector/exchange/service_restore.go b/src/internal/connector/exchange/service_restore.go index 3abff9e4b..1f5c70347 100644 --- a/src/internal/connector/exchange/service_restore.go +++ b/src/internal/connector/exchange/service_restore.go @@ -609,7 +609,7 @@ func establishMailRestoreLocation( temp, err := ac.Mail().CreateMailFolderWithParent(ctx, user, folder, folderID) if err != nil { // Should only error if cache malfunctions or incorrect parameters - return "", errors.Wrap(err, support.ConnectorStackErrorTrace(err)) + return "", err } folderID = *temp.GetId() @@ -658,7 +658,7 @@ func establishContactsRestoreLocation( temp, err := ac.Contacts().CreateContactFolder(ctx, user, folders[0]) if err != nil { - return "", errors.Wrap(err, support.ConnectorStackErrorTrace(err)) + return "", err } folderID := *temp.GetId() @@ -695,7 +695,7 @@ func establishEventsRestoreLocation( temp, err := ac.Events().CreateCalendar(ctx, user, folders[0]) if err != nil { - return "", errors.Wrap(err, support.ConnectorStackErrorTrace(err)) + return "", err } folderID := *temp.GetId() diff --git a/src/internal/connector/graph_connector_test.go b/src/internal/connector/graph_connector_test.go index db0a97b30..8d0d88bd2 100644 --- a/src/internal/connector/graph_connector_test.go +++ b/src/internal/connector/graph_connector_test.go @@ -5,16 +5,16 @@ import ( "testing" "time" - "github.com/pkg/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "golang.org/x/exp/maps" + "github.com/alcionai/clues" + "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/discovery/api" "github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/mockconnector" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/data" "github.com/alcionai/corso/src/internal/tester" "github.com/alcionai/corso/src/pkg/account" @@ -349,19 +349,15 @@ func mustGetDefaultDriveID( //revive:enable:context-as-argument d, err := service.Client().UsersById(userID).Drive().Get(ctx, nil) if err != nil { - err = errors.Wrapf( - err, - "failed to retrieve default user drive. user: %s, details: %s", - userID, - support.ConnectorStackErrorTrace(err), - ) + err = clues.Wrap(err, "retrieving drive").WithClues(ctx).With(graph.ErrData(err)...) } require.NoError(t, err) - require.NotNil(t, d.GetId()) - require.NotEmpty(t, *d.GetId()) - return *d.GetId() + id := ptr.Val(d.GetId()) + require.NotEmpty(t, id) + + return id } func getCollectionsAndExpected( diff --git a/src/internal/connector/onedrive/drive.go b/src/internal/connector/onedrive/drive.go index e12fa20c8..8cb75cb35 100644 --- a/src/internal/connector/onedrive/drive.go +++ b/src/internal/connector/onedrive/drive.go @@ -16,7 +16,6 @@ import ( "github.com/alcionai/corso/src/internal/connector/graph" gapi "github.com/alcionai/corso/src/internal/connector/graph/api" "github.com/alcionai/corso/src/internal/connector/onedrive/api" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/pkg/fault" "github.com/alcionai/corso/src/pkg/logger" ) @@ -98,7 +97,9 @@ func drives( page, err = pager.GetPage(ctx) if err != nil { // Various error handling. May return an error or perform a retry. - errMsg := support.ConnectorStackErrorTraceWrap(err, "").Error() + // errMsg := support.ConnectorStackErrorTraceWrap(err, "").Error() + // temporarily broken until ^ is fixed in next PR. + errMsg := err.Error() if strings.Contains(errMsg, userMysiteURLNotFound) || strings.Contains(errMsg, userMysiteURLNotFoundMsg) || strings.Contains(errMsg, userMysiteNotFound) || diff --git a/src/internal/connector/onedrive/drive_test.go b/src/internal/connector/onedrive/drive_test.go index d330682d1..b8ca30b06 100644 --- a/src/internal/connector/onedrive/drive_test.go +++ b/src/internal/connector/onedrive/drive_test.go @@ -15,7 +15,6 @@ 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/fault" @@ -95,18 +94,9 @@ 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. - 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", - ) + mySiteURLNotFound := odErr(userMysiteURLNotFound) + mySiteNotFound := odErr(userMysiteNotFound) + deadlineExceeded := odErr(contextDeadlineExceeded) resultDrives := make([]models.Driveable, 0, numDriveResults) diff --git a/src/internal/connector/sharepoint/collection_test.go b/src/internal/connector/sharepoint/collection_test.go index f0a3ed4da..158c22143 100644 --- a/src/internal/connector/sharepoint/collection_test.go +++ b/src/internal/connector/sharepoint/collection_test.go @@ -206,7 +206,7 @@ func (suite *SharePointCollectionSuite) TestListCollection_Restore() { for { resp, err := builder.Get(ctx, nil) - assert.NoError(t, err, "experienced query error during clean up. Details: "+support.ConnectorStackErrorTrace(err)) + assert.NoError(t, err, "getting site lists") for _, temp := range resp.GetValue() { if *temp.GetDisplayName() == deets.SharePoint.ItemName { diff --git a/src/internal/connector/sharepoint/list.go b/src/internal/connector/sharepoint/list.go index 79989983c..dbbc42c33 100644 --- a/src/internal/connector/sharepoint/list.go +++ b/src/internal/connector/sharepoint/list.go @@ -7,11 +7,9 @@ import ( "github.com/alcionai/clues" "github.com/microsoftgraph/msgraph-sdk-go/models" mssite "github.com/microsoftgraph/msgraph-sdk-go/sites" - "github.com/pkg/errors" "github.com/alcionai/corso/src/internal/common/ptr" "github.com/alcionai/corso/src/internal/connector/graph" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/pkg/fault" ) @@ -209,7 +207,7 @@ func fetchListItems( resp, err := builder.Get(ctx, nil) if err != nil { - return nil, errors.Wrap(err, support.ConnectorStackErrorTrace(err)) + return nil, err } for _, itm := range resp.GetValue() { @@ -315,7 +313,7 @@ func fetchContentTypes( resp, err := builder.Get(ctx, nil) if err != nil { - return nil, errors.Wrap(err, support.ConnectorStackErrorTrace(err)) + return nil, err } for _, cont := range resp.GetValue() { diff --git a/src/internal/connector/support/errors.go b/src/internal/connector/support/errors.go deleted file mode 100644 index 26c2e9aca..000000000 --- a/src/internal/connector/support/errors.go +++ /dev/null @@ -1,130 +0,0 @@ -package support - -import ( - "fmt" - "strconv" - "strings" - - multierror "github.com/hashicorp/go-multierror" - msgraph_errors "github.com/microsoftgraph/msgraph-sdk-go/models/odataerrors" - "github.com/pkg/errors" -) - -// WrapErrorAndAppend helper function used to attach identifying information to an error -// and return it as a mulitierror -func WrapAndAppend(identifier string, e, previous error) error { - return multierror.Append(previous, errors.Wrap(e, identifier)) -} - -// WrapErrorAndAppendf format version of WrapErrorAndAppend -func WrapAndAppendf(identifier interface{}, e, previous error) error { - return multierror.Append(previous, errors.Wrapf(e, "%v", identifier)) -} - -// GetErrors Helper method to return the integer amount of errors in multi error -func GetNumberOfErrors(err error) int { - if err == nil { - return 0 - } - - result, _, wasFound := strings.Cut(err.Error(), " ") - if wasFound { - aNum, err := strconv.Atoi(result) - if err == nil { - return aNum - } - } - - return 1 -} - -// ListErrors is a helper method used to return the string of errors when -// the multiError library is used. -// depends on ConnectorStackErrorTrace -func ListErrors(multi multierror.Error) string { - aString := "" - - for idx, err := range multi.Errors { - detail := ConnectorStackErrorTrace(err) - if detail == "" { - detail = fmt.Sprintf("%v", err) - } - - aString = aString + fmt.Sprintf("\n\tErr: %d %v", idx+1, detail) - } - - return aString -} - -// concatenateStringFromPointers is a helper function that adds -// strings to the originalMessage iff the pointer is not nil -func concatenateStringFromPointers(orig string, pointers []*string) string { - for _, pointer := range pointers { - if pointer != nil { - orig = strings.Join([]string{orig, *pointer}, " ") - } - } - - return orig -} - -// ConnectorStackErrorTraceWrap is a helper function that wraps the -// stack trace for oDataErrors (if the error has one) onto the prefix. -// If no stack trace is found, wraps the error with only the prefix. -func ConnectorStackErrorTraceWrap(e error, prefix string) error { - cset := ConnectorStackErrorTrace(e) - if len(cset) > 0 { - return errors.Wrap(e, prefix+": "+cset) - } - - return errors.Wrap(e, prefix) -} - -// ConnectorStackErrorTrace is a helper function that extracts -// the stack trace for oDataErrors, if the error has one. -func ConnectorStackErrorTrace(e error) string { - eMessage := "" - - if oDataError, ok := e.(msgraph_errors.ODataErrorable); ok { - // Get MainError - mainErr := oDataError.GetError() - // message *string - // target *string - // code *string - // details ErrorDetailsable - // Ignoring Additional Detail - code := mainErr.GetCode() - subject := mainErr.GetMessage() - target := mainErr.GetTarget() - details := mainErr.GetDetails() - inners := mainErr.GetInnererror() - eMessage = concatenateStringFromPointers(eMessage, - []*string{code, subject, target}) - - // Get Error Details - // code, message, target - if details != nil { - eMessage = eMessage + "\nDetails Section:" - - for idx, detail := range details { - dMessage := fmt.Sprintf("Detail %d:", idx) - c := detail.GetCode() - m := detail.GetMessage() - t := detail.GetTarget() - dMessage = concatenateStringFromPointers(dMessage, - []*string{c, m, t}) - eMessage = eMessage + dMessage - } - } - - if inners != nil { - eMessage = eMessage + "\nConnector Section:" - client := inners.GetClientRequestId() - rID := inners.GetRequestId() - eMessage = concatenateStringFromPointers(eMessage, - []*string{client, rID}) - } - } - - return eMessage -} diff --git a/src/internal/connector/support/errors_test.go b/src/internal/connector/support/errors_test.go deleted file mode 100644 index b8d39df7a..000000000 --- a/src/internal/connector/support/errors_test.go +++ /dev/null @@ -1,103 +0,0 @@ -package support - -import ( - "errors" - "fmt" - "strings" - "testing" - - multierror "github.com/hashicorp/go-multierror" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/suite" -) - -type GraphConnectorErrorSuite struct { - suite.Suite -} - -func TestGraphConnectorErrorSuite(t *testing.T) { - suite.Run(t, new(GraphConnectorErrorSuite)) -} - -func (suite *GraphConnectorErrorSuite) TestWrapAndAppend() { - err1 := fmt.Errorf("New Error") - err2 := errors.New("I have two") - returnErr := WrapAndAppend("arc376", err2, err1) - suite.True(strings.Contains(returnErr.Error(), "arc376")) - suite.Error(returnErr) - - multi := &multierror.Error{Errors: []error{err1, err2}} - suite.True(strings.Contains(ListErrors(*multi), "two")) // Does not contain the wrapped information - suite.T().Log(ListErrors(*multi)) -} - -func (suite *GraphConnectorErrorSuite) TestWrapAndAppend_OnVar() { - var ( - err1 error - id = "xi2058" - ) - - received := WrapAndAppend(id, errors.New("network error"), err1) - suite.True(strings.Contains(received.Error(), id)) -} - -func (suite *GraphConnectorErrorSuite) TestWrapAndAppend_Add3() { - errOneTwo := WrapAndAppend("user1", assert.AnError, assert.AnError) - combined := WrapAndAppend("unix36", assert.AnError, errOneTwo) - allErrors := WrapAndAppend("fxi92874", assert.AnError, combined) - suite.True(strings.Contains(combined.Error(), "unix36")) - suite.True(strings.Contains(combined.Error(), "user1")) - suite.True(strings.Contains(allErrors.Error(), "fxi92874")) -} - -func (suite *GraphConnectorErrorSuite) TestWrapAndAppendf() { - err1 := assert.AnError - err2 := assert.AnError - combined := WrapAndAppendf(134323, err2, err1) - suite.True(strings.Contains(combined.Error(), "134323")) -} - -func (suite *GraphConnectorErrorSuite) TestConcatenateStringFromPointers() { - var ( - outString string - v1 = "Corso" - v3 = "remains" - s1 = &v1 - s2 *string - s3 = &v3 - ) - - outString = concatenateStringFromPointers(outString, []*string{s1, s2, s3}) - suite.True(strings.Contains(outString, v1)) - suite.True(strings.Contains(outString, v3)) -} - -func (suite *GraphConnectorErrorSuite) TestGetNumberOfErrors() { - table := []struct { - name string - errs error - expected int - }{ - { - name: "No error", - errs: nil, - expected: 0, - }, - { - name: "Not an ErrorList", - errs: errors.New("network error"), - expected: 1, - }, - { - name: "Three Errors", - errs: WrapAndAppend("tres", errors.New("three"), WrapAndAppend("arc376", errors.New("one"), errors.New("two"))), - expected: 3, - }, - } - for _, test := range table { - suite.T().Run(test.name, func(t *testing.T) { - result := GetNumberOfErrors(test.errs) - suite.Equal(result, test.expected) - }) - } -} diff --git a/src/internal/operations/backup_integration_test.go b/src/internal/operations/backup_integration_test.go index 805303802..69bc7d2ce 100644 --- a/src/internal/operations/backup_integration_test.go +++ b/src/internal/operations/backup_integration_test.go @@ -1006,7 +1006,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { require.NotEmpty(t, ids, "message ids in folder") err = cli.MessagesById(ids[0]).Delete(ctx, nil) - require.NoError(t, err, "deleting email item: %s", support.ConnectorStackErrorTrace(err)) + require.NoError(t, err, "deleting email item") case path.ContactsCategory: ids, _, _, err := ac.Contacts().GetAddedAndRemovedItemIDs(ctx, suite.user, containerID, "") @@ -1014,7 +1014,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { require.NotEmpty(t, ids, "contact ids in folder") err = cli.ContactsById(ids[0]).Delete(ctx, nil) - require.NoError(t, err, "deleting contact item: %s", support.ConnectorStackErrorTrace(err)) + require.NoError(t, err, "deleting contact item") case path.EventsCategory: ids, _, _, err := ac.Events().GetAddedAndRemovedItemIDs(ctx, suite.user, containerID, "") @@ -1022,7 +1022,7 @@ func (suite *BackupOpIntegrationSuite) TestBackup_Run_exchangeIncrementals() { require.NotEmpty(t, ids, "event ids in folder") err = cli.CalendarsById(ids[0]).Delete(ctx, nil) - require.NoError(t, err, "deleting calendar: %s", support.ConnectorStackErrorTrace(err)) + require.NoError(t, err, "deleting calendar") } } }, diff --git a/src/pkg/backup/backup.go b/src/pkg/backup/backup.go index 2f0c09671..f8f11712a 100644 --- a/src/pkg/backup/backup.go +++ b/src/pkg/backup/backup.go @@ -7,7 +7,6 @@ import ( "github.com/alcionai/corso/src/cli/print" "github.com/alcionai/corso/src/internal/common" - "github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/internal/model" "github.com/alcionai/corso/src/internal/stats" "github.com/alcionai/corso/src/pkg/fault" @@ -148,20 +147,10 @@ func (b Backup) Values() []string { } func (b Backup) errorCount() int { - var errCount int + errCount := len(b.Errors.Recovered) - // current tracking - if b.ReadErrors != nil || b.WriteErrors != nil { - return support.GetNumberOfErrors(b.ReadErrors) + support.GetNumberOfErrors(b.WriteErrors) - } - - // future tracking - if b.Errors.Failure != nil || len(b.Errors.Recovered) > 0 { - if b.Errors.Failure != nil { - errCount++ - } - - errCount += len(b.Errors.Recovered) + if b.Errors.Failure != nil { + errCount++ } return errCount