wrap up clues and fault additions to exchange (#2504)

## 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

@@ -2,12 +2,12 @@ package connector
 
 import (
 	"context"
-	"fmt"
 	"strings"
 
 	"github.com/pkg/errors"
 	"golang.org/x/exp/maps"
 
+	"github.com/alcionai/clues"
 	"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"
@@ -46,10 +46,14 @@ func (gc *GraphConnector) DataCollections(
 
 	err := verifyBackupInputs(sels, gc.GetUsers(), gc.GetSiteIDs())
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, clues.Stack(err).WithClues(ctx)
 	}
 
-	serviceEnabled, err := checkServiceEnabled(ctx, gc.Owners.Users(), path.ServiceType(sels.Service), sels.DiscreteOwner)
+	serviceEnabled, err := checkServiceEnabled(
+		ctx,
+		gc.Owners.Users(),
+		path.ServiceType(sels.Service),
+		sels.DiscreteOwner)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -65,7 +69,6 @@ func (gc *GraphConnector) DataCollections(
 			sels,
 			metadata,
 			gc.credentials,
-			// gc.Service,
 			gc.UpdateStatus,
 			ctrlOpts,
 			errs)
@@ -110,7 +113,7 @@ func (gc *GraphConnector) DataCollections(
 		return colls, excludes, nil
 
 	default:
-		return nil, nil, errors.Errorf("service %s not supported", sels.Service.String())
+		return nil, nil, clues.Wrap(clues.New(sels.Service.String()), "service not supported").WithClues(ctx)
 	}
 }
 
@@ -137,7 +140,7 @@ func verifyBackupInputs(sels selectors.Selector, userPNs, siteIDs []string) erro
 	}
 
 	if !found {
-		return fmt.Errorf("resource owner [%s] not found within tenant", sels.DiscreteOwner)
+		return clues.New("resource owner not found within tenant").With("missing_resource_owner", sels.DiscreteOwner)
 	}
 
 	return nil
@@ -192,19 +195,18 @@ func (gc *GraphConnector) OneDriveDataCollections(
 ) ([]data.BackupCollection, map[string]struct{}, error) {
 	odb, err := selector.ToOneDriveBackup()
 	if err != nil {
-		return nil, nil, errors.Wrap(err, "oneDriveDataCollection: parsing selector")
+		return nil, nil, clues.Wrap(err, "parsing selector").WithClues(ctx)
 	}
 
 	var (
 		user        = selector.DiscreteOwner
 		collections = []data.BackupCollection{}
 		allExcludes = map[string]struct{}{}
-		errs        error
 	)
 
 	// for each scope that includes oneDrive items, get all
 	for _, scope := range odb.Scopes() {
-		logger.Ctx(ctx).With("user", user).Debug("Creating OneDrive collections")
+		logger.Ctx(ctx).Debug("creating OneDrive collections")
 
 		odcs, excludes, err := onedrive.NewCollections(
 			gc.itemClient,
@@ -217,7 +219,7 @@ func (gc *GraphConnector) OneDriveDataCollections(
 			ctrlOpts,
 		).Get(ctx, metadata)
 		if err != nil {
-			return nil, nil, support.WrapAndAppend(user, err, errs)
+			return nil, nil, err
 		}
 
 		collections = append(collections, odcs...)
@@ -232,7 +234,7 @@ func (gc *GraphConnector) OneDriveDataCollections(
 		}
 	}
 
-	return collections, allExcludes, errs
+	return collections, allExcludes, nil
 }
 
 // RestoreDataCollections restores data from the specified collections
@@ -253,7 +255,6 @@ func (gc *GraphConnector) RestoreDataCollections(
 
 	var (
 		status *support.ConnectorOperationStatus
-		err    error
 		deets  = &details.Builder{}
 	)
 
@@ -270,7 +271,7 @@ func (gc *GraphConnector) RestoreDataCollections(
 	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())
+		err = clues.Wrap(clues.New(selector.Service.String()), "service not supported")
 	}
 
 	gc.incrementAwaitingMessages()

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

@@ -47,7 +47,7 @@ func (c Contacts) CreateContactFolder(
 
 	mdl, err := c.stable.Client().UsersById(user).ContactFolders().Post(ctx, requestBody, nil)
 	if err != nil {
-		return nil, clues.Stack(err).WithClues(ctx).WithAll(graph.ErrData(err)...)
+		return nil, clues.Wrap(err, "creating contact folder").WithClues(ctx).WithAll(graph.ErrData(err)...)
 	}
 
 	return mdl, nil

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

@@ -48,7 +48,7 @@ func (c Events) CreateCalendar(
 
 	mdl, err := c.stable.Client().UsersById(user).Calendars().Post(ctx, requestbody, nil)
 	if err != nil {
-		return nil, clues.Stack(err).WithClues(ctx).WithAll(graph.ErrData(err)...)
+		return nil, clues.Wrap(err, "creating calendar").WithClues(ctx).WithAll(graph.ErrData(err)...)
 	}
 
 	return mdl, nil
@@ -238,7 +238,7 @@ func (c Events) GetAddedAndRemovedItemIDs(
 
 	ctx = clues.AddAll(
 		ctx,
-		"calendar_id", calendarID)
+		"container_id", calendarID)
 
 	if len(oldDelta) > 0 {
 		var (

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

@@ -48,7 +48,7 @@ func (c Mail) CreateMailFolder(
 
 	mdl, err := c.stable.Client().UsersById(user).MailFolders().Post(ctx, requestBody, nil)
 	if err != nil {
-		return nil, clues.Stack(err).WithClues(ctx).WithAll(graph.ErrData(err)...)
+		return nil, clues.Wrap(err, "creating mail folder").WithClues(ctx).WithAll(graph.ErrData(err)...)
 	}
 
 	return mdl, nil
@@ -75,7 +75,7 @@ func (c Mail) CreateMailFolderWithParent(
 		ChildFolders().
 		Post(ctx, requestBody, nil)
 	if err != nil {
-		return nil, clues.Stack(err).WithClues(ctx).WithAll(graph.ErrData(err)...)
+		return nil, clues.Wrap(err, "creating nested mail folder").WithClues(ctx).WithAll(graph.ErrData(err)...)
 	}
 
 	return mdl, nil
@@ -250,7 +250,7 @@ func (c Mail) GetAddedAndRemovedItemIDs(
 	ctx = clues.AddAll(
 		ctx,
 		"category", selectors.ExchangeMail,
-		"folder_id", directoryID)
+		"container_id", directoryID)
 
 	options, err := optionsForFolderMessagesDelta([]string{"isRead"})
 	if err != nil {

src/internal/connector/exchange/attachment.go

@@ -6,7 +6,6 @@ import (
 	"io"
 
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
-	"github.com/pkg/errors"
 
 	"github.com/alcionai/clues"
 	"github.com/alcionai/corso/src/internal/common/ptr"
@@ -58,11 +57,11 @@ func uploadAttachment(
 		"internal_item_type", getItemAttachmentItemType(attachment),
 		"uploader_item_id", uploader.getItemID())
 
-	logger.Ctx(ctx).Debugw("uploading attachment")
+	logger.Ctx(ctx).Debug("uploading attachment")
 
 	// Reference attachments that are inline() do not need to be recreated. The contents are part of the body.
 	if attachmentType == models.REFERENCE_ATTACHMENTTYPE && ptr.Val(attachment.GetIsInline()) {
-		logger.Ctx(ctx).Debug("skip uploading inline reference attachment: ", ptr.Val(attachment.GetName()))
+		logger.Ctx(ctx).Debug("skip uploading inline reference attachment")
 		return nil
 	}
 
@@ -95,24 +94,26 @@ func uploadLargeAttachment(
 	uploader attachmentUploadable,
 	attachment models.Attachmentable,
 ) error {
-	ab := attachmentBytes(attachment)
-	size := int64(len(ab))
+	var (
+		bs   = attachmentBytes(attachment)
+		size = int64(len(bs))
+	)
 
-	session, err := uploader.uploadSession(ctx, *attachment.GetName(), size)
+	session, err := uploader.uploadSession(ctx, ptr.Val(attachment.GetName()), size)
 	if err != nil {
-		return err
+		return clues.Stack(err).WithClues(ctx)
 	}
 
-	url := *session.GetUploadUrl()
+	url := ptr.Val(session.GetUploadUrl())
 	aw := uploadsession.NewWriter(uploader.getItemID(), url, size)
-	logger.Ctx(ctx).Debugf("Created an upload session for item %s. URL: %s", uploader.getItemID(), url)
+	logger.Ctx(ctx).Debugw("uploading large attachment", "attachment_url", url) // TODO: url pii
 
 	// Upload the stream data
 	copyBuffer := make([]byte, attachmentChunkSize)
 
-	_, err = io.CopyBuffer(aw, bytes.NewReader(ab), copyBuffer)
+	_, err = io.CopyBuffer(aw, bytes.NewReader(bs), copyBuffer)
 	if err != nil {
-		return errors.Wrapf(err, "failed to upload attachment: item %s", uploader.getItemID())
+		return clues.Wrap(err, "uploading large attachment").WithClues(ctx)
 	}
 
 	return nil

src/internal/connector/exchange/cache_container.go

@@ -3,20 +3,21 @@ package exchange
 import (
 	"github.com/pkg/errors"
 
+	"github.com/alcionai/clues"
+	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/internal/connector/graph"
 )
 
 // checkIDAndName is a helper function to ensure that
 // the ID and name pointers are set prior to being called.
 func checkIDAndName(c graph.Container) error {
-	idPtr := c.GetId()
-	if idPtr == nil || len(*idPtr) == 0 {
-		return errors.New("folder without ID")
+	id, ok := ptr.ValOK(c.GetId())
+	if !ok {
+		return errors.New("container missing ID")
 	}
 
-	ptr := c.GetDisplayName()
-	if ptr == nil || len(*ptr) == 0 {
-		return errors.Errorf("folder %s without display name", *idPtr)
+	if _, ok := ptr.ValOK(c.GetDisplayName()); !ok {
+		return clues.New("container missing display name").With("container_id", id)
 	}
 
 	return nil
@@ -29,9 +30,8 @@ func checkRequiredValues(c graph.Container) error {
 		return err
 	}
 
-	ptr := c.GetParentFolderId()
-	if ptr == nil || len(*ptr) == 0 {
-		return errors.Errorf("folder %s without parent ID", *c.GetId())
+	if _, ok := ptr.ValOK(c.GetParentFolderId()); !ok {
+		return clues.New("container missing parent ID").With("container_id", ptr.Val(c.GetId()))
 	}
 
 	return nil

src/internal/connector/exchange/container_resolver_test.go

@@ -96,7 +96,7 @@ var (
 				displayName: &testName,
 				parentID:    &testParentID,
 			},
-			check: assert.Error,
+			check: assert.NoError,
 		},
 		{
 			name: "EmptyDisplayName",
@@ -105,7 +105,7 @@ var (
 				displayName: &emptyString,
 				parentID:    &testParentID,
 			},
-			check: assert.Error,
+			check: assert.NoError,
 		},
 		{
 			name: "AllValues",
@@ -145,7 +145,7 @@ func (suite *FolderCacheUnitSuite) TestCheckRequiredValues() {
 				displayName: &testName,
 				parentID:    &emptyString,
 			},
-			check: assert.Error,
+			check: assert.NoError,
 		},
 	}
 

src/internal/connector/graph/api/api.go

@@ -1,5 +1,7 @@
 package api
 
+import "github.com/alcionai/corso/src/internal/common/ptr"
+
 type PageLinker interface {
 	GetOdataNextLink() *string
 }
@@ -10,21 +12,9 @@ type DeltaPageLinker interface {
 }
 
 func NextLink(pl PageLinker) string {
-	next := pl.GetOdataNextLink()
-	if next == nil || len(*next) == 0 {
-		return ""
-	}
-
-	return *next
+	return ptr.Val(pl.GetOdataNextLink())
 }
 
 func NextAndDeltaLink(pl DeltaPageLinker) (string, string) {
-	next := NextLink(pl)
-
-	delta := pl.GetOdataDeltaLink()
-	if delta == nil || len(*delta) == 0 {
-		return next, ""
-	}
-
-	return next, *delta
+	return NextLink(pl), ptr.Val(pl.GetOdataDeltaLink())
 }

src/internal/connector/graph/cache_container.go

@@ -3,9 +3,11 @@ package graph
 import (
 	"context"
 
+	"github.com/alcionai/clues"
 	"github.com/microsoftgraph/msgraph-sdk-go/models"
 	"github.com/pkg/errors"
 
+	"github.com/alcionai/corso/src/internal/common/ptr"
 	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/path"
 )
@@ -171,19 +173,17 @@ func CreateCalendarDisplayable(entry any, parentID string) *CalendarDisplayable
 // checkRequiredValues is a helper function to ensure that
 // all the pointers are set prior to being called.
 func CheckRequiredValues(c Container) error {
-	idPtr := c.GetId()
-	if idPtr == nil || len(*idPtr) == 0 {
-		return errors.New("folder without ID")
+	id, ok := ptr.ValOK(c.GetId())
+	if !ok {
+		return errors.New("container missing ID")
 	}
 
-	ptr := c.GetDisplayName()
-	if ptr == nil || len(*ptr) == 0 {
-		return errors.Errorf("folder %s without display name", *idPtr)
+	if _, ok := ptr.ValOK(c.GetDisplayName()); !ok {
+		return clues.New("container missing display name").With("container_id", id)
 	}
 
-	ptr = c.GetParentFolderId()
-	if ptr == nil || len(*ptr) == 0 {
-		return errors.Errorf("folder %s without parent ID", *idPtr)
+	if _, ok := ptr.ValOK(c.GetParentFolderId()); !ok {
+		return clues.New("container missing parent ID").With("container_id", id)
 	}
 
 	return nil

src/internal/connector/graph/service.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azidentity"
+	"github.com/alcionai/clues"
 	backoff "github.com/cenkalti/backoff/v4"
 	"github.com/microsoft/kiota-abstractions-go/serialization"
 	ka "github.com/microsoft/kiota-authentication-azure-go"
@@ -15,7 +16,6 @@ import (
 	msgraphgocore "github.com/microsoftgraph/msgraph-sdk-go-core"
 	"github.com/pkg/errors"
 
-	"github.com/alcionai/corso/src/internal/connector/support"
 	"github.com/alcionai/corso/src/pkg/account"
 	"github.com/alcionai/corso/src/pkg/logger"
 	"github.com/alcionai/corso/src/pkg/path"
@@ -79,7 +79,7 @@ func (s Service) Serialize(object serialization.Parsable) ([]byte, error) {
 
 	err = writer.WriteObjectValue("", object)
 	if err != nil {
-		return nil, errors.Wrap(err, "writeObjecValue serialization")
+		return nil, errors.Wrap(err, "serializing object")
 	}
 
 	return writer.GetSerializedContent()
@@ -161,7 +161,7 @@ func CreateAdapter(tenant, client, secret string, opts ...option) (*msgraphsdk.G
 	// Client Provider: Uses Secret for access to tenant-level data
 	cred, err := azidentity.NewClientSecretCredential(tenant, client, secret, nil)
 	if err != nil {
-		return nil, errors.Wrap(err, "creating m365 client secret credentials")
+		return nil, errors.Wrap(err, "creating m365 client identity")
 	}
 
 	auth, err := ka.NewAzureIdentityAuthenticationProviderWithScopes(
@@ -169,13 +169,15 @@ func CreateAdapter(tenant, client, secret string, opts ...option) (*msgraphsdk.G
 		[]string{"https://graph.microsoft.com/.default"},
 	)
 	if err != nil {
-		return nil, errors.Wrap(err, "creating new AzureIdentityAuthentication")
+		return nil, errors.Wrap(err, "creating azure authentication")
 	}
 
 	httpClient := HTTPClient(opts...)
 
 	return msgraphsdk.NewGraphRequestAdapterWithParseNodeFactoryAndSerializationWriterFactoryAndHttpClient(
-		auth, nil, nil, httpClient)
+		auth,
+		nil, nil,
+		httpClient)
 }
 
 // HTTPClient creates the httpClient with middlewares and timeout configured
@@ -327,14 +329,14 @@ func (middleware RetryHandler) Intercept(
 
 	response, err := pipeline.Next(req, middlewareIndex)
 	if err != nil && !IsErrTimeout(err) {
-		return response, support.ConnectorStackErrorTraceWrap(err, "maximum retries or unretryable")
+		return nil, clues.Stack(err).WithClues(ctx).WithAll(ErrData(err)...)
 	}
 
 	exponentialBackOff := backoff.NewExponentialBackOff()
 	exponentialBackOff.InitialInterval = middleware.Delay
 	exponentialBackOff.Reset()
 
-	return middleware.retryRequest(
+	response, err = middleware.retryRequest(
 		ctx,
 		pipeline,
 		middlewareIndex,
@@ -344,4 +346,9 @@ func (middleware RetryHandler) Intercept(
 		0,
 		exponentialBackOff,
 		err)
+	if err != nil {
+		return nil, clues.Stack(err).WithClues(ctx).WithAll(ErrData(err)...)
+	}
+
+	return response, nil
 }

src/internal/connector/graph_connector.go

@@ -192,7 +192,7 @@ const personalSitePath = "sharepoint.com/personal/"
 func identifySite(item any) (string, string, error) {
 	m, ok := item.(models.Siteable)
 	if !ok {
-		return "", "", clues.New("iteration retrieved non-Site item").With("item_type", fmt.Sprintf("%T", item))
+		return "", "", clues.New("non-Siteable item").With("item_type", fmt.Sprintf("%T", item))
 	}
 
 	if m.GetName() == nil {

src/internal/operations/backup_integration_test.go

@@ -146,8 +146,7 @@ func runAndCheckBackup(
 		Completed,
 		bo.Status,
 		"backup status should be Completed, got %s",
-		bo.Status,
-	)
+		bo.Status)
 	require.Less(t, 0, bo.Results.ItemsWritten)
 
 	assert.Less(t, 0, bo.Results.ItemsRead, "count of items read")