add fault/clues to graph_conector.go (#2376)

## Description

Refactors error handling in graph_connector.
Also begins some error refactoring in support by
moving StackTraceErrror style funcs into a more
normalized handler in graph/errors.go.  And
removes the (Non)Recoverable error wraps which
we weren't using anyway.

## Does this PR need a docs update or release note?

- [x] ⛔ No 

## Type of change

- [x] 🧹 Tech Debt/Cleanup

## Issue(s)

* #1970

## Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E

src/internal/connector/data_collections.go

@@ -16,6 +16,8 @@ import (
 	"github.com/alcionai/corso/src/internal/connector/support"
 	"github.com/alcionai/corso/src/internal/data"
 	D "github.com/alcionai/corso/src/internal/diagnostics"
+	"github.com/alcionai/corso/src/pkg/account"
+	"github.com/alcionai/corso/src/pkg/backup/details"
 	"github.com/alcionai/corso/src/pkg/control"
 	"github.com/alcionai/corso/src/pkg/logger"
 	"github.com/alcionai/corso/src/pkg/path"
@@ -226,3 +228,46 @@ func (gc *GraphConnector) OneDriveDataCollections(
 
 	return collections, allExcludes, errs
 }
+
+// RestoreDataCollections restores data from the specified collections
+// into M365 using the GraphAPI.
+// SideEffect: gc.status is updated at the completion of operation
+func (gc *GraphConnector) RestoreDataCollections(
+	ctx context.Context,
+	backupVersion int,
+	acct account.Account,
+	selector selectors.Selector,
+	dest control.RestoreDestination,
+	opts control.Options,
+	dcs []data.RestoreCollection,
+) (*details.Details, error) {
+	ctx, end := D.Span(ctx, "connector:restore")
+	defer end()
+
+	var (
+		status *support.ConnectorOperationStatus
+		err    error
+		deets  = &details.Builder{}
+	)
+
+	creds, err := acct.M365Config()
+	if err != nil {
+		return nil, errors.Wrap(err, "malformed azure credentials")
+	}
+
+	switch selector.Service {
+	case selectors.ServiceExchange:
+		status, err = exchange.RestoreExchangeDataCollections(ctx, creds, gc.Service, dest, dcs, deets)
+	case selectors.ServiceOneDrive:
+		status, err = onedrive.RestoreCollections(ctx, backupVersion, gc.Service, dest, opts, dcs, deets)
+	case selectors.ServiceSharePoint:
+		status, err = sharepoint.RestoreCollections(ctx, backupVersion, creds, gc.Service, dest, dcs, deets)
+	default:
+		err = errors.Errorf("restore data from service %s not supported", selector.Service.String())
+	}
+
+	gc.incrementAwaitingMessages()
+	gc.UpdateStatus(status)
+
+	return deets.Details(), err
+}

src/internal/connector/exchange/api/contacts.go

@@ -291,6 +291,8 @@ func (c Contacts) Serialize(
 		return nil, fmt.Errorf("expected Contactable, got %T", item)
 	}
 
+	ctx = clues.Add(ctx, "item_id", *contact.GetId())
+
 	var (
 		err    error
 		writer = kioser.NewJsonSerializationWriter()
@@ -299,7 +301,7 @@ func (c Contacts) Serialize(
 	defer writer.Close()
 
 	if err = writer.WriteObjectValue("", contact); err != nil {
-		return nil, support.SetNonRecoverableError(errors.Wrap(err, itemID))
+		return nil, clues.Stack(err).WithClues(ctx)
 	}
 
 	bs, err := writer.GetSerializedContent()

src/internal/connector/exchange/api/events.go

@@ -340,6 +340,8 @@ func (c Events) Serialize(
 		return nil, fmt.Errorf("expected Eventable, got %T", item)
 	}
 
+	ctx = clues.Add(ctx, "item_id", *event.GetId())
+
 	var (
 		err    error
 		writer = kioser.NewJsonSerializationWriter()
@@ -348,7 +350,7 @@ func (c Events) Serialize(
 	defer writer.Close()
 
 	if err = writer.WriteObjectValue("", event); err != nil {
-		return nil, support.SetNonRecoverableError(errors.Wrap(err, itemID))
+		return nil, clues.Stack(err).WithClues(ctx)
 	}
 
 	bs, err := writer.GetSerializedContent()

src/internal/connector/exchange/api/mail.go

@@ -321,6 +321,8 @@ func (c Mail) Serialize(
 		return nil, fmt.Errorf("expected Messageable, got %T", item)
 	}
 
+	ctx = clues.Add(ctx, "item_id", *msg.GetId())
+
 	var (
 		err    error
 		writer = kioser.NewJsonSerializationWriter()
@@ -329,7 +331,7 @@ func (c Mail) Serialize(
 	defer writer.Close()
 
 	if err = writer.WriteObjectValue("", msg); err != nil {
-		return nil, support.SetNonRecoverableError(errors.Wrap(err, itemID))
+		return nil, clues.Stack(err).WithClues(ctx)
 	}
 
 	bs, err := writer.GetSerializedContent()

src/internal/connector/graph/errors.go

@@ -2,6 +2,7 @@ package graph
 
 import (
 	"context"
+	"fmt"
 	"net/url"
 	"os"
 
@@ -176,3 +177,50 @@ func hasErrorCode(err error, codes ...string) bool {
 
 	return slices.Contains(codes, *oDataError.GetError().GetCode())
 }
+
+// ErrData is a helper function that extracts ODataError metadata from
+// the error.  If the error is not an ODataError type, returns an empty
+// slice.  The returned value is guaranteed to be an even-length pairing
+// of key, value tuples.
+func ErrData(e error) []any {
+	result := make([]any, 0)
+
+	if e == nil {
+		return result
+	}
+
+	odErr, ok := e.(odataerrors.ODataErrorable)
+	if !ok {
+		return result
+	}
+
+	// Get MainError
+	mainErr := odErr.GetError()
+
+	result = appendIf(result, "odataerror_code", mainErr.GetCode())
+	result = appendIf(result, "odataerror_message", mainErr.GetMessage())
+	result = appendIf(result, "odataerror_target", mainErr.GetTarget())
+
+	for i, d := range mainErr.GetDetails() {
+		pfx := fmt.Sprintf("odataerror_details_%d_", i)
+		result = appendIf(result, pfx+"code", d.GetCode())
+		result = appendIf(result, pfx+"message", d.GetMessage())
+		result = appendIf(result, pfx+"target", d.GetTarget())
+	}
+
+	inner := mainErr.GetInnererror()
+	if inner != nil {
+		result = appendIf(result, "odataerror_inner_cli_req_id", inner.GetClientRequestId())
+		result = appendIf(result, "odataerror_inner_req_id", inner.GetRequestId())
+	}
+
+	return result
+}
+
+func appendIf(a []any, k string, v *string) []any {
+	if v == nil {
+		return a
+	}
+
+	return append(a, k, *v)
+}

src/internal/connector/graph_connector.go

@@ -4,11 +4,14 @@ package connector
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"runtime/trace"
 	"strings"
 	"sync"
 
+	"github.com/alcionai/clues"
+	"github.com/hashicorp/go-multierror"
 	"github.com/microsoft/kiota-abstractions-go/serialization"
 	msgraphgocore "github.com/microsoftgraph/msgraph-sdk-go-core"
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
@@ -17,18 +20,12 @@ import (
 
 	"github.com/alcionai/corso/src/internal/connector/discovery"
 	"github.com/alcionai/corso/src/internal/connector/discovery/api"
-	"github.com/alcionai/corso/src/internal/connector/exchange"
 	"github.com/alcionai/corso/src/internal/connector/graph"
-	"github.com/alcionai/corso/src/internal/connector/onedrive"
 	"github.com/alcionai/corso/src/internal/connector/sharepoint"
 	"github.com/alcionai/corso/src/internal/connector/support"
-	"github.com/alcionai/corso/src/internal/data"
 	D "github.com/alcionai/corso/src/internal/diagnostics"
 	"github.com/alcionai/corso/src/pkg/account"
-	"github.com/alcionai/corso/src/pkg/backup/details"
-	"github.com/alcionai/corso/src/pkg/control"
 	"github.com/alcionai/corso/src/pkg/filters"
-	"github.com/alcionai/corso/src/pkg/selectors"
 )
 
 // ---------------------------------------------------------------------------
@@ -74,7 +71,7 @@ func NewGraphConnector(
 ) (*GraphConnector, error) {
 	m365, err := acct.M365Config()
 	if err != nil {
-		return nil, errors.Wrap(err, "retrieving m365 account configuration")
+		return nil, clues.Wrap(err, "retrieving m365 account configuration").WithClues(ctx)
 	}
 
 	gc := GraphConnector{
@@ -87,12 +84,12 @@ func NewGraphConnector(
 
 	gc.Service, err = gc.createService()
 	if err != nil {
-		return nil, errors.Wrap(err, "creating service connection")
+		return nil, clues.Wrap(err, "creating service connection").WithClues(ctx)
 	}
 
 	gc.Owners, err = api.NewClient(m365)
 	if err != nil {
-		return nil, errors.Wrap(err, "creating api client")
+		return nil, clues.Wrap(err, "creating api client").WithClues(ctx)
 	}
 
 	// TODO(ashmrtn): When selectors only encapsulate a single resource owner that
@@ -174,8 +171,7 @@ func (gc *GraphConnector) setTenantSites(ctx context.Context) error {
 		gc.tenant,
 		sharepoint.GetAllSitesForTenant,
 		models.CreateSiteCollectionResponseFromDiscriminatorValue,
-		identifySite,
+		identifySite)
-	)
 	if err != nil {
 		return err
 	}
@@ -194,22 +190,23 @@ const personalSitePath = "sharepoint.com/personal/"
 func identifySite(item any) (string, string, error) {
 	m, ok := item.(models.Siteable)
 	if !ok {
-		return "", "", errors.New("iteration retrieved non-Site item")
+		return "", "", clues.New("iteration retrieved non-Site item").With("item_type", fmt.Sprintf("%T", item))
 	}
 
 	if m.GetName() == nil {
 		// the built-in site at "https://{tenant-domain}/search" never has a name.
 		if m.GetWebUrl() != nil && strings.HasSuffix(*m.GetWebUrl(), "/search") {
-			return "", "", errKnownSkippableCase
+			// TODO: pii siteID, on this and all following cases
+			return "", "", clues.Stack(errKnownSkippableCase).With("site_id", *m.GetId())
 		}
 
-		return "", "", errors.Errorf("no name for Site: %s", *m.GetId())
+		return "", "", clues.New("site has no name").With("site_id", *m.GetId())
 	}
 
 	// personal (ie: oneDrive) sites have to be filtered out server-side.
 	url := m.GetWebUrl()
 	if url != nil && strings.Contains(*url, personalSitePath) {
-		return "", "", errKnownSkippableCase
+		return "", "", clues.Stack(errKnownSkippableCase).With("site_id", *m.GetId())
 	}
 
 	return *m.GetWebUrl(), *m.GetId(), nil
@@ -261,49 +258,6 @@ func (gc *GraphConnector) UnionSiteIDsAndWebURLs(ctx context.Context, ids, urls
 	return idsl, nil
 }
 
-// RestoreDataCollections restores data from the specified collections
-// into M365 using the GraphAPI.
-// SideEffect: gc.status is updated at the completion of operation
-func (gc *GraphConnector) RestoreDataCollections(
-	ctx context.Context,
-	backupVersion int,
-	acct account.Account,
-	selector selectors.Selector,
-	dest control.RestoreDestination,
-	opts control.Options,
-	dcs []data.RestoreCollection,
-) (*details.Details, error) {
-	ctx, end := D.Span(ctx, "connector:restore")
-	defer end()
-
-	var (
-		status *support.ConnectorOperationStatus
-		err    error
-		deets  = &details.Builder{}
-	)
-
-	creds, err := acct.M365Config()
-	if err != nil {
-		return nil, errors.Wrap(err, "malformed azure credentials")
-	}
-
-	switch selector.Service {
-	case selectors.ServiceExchange:
-		status, err = exchange.RestoreExchangeDataCollections(ctx, creds, gc.Service, dest, dcs, deets)
-	case selectors.ServiceOneDrive:
-		status, err = onedrive.RestoreCollections(ctx, backupVersion, gc.Service, dest, opts, dcs, deets)
-	case selectors.ServiceSharePoint:
-		status, err = sharepoint.RestoreCollections(ctx, backupVersion, creds, gc.Service, dest, dcs, deets)
-	default:
-		err = errors.Errorf("restore data from service %s not supported", selector.Service.String())
-	}
-
-	gc.incrementAwaitingMessages()
-	gc.UpdateStatus(status)
-
-	return deets.Details(), err
-}
-
 // AwaitStatus waits for all gc tasks to complete and then returns status
 func (gc *GraphConnector) AwaitStatus() *support.ConnectorOperationStatus {
 	defer func() {
@@ -359,30 +313,27 @@ func getResources(
 
 	response, err := query(ctx, gs)
 	if err != nil {
-		return nil, errors.Wrapf(
+		return nil, clues.Wrap(err, "retrieving tenant's resources").
-			err,
+			WithClues(ctx).
-			"retrieving resources for tenant %s: %s",
+			WithAll(graph.ErrData(err)...)
-			tenantID,
-			support.ConnectorStackErrorTrace(err),
-		)
 	}
 
+	errs := &multierror.Error{}
+
 	iter, err := msgraphgocore.NewPageIterator(response, gs.Adapter(), parser)
 	if err != nil {
-		return nil, errors.Wrap(err, support.ConnectorStackErrorTrace(err))
+		return nil, clues.Stack(err).WithClues(ctx).WithAll(graph.ErrData(err)...)
 	}
 
-	var iterErrs error
-
 	callbackFunc := func(item any) bool {
 		k, v, err := identify(item)
 		if err != nil {
-			if errors.Is(err, errKnownSkippableCase) {
+			if !errors.Is(err, errKnownSkippableCase) {
-				return true
+				errs = multierror.Append(errs, clues.Stack(err).
+					WithClues(ctx).
+					With("query_url", gs.Adapter().GetBaseUrl()))
 			}
 
-			iterErrs = support.WrapAndAppend(gs.Adapter().GetBaseUrl(), err, iterErrs)
-
 			return true
 		}
 
@@ -392,20 +343,8 @@ func getResources(
 	}
 
 	if err := iter.Iterate(ctx, callbackFunc); err != nil {
-		return nil, errors.Wrap(err, support.ConnectorStackErrorTrace(err))
+		return nil, clues.Stack(err).WithClues(ctx).WithAll(graph.ErrData(err)...)
 	}
 
-	return resources, iterErrs
+	return resources, errs.ErrorOrNil()
-}
-
-// IsRecoverableError returns true iff error is a RecoverableGCEerror
-func IsRecoverableError(e error) bool {
-	var recoverable support.RecoverableGCError
-	return errors.As(e, &recoverable)
-}
-
-// IsNonRecoverableError returns true iff error is a NonRecoverableGCEerror
-func IsNonRecoverableError(e error) bool {
-	var nonRecoverable support.NonRecoverableGCError
-	return errors.As(e, &nonRecoverable)
 }

src/internal/connector/graph_connector_disconnected_test.go

@@ -116,58 +116,6 @@ func (suite *DisconnectedGraphConnectorSuite) TestGraphConnector_Status() {
 	suite.Equal(2, gc.Status().FolderCount)
 }
 
-func (suite *DisconnectedGraphConnectorSuite) TestGraphConnector_ErrorChecking() {
-	tests := []struct {
-		name                 string
-		err                  error
-		returnRecoverable    assert.BoolAssertionFunc
-		returnNonRecoverable assert.BoolAssertionFunc
-	}{
-		{
-			name:                 "Neither Option",
-			err:                  errors.New("regular error"),
-			returnRecoverable:    assert.False,
-			returnNonRecoverable: assert.False,
-		},
-		{
-			name:                 "Validate Recoverable",
-			err:                  support.SetRecoverableError(errors.New("Recoverable")),
-			returnRecoverable:    assert.True,
-			returnNonRecoverable: assert.False,
-		},
-		{
-			name:                 "Validate NonRecoverable",
-			err:                  support.SetNonRecoverableError(errors.New("Non-recoverable")),
-			returnRecoverable:    assert.False,
-			returnNonRecoverable: assert.True,
-		},
-		{
-			name: "Wrapped Recoverable",
-			err: support.WrapAndAppend(
-				"Wrapped Recoverable",
-				support.SetRecoverableError(errors.New("Recoverable")),
-				nil),
-			returnRecoverable:    assert.True,
-			returnNonRecoverable: assert.False,
-		},
-		{
-			name:                 "On Nil",
-			err:                  nil,
-			returnRecoverable:    assert.False,
-			returnNonRecoverable: assert.False,
-		},
-	}
-	for _, test := range tests {
-		suite.T().Run(test.name, func(t *testing.T) {
-			recoverable := IsRecoverableError(test.err)
-			nonRecoverable := IsNonRecoverableError(test.err)
-			test.returnRecoverable(suite.T(), recoverable, "Test: %s Recoverable-received %v", test.name, recoverable)
-			test.returnNonRecoverable(suite.T(), nonRecoverable, "Test: %s non-recoverable: %v", test.name, nonRecoverable)
-			t.Logf("Is nil: %v", test.err == nil)
-		})
-	}
-}
-
 func (suite *DisconnectedGraphConnectorSuite) TestVerifyBackupInputs() {
 	users := []string{
 		"elliotReid@someHospital.org",

src/internal/connector/support/errors.go

@@ -8,29 +8,8 @@ import (
 	multierror "github.com/hashicorp/go-multierror"
 	msgraph_errors "github.com/microsoftgraph/msgraph-sdk-go/models/odataerrors"
 	"github.com/pkg/errors"
-
-	"github.com/alcionai/corso/src/internal/common"
 )
 
-// GraphConnector has two types of errors that are exported
-// RecoverableGCError is a query error that can be overcome with time
-type RecoverableGCError struct {
-	common.Err
-}
-
-func SetRecoverableError(e error) error {
-	return RecoverableGCError{*common.EncapsulateError(e)}
-}
-
-// NonRecoverableGCError is a permanent query error
-type NonRecoverableGCError struct {
-	common.Err
-}
-
-func SetNonRecoverableError(e error) error {
-	return NonRecoverableGCError{*common.EncapsulateError(e)}
-}
-
 // 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 {
@@ -101,7 +80,7 @@ func ConnectorStackErrorTraceWrap(e error, prefix string) error {
 	return errors.Wrap(e, prefix)
 }
 
-// ConnectorStackErrorTracew is a helper function that extracts
+// ConnectorStackErrorTrace is a helper function that extracts
 // the stack trace for oDataErrors, if the error has one.
 func ConnectorStackErrorTrace(e error) string {
 	eMessage := ""

src/internal/connector/support/errors_test.go

@@ -41,26 +41,6 @@ func (suite *GraphConnectorErrorSuite) TestWrapAndAppend_OnVar() {
 	suite.True(strings.Contains(received.Error(), id))
 }
 
-func (suite *GraphConnectorErrorSuite) TestAsRecoverableError() {
-	err := assert.AnError
-
-	rcv := RecoverableGCError{}
-	suite.False(errors.As(err, &rcv))
-
-	aRecoverable := SetRecoverableError(err)
-	suite.True(errors.As(aRecoverable, &rcv))
-}
-
-func (suite *GraphConnectorErrorSuite) TestAsNonRecoverableError() {
-	err := assert.AnError
-
-	noRecover := NonRecoverableGCError{}
-	suite.False(errors.As(err, &noRecover))
-
-	nonRecoverable := SetNonRecoverableError(err)
-	suite.True(errors.As(nonRecoverable, &noRecover))
-}
-
 func (suite *GraphConnectorErrorSuite) TestWrapAndAppend_Add3() {
 	errOneTwo := WrapAndAppend("user1", assert.AnError, assert.AnError)
 	combined := WrapAndAppend("unix36", assert.AnError, errOneTwo)

src/internal/operations/backup_integration_test.go

@@ -347,8 +347,7 @@ func generateContainerOfItems(
 		sel,
 		dest,
 		control.Options{RestorePermissions: true},
-		dataColls,
+		dataColls)
-	)
 	require.NoError(t, err)
 
 	return deets

src/pkg/fault/fault.go

@@ -84,6 +84,11 @@ func (e *Errors) Fail(err error) *Errors {
 	return e.setErr(err)
 }
 
+// Failed returns true if e.err != nil, signifying a catastrophic failure.
+func (e *Errors) Failed() bool {
+	return e.err != nil
+}
+
 // setErr handles setting errors.err.  Sync locking gets
 // handled upstream of this call.
 func (e *Errors) setErr(err error) *Errors {
@@ -99,6 +104,7 @@ func (e *Errors) setErr(err error) *Errors {
 
 type Adder interface {
 	Add(err error) *Errors
+	Failed() bool
 }
 
 // Add appends the error to the slice of recoverable and

src/pkg/fault/mock/mock.go

@@ -4,7 +4,8 @@ import "github.com/alcionai/corso/src/pkg/fault"
 
 // Adder mocks an adder interface for testing.
 type Adder struct {
-	Errs []error
+	FailFast bool
+	Errs     []error
 }
 
 func NewAdder() *Adder {
@@ -15,3 +16,7 @@ func (ma *Adder) Add(err error) *fault.Errors {
 	ma.Errs = append(ma.Errs, err)
 	return fault.New(true)
 }
+
+func (ma *Adder) Failed() bool {
+	return ma.FailFast && len(ma.Errs) > 0
+}