adding fault/clues to non-service connector pkgs (#2380)

## 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
This commit is contained in:
Keepers 2023-02-13 09:54:09 -07:00 committed by GitHub
parent 4739b3be9b
commit 680ec2b751
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 52 additions and 70 deletions

View File

@ -3,6 +3,7 @@ package api
import ( import (
"context" "context"
"github.com/alcionai/clues"
absser "github.com/microsoft/kiota-abstractions-go" absser "github.com/microsoft/kiota-abstractions-go"
msgraphgocore "github.com/microsoftgraph/msgraph-sdk-go-core" msgraphgocore "github.com/microsoftgraph/msgraph-sdk-go-core"
"github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/microsoftgraph/msgraph-sdk-go/models"
@ -10,7 +11,7 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/connector/graph" "github.com/alcionai/corso/src/internal/connector/graph"
"github.com/alcionai/corso/src/internal/connector/support" "github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/path" "github.com/alcionai/corso/src/pkg/path"
) )
@ -85,7 +86,7 @@ func userOptions(fs *string) *users.UsersRequestBuilderGetRequestConfiguration {
} }
// GetAll retrieves all users. // GetAll retrieves all users.
func (c Users) GetAll(ctx context.Context) ([]models.Userable, error) { func (c Users) GetAll(ctx context.Context, errs *fault.Errors) ([]models.Userable, error) {
service, err := c.service() service, err := c.service()
if err != nil { if err != nil {
return nil, err return nil, err
@ -99,7 +100,7 @@ func (c Users) GetAll(ctx context.Context) ([]models.Userable, error) {
}) })
if err != nil { if err != nil {
return nil, support.ConnectorStackErrorTraceWrap(err, "getting all users") return nil, clues.Wrap(err, "getting all users").WithClues(ctx).WithAll(graph.ErrData(err)...)
} }
iter, err := msgraphgocore.NewPageIterator( iter, err := msgraphgocore.NewPageIterator(
@ -107,18 +108,19 @@ func (c Users) GetAll(ctx context.Context) ([]models.Userable, error) {
service.Adapter(), service.Adapter(),
models.CreateUserCollectionResponseFromDiscriminatorValue) models.CreateUserCollectionResponseFromDiscriminatorValue)
if err != nil { if err != nil {
return nil, support.ConnectorStackErrorTraceWrap(err, "constructing user iterator") return nil, clues.Wrap(err, "creating users iterator").WithClues(ctx).WithAll(graph.ErrData(err)...)
} }
var ( us := make([]models.Userable, 0)
iterErrs error
us = make([]models.Userable, 0)
)
iterator := func(item any) bool { iterator := func(item any) bool {
if errs.Failed() {
return false
}
u, err := validateUser(item) u, err := validateUser(item)
if err != nil { if err != nil {
iterErrs = support.WrapAndAppend("validating user", err, iterErrs) errs.Add(clues.Wrap(err, "validating user").WithClues(ctx).WithAll(graph.ErrData(err)...))
} else { } else {
us = append(us, u) us = append(us, u)
} }
@ -127,10 +129,10 @@ func (c Users) GetAll(ctx context.Context) ([]models.Userable, error) {
} }
if err := iter.Iterate(ctx, iterator); err != nil { if err := iter.Iterate(ctx, iterator); err != nil {
return nil, support.ConnectorStackErrorTraceWrap(err, "iterating all users") return nil, clues.Wrap(err, "iterating all users").WithClues(ctx).WithAll(graph.ErrData(err)...)
} }
return us, iterErrs return us, errs.Err()
} }
func (c Users) GetByID(ctx context.Context, userID string) (models.Userable, error) { func (c Users) GetByID(ctx context.Context, userID string) (models.Userable, error) {
@ -145,7 +147,7 @@ func (c Users) GetByID(ctx context.Context, userID string) (models.Userable, err
}) })
if err != nil { if err != nil {
return nil, support.ConnectorStackErrorTraceWrap(err, "getting user by id") return nil, clues.Wrap(err, "getting user").WithClues(ctx).WithAll(graph.ErrData(err)...)
} }
return resp, err return resp, err
@ -167,7 +169,7 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
if err != nil { if err != nil {
if !graph.IsErrExchangeMailFolderNotFound(err) { if !graph.IsErrExchangeMailFolderNotFound(err) {
return nil, support.ConnectorStackErrorTraceWrap(err, "getting user's exchange mailfolders") return nil, clues.Wrap(err, "getting user's mail folder").WithClues(ctx).WithAll(graph.ErrData(err)...)
} }
delete(userInfo.DiscoveredServices, path.ExchangeService) delete(userInfo.DiscoveredServices, path.ExchangeService)
@ -186,15 +188,15 @@ func (c Users) GetInfo(ctx context.Context, userID string) (*UserInfo, error) {
func validateUser(item any) (models.Userable, error) { func validateUser(item any) (models.Userable, error) {
m, ok := item.(models.Userable) m, ok := item.(models.Userable)
if !ok { if !ok {
return nil, errors.Errorf("expected Userable, got %T", item) return nil, clues.Stack(clues.New("unexpected model"), errors.Errorf("%T", item))
} }
if m.GetId() == nil { if m.GetId() == nil {
return nil, errors.Errorf("missing ID") return nil, clues.New("missing ID")
} }
if m.GetUserPrincipalName() == nil { if m.GetUserPrincipalName() == nil {
return nil, errors.New("missing principalName") return nil, clues.New("missing principalName")
} }
return m, nil return m, nil

View File

@ -7,6 +7,7 @@ import (
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/connector/discovery/api" "github.com/alcionai/corso/src/internal/connector/discovery/api"
"github.com/alcionai/corso/src/pkg/fault"
) )
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
@ -14,7 +15,7 @@ import (
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
type getAller interface { type getAller interface {
GetAll(context.Context) ([]models.Userable, error) GetAll(context.Context, *fault.Errors) ([]models.Userable, error)
} }
type getter interface { type getter interface {
@ -35,8 +36,8 @@ type getWithInfoer interface {
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
// Users fetches all users in the tenant. // Users fetches all users in the tenant.
func Users(ctx context.Context, ga getAller) ([]models.Userable, error) { func Users(ctx context.Context, ga getAller, errs *fault.Errors) ([]models.Userable, error) {
return ga.GetAll(ctx) return ga.GetAll(ctx, errs)
} }
func User(ctx context.Context, gwi getWithInfoer, userID string) (models.Userable, *api.UserInfo, error) { func User(ctx context.Context, gwi getWithInfoer, userID string) (models.Userable, *api.UserInfo, error) {

View File

@ -1,6 +1,7 @@
package graph package graph
import ( import (
"github.com/alcionai/clues"
"github.com/microsoftgraph/msgraph-sdk-go/models" "github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/pkg/errors" "github.com/pkg/errors"
@ -26,12 +27,12 @@ func CheckRequiredValues(c Container) error {
ptr := c.GetDisplayName() ptr := c.GetDisplayName()
if ptr == nil || len(*ptr) == 0 { if ptr == nil || len(*ptr) == 0 {
return errors.Errorf("folder %s without display name", *idPtr) return clues.New("folder missing display name").With("container_id", *idPtr)
} }
ptr = c.GetParentFolderId() ptr = c.GetParentFolderId()
if ptr == nil || len(*ptr) == 0 { if ptr == nil || len(*ptr) == 0 {
return errors.Errorf("folder %s without parent ID", *idPtr) return clues.New("folder missing parent ID").With("container_parent_id", *idPtr)
} }
return nil return nil

View File

@ -98,7 +98,7 @@ func NewGraphConnector(
// For now this keeps things functioning if callers do pass in a selector like // For now this keeps things functioning if callers do pass in a selector like
// "*" instead of. // "*" instead of.
if r == AllResources || r == Users { if r == AllResources || r == Users {
if err = gc.setTenantUsers(ctx); err != nil { if err = gc.setTenantUsers(ctx, errs); err != nil {
return nil, errors.Wrap(err, "retrieving tenant user list") return nil, errors.Wrap(err, "retrieving tenant user list")
} }
} }
@ -129,11 +129,11 @@ func (gc *GraphConnector) createService() (*graph.Service, error) {
// setTenantUsers queries the M365 to identify the users in the // setTenantUsers queries the M365 to identify the users in the
// workspace. The users field is updated during this method // workspace. The users field is updated during this method
// iff the returned error is nil // iff the returned error is nil
func (gc *GraphConnector) setTenantUsers(ctx context.Context) error { func (gc *GraphConnector) setTenantUsers(ctx context.Context, errs *fault.Errors) error {
ctx, end := D.Span(ctx, "gc:setTenantUsers") ctx, end := D.Span(ctx, "gc:setTenantUsers")
defer end() defer end()
users, err := discovery.Users(ctx, gc.Owners.Users()) users, err := discovery.Users(ctx, gc.Owners.Users(), errs)
if err != nil { if err != nil {
return err return err
} }

View File

@ -129,12 +129,8 @@ func (suite *GraphConnectorUnitSuite) TestUnionSiteIDsAndWebURLs() {
ctx, flush := tester.NewContext() ctx, flush := tester.NewContext()
defer flush() defer flush()
errs := fault.New(true) result, err := gc.UnionSiteIDsAndWebURLs(ctx, test.ids, test.urls, fault.New(true))
result, err := gc.UnionSiteIDsAndWebURLs(ctx, test.ids, test.urls, errs)
assert.NoError(t, err) assert.NoError(t, err)
assert.NoError(t, errs.Err())
assert.Empty(t, errs.Errs())
assert.ElementsMatch(t, test.expect, result) assert.ElementsMatch(t, test.expect, result)
}) })
} }
@ -192,9 +188,11 @@ func (suite *GraphConnectorIntegrationSuite) TestSetTenantUsers() {
require.NoError(suite.T(), err) require.NoError(suite.T(), err)
newConnector.Owners = owners newConnector.Owners = owners
suite.Empty(len(newConnector.Users)) suite.Empty(len(newConnector.Users))
err = newConnector.setTenantUsers(ctx)
errs := fault.New(true)
err = newConnector.setTenantUsers(ctx, errs)
suite.NoError(err) suite.NoError(err)
suite.Less(0, len(newConnector.Users)) suite.Less(0, len(newConnector.Users))
} }
@ -219,12 +217,8 @@ func (suite *GraphConnectorIntegrationSuite) TestSetTenantSites() {
newConnector.Service = service newConnector.Service = service
assert.Equal(t, 0, len(newConnector.Sites)) assert.Equal(t, 0, len(newConnector.Sites))
errs := fault.New(true) err = newConnector.setTenantSites(ctx, fault.New(true))
err = newConnector.setTenantSites(ctx, errs)
assert.NoError(t, err) assert.NoError(t, err)
assert.NoError(t, errs.Err())
assert.Empty(t, errs.Errs())
assert.Less(t, 0, len(newConnector.Sites)) assert.Less(t, 0, len(newConnector.Sites))
for _, site := range newConnector.Sites { for _, site := range newConnector.Sites {

View File

@ -6,8 +6,6 @@ import (
"github.com/dustin/go-humanize" "github.com/dustin/go-humanize"
multierror "github.com/hashicorp/go-multierror" multierror "github.com/hashicorp/go-multierror"
"github.com/alcionai/corso/src/pkg/logger"
) )
// ConnectorOperationStatus is a data type used to describe the state of // ConnectorOperationStatus is a data type used to describe the state of
@ -80,15 +78,6 @@ func CreateStatus(
additionalDetails: details, additionalDetails: details,
} }
if status.ObjectCount != status.ErrorCount+status.Successful {
logger.Ctx(ctx).Errorw(
"status object count does not match errors + successes",
"objects", cm.Objects,
"successes", cm.Successes,
"numErrors", numErr,
"errors", err)
}
return &status return &status
} }
@ -114,10 +103,11 @@ func MergeStatus(one, two ConnectorOperationStatus) ConnectorOperationStatus {
} }
status := ConnectorOperationStatus{ status := ConnectorOperationStatus{
lastOperation: one.lastOperation, lastOperation: one.lastOperation,
ObjectCount: one.ObjectCount + two.ObjectCount, ObjectCount: one.ObjectCount + two.ObjectCount,
FolderCount: one.FolderCount + two.FolderCount, FolderCount: one.FolderCount + two.FolderCount,
Successful: one.Successful + two.Successful, Successful: one.Successful + two.Successful,
// TODO: remove in favor of fault.Errors
ErrorCount: one.ErrorCount + two.ErrorCount, ErrorCount: one.ErrorCount + two.ErrorCount,
Err: multierror.Append(one.Err, two.Err).ErrorOrNil(), Err: multierror.Append(one.Err, two.Err).ErrorOrNil(),
bytes: one.bytes + two.bytes, bytes: one.bytes + two.bytes,
@ -144,14 +134,11 @@ func (cos *ConnectorOperationStatus) String() string {
cos.Successful, cos.Successful,
cos.ObjectCount, cos.ObjectCount,
humanize.Bytes(uint64(cos.bytes)), humanize.Bytes(uint64(cos.bytes)),
cos.FolderCount, cos.FolderCount)
)
if cos.incomplete { if cos.incomplete {
message += " " + cos.incompleteReason message += " " + cos.incompleteReason
} }
message += " " + operationStatement + cos.additionalDetails + "\n" return message + " " + operationStatement + cos.additionalDetails
return message
} }

View File

@ -5,7 +5,7 @@ import (
"context" "context"
"fmt" "fmt"
"github.com/pkg/errors" "github.com/alcionai/clues"
"gopkg.in/resty.v1" "gopkg.in/resty.v1"
"github.com/alcionai/corso/src/pkg/logger" "github.com/alcionai/corso/src/pkg/logger"
@ -38,7 +38,7 @@ func NewWriter(id, url string, size int64) *writer {
// Write will upload the provided data to M365. It sets the `Content-Length` and `Content-Range` headers based on // Write will upload the provided data to M365. It sets the `Content-Length` and `Content-Range` headers based on
// https://docs.microsoft.com/en-us/graph/api/driveitem-createuploadsession // https://docs.microsoft.com/en-us/graph/api/driveitem-createuploadsession
func (iw *writer) Write(p []byte) (n int, err error) { func (iw *writer) Write(p []byte) (int, error) {
rangeLength := len(p) rangeLength := len(p)
logger.Ctx(context.Background()).Debugf("WRITE for %s. Size:%d, Offset: %d, TotalSize: %d", logger.Ctx(context.Background()).Debugf("WRITE for %s. Size:%d, Offset: %d, TotalSize: %d",
iw.id, rangeLength, iw.lastWrittenOffset, iw.contentLength) iw.id, rangeLength, iw.lastWrittenOffset, iw.contentLength)
@ -47,7 +47,7 @@ func (iw *writer) Write(p []byte) (n int, err error) {
// PUT the request - set headers `Content-Range`to describe total size and `Content-Length` to describe size of // PUT the request - set headers `Content-Range`to describe total size and `Content-Length` to describe size of
// data in the current request // data in the current request
resp, err := iw.client.R(). _, err := iw.client.R().
SetHeaders(map[string]string{ SetHeaders(map[string]string{
contentRangeHeaderKey: fmt.Sprintf(contentRangeHeaderValueFmt, contentRangeHeaderKey: fmt.Sprintf(contentRangeHeaderValueFmt,
iw.lastWrittenOffset, iw.lastWrittenOffset,
@ -57,15 +57,15 @@ func (iw *writer) Write(p []byte) (n int, err error) {
}). }).
SetBody(bytes.NewReader(p)).Put(iw.url) SetBody(bytes.NewReader(p)).Put(iw.url)
if err != nil { if err != nil {
return 0, errors.Wrapf(err, return 0, clues.Wrap(err, "uploading item").WithAll(
"failed to upload item %s. Upload failed at Size:%d, Offset: %d, TotalSize: %d ", "upload_id", iw.id,
iw.id, rangeLength, iw.lastWrittenOffset, iw.contentLength) "upload_chunk_size", rangeLength,
"upload_offset", iw.lastWrittenOffset,
"upload_size", iw.contentLength)
} }
// Update last offset // Update last offset
iw.lastWrittenOffset = endOffset iw.lastWrittenOffset = endOffset
logger.Ctx(context.Background()).Debugf("Response: %s", resp.String())
return rangeLength, nil return rangeLength, nil
} }

View File

@ -284,7 +284,7 @@ func (suite *SelectorScopesSuite) TestReduce() {
dataCats, dataCats,
errs) errs)
require.NotNil(t, result) require.NotNil(t, result)
require.Empty(t, errs.Errs(), "recoverable errors") require.NoError(t, errs.Err(), "no recoverable errors")
assert.Len(t, result.Entries, test.expectLen) assert.Len(t, result.Entries, test.expectLen)
}) })
} }

View File

@ -28,7 +28,7 @@ func Users(ctx context.Context, acct account.Account, errs *fault.Errors) ([]*Us
return nil, errors.Wrap(err, "initializing M365 graph connection") return nil, errors.Wrap(err, "initializing M365 graph connection")
} }
users, err := discovery.Users(ctx, gc.Owners.Users()) users, err := discovery.Users(ctx, gc.Owners.Users(), errs)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -31,13 +31,10 @@ func (suite *M365IntegrationSuite) TestUsers() {
var ( var (
t = suite.T() t = suite.T()
acct = tester.NewM365Account(suite.T()) acct = tester.NewM365Account(suite.T())
errs = fault.New(true)
) )
users, err := Users(ctx, acct, errs) users, err := Users(ctx, acct, fault.New(true))
require.NoError(t, err) require.NoError(t, err)
require.NoError(t, errs.Err())
require.Empty(t, errs.Errs())
require.NotNil(t, users) require.NotNil(t, users)
require.Greater(t, len(users), 0) require.Greater(t, len(users), 0)