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

centralize core err interpretation (#4972)

Browse Source 
now that we have a core error library, we can start swapping out the percolation of graph api errors with core error sentinels.  This PR begins that process by adding a centralized transformer to the graph service handlers that ensures all calls produce the same set of core errors according to some standard categorizations. This is just an initial step, and does not transfer all graph errors into core sentinels.

---

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

- [x] ⛔ No

#### Type of change

- [x] 🤖 Supportability/Tests

#### Issue(s)

* #4685 

#### Test Plan

- [x] ⚡ Unit test
- [x] 💚 E2E
This commit is contained in:
Keepers 2024-01-10 10:25:25 -07:00 committed by GitHub
parent fad3dfe769
commit b1af13b7b6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 158 additions and 190 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/cli/backup/helpers_test.go
View File

@ -25,7 +25,6 @@ import (
"github.com/alcionai/corso/src/pkg/repository"
"github.com/alcionai/corso/src/pkg/services/m365/api"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
gmock "github.com/alcionai/corso/src/pkg/services/m365/api/graph/mock"
"github.com/alcionai/corso/src/pkg/storage"
"github.com/alcionai/corso/src/pkg/storage/testdata"
)
@ -37,12 +36,12 @@ import (
// GockClient produces a new exchange api client that can be
// mocked using gock.
func gockClient(creds account.M365Config, counter *count.Bus) (api.Client, error) {
s, err := gmock.NewService(creds, counter)
s, err := graph.NewGockService(creds, counter)
if err != nil {
return api.Client{}, err
}
li, err := gmock.NewService(creds, counter, graph.NoTimeout())
li, err := graph.NewGockService(creds, counter, graph.NoTimeout())
if err != nil {
return api.Client{}, err
}

10
src/internal/m365/controller.go
View File

@ -255,15 +255,7 @@ func (r resourceGetter) GetResourceIDAndNameFrom(
id, name, err = r.getter.GetIDAndName(ctx, owner, api.CallConfig{})
if err != nil {
if graph.IsErrUserNotFound(err) {
return nil, clues.Stack(core.ErrResourceOwnerNotFound, err)
}
if graph.IsErrResourceLocked(err) {
return nil, clues.Stack(graph.ErrResourceLocked, err)
}
return nil, err
return nil, clues.Stack(err)
}
if len(id) == 0 || len(name) == 0 {

4
src/internal/m365/service/exchange/enabled_test.go
View File

@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/suite"
"github.com/alcionai/corso/src/internal/tester"
"github.com/alcionai/corso/src/pkg/errs/core"
"github.com/alcionai/corso/src/pkg/services/m365/api"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
graphTD "github.com/alcionai/corso/src/pkg/services/m365/api/graph/testdata"
@ -91,9 +92,10 @@ func (suite *EnabledUnitSuite) TestIsServiceEnabled() {
name: "overlapping resourcenotfound",
mock: func(ctx context.Context) getMailInboxer {
odErr := graphTD.ODataErrWithMsg(string(graph.ResourceNotFound), "User not found")
err := clues.Stack(core.ErrResourceOwnerNotFound, odErr)
return mockGMB{
mailboxErr: graph.Stack(ctx, odErr),
mailboxErr: graph.Stack(ctx, err),
}
},
expect: assert.False,

9
src/internal/m365/service/onedrive/enabled.go
View File

@ -6,7 +6,6 @@ import (
"github.com/alcionai/clues"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/alcionai/corso/src/pkg/errs/core"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
)
@ -27,14 +26,6 @@ func IsServiceEnabled(
return false, nil
}
if graph.IsErrUserNotFound(err) {
return false, clues.Stack(core.ErrResourceOwnerNotFound, err)
}
if graph.IsErrResourceLocked(err) {
return false, clues.Stack(graph.ErrResourceLocked, err)
}
return false, clues.Stack(err)
}

5
src/internal/operations/test/m365/helper.go
View File

@ -34,7 +34,6 @@ import (
"github.com/alcionai/corso/src/pkg/selectors"
"github.com/alcionai/corso/src/pkg/services/m365/api"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
gmock "github.com/alcionai/corso/src/pkg/services/m365/api/graph/mock"
)
// ---------------------------------------------------------------------------
@ -44,12 +43,12 @@ import (
// GockClient produces a new exchange api client that can be
// mocked using gock.
func GockClient(creds account.M365Config, counter *count.Bus) (api.Client, error) {
s, err := gmock.NewService(creds, counter)
s, err := graph.NewGockService(creds, counter)
if err != nil {
return api.Client{}, err
}
li, err := gmock.NewService(creds, counter, graph.NoTimeout())
li, err := graph.NewGockService(creds, counter, graph.NoTimeout())
if err != nil {
return api.Client{}, err
}

3
src/pkg/errs/errs.go
View File

@ -24,10 +24,7 @@ type ErrCheck func(error) bool
// checks. This allows us to apply those comparison checks instead of relying
// only on sentinels.
var externalToInternalCheck = map[*core.Err][]ErrCheck{
core.ErrApplicationThrottled: {graph.IsErrApplicationThrottled},
core.ErrResourceNotAccessible: {graph.IsErrResourceLocked},
core.ErrResourceOwnerNotFound: {graph.IsErrItemNotFound},
core.ErrInsufficientAuthorization: {graph.IsErrInsufficientAuthorization},
}
// Internal returns the internal errors and error checking functions which

22
src/pkg/errs/errs_test.go
View File

@ -56,12 +56,6 @@ func (suite *ErrUnitSuite) TestInternal_checks() {
expectHasChecks assert.ValueAssertionFunc
expect assert.BoolAssertionFunc
}{
{
get: core.ErrApplicationThrottled,
err: graphTD.ODataErr(string(graph.ApplicationThrottled)),
expectHasChecks: assert.NotEmpty,
expect: assert.True,
},
{
get: core.ErrRepoAlreadyExists,
err: graphTD.ODataErr(string(graph.ApplicationThrottled)),
@ -86,18 +80,6 @@ func (suite *ErrUnitSuite) TestInternal_checks() {
expectHasChecks: assert.NotEmpty,
expect: assert.True,
},
{
get: core.ErrResourceNotAccessible,
err: graph.ErrResourceLocked,
expectHasChecks: assert.NotEmpty,
expect: assert.True,
},
{
get: core.ErrInsufficientAuthorization,
err: graphTD.ODataErr(string(graph.AuthorizationRequestDenied)),
expectHasChecks: assert.NotEmpty,
expect: assert.True,
},
}
for _, test := range table {
suite.Run(test.get.Error(), func() {
@ -138,10 +120,6 @@ func (suite *ErrUnitSuite) TestIs() {
target: core.ErrResourceNotAccessible,
err: graph.ErrResourceLocked,
},
{
target: core.ErrInsufficientAuthorization,
err: graphTD.ODataErr(string(graph.AuthorizationRequestDenied)),
},
}
for _, test := range table {
suite.Run(test.target.Error(), func() {

42
src/pkg/services/m365/api/graph/errors.go
View File

@ -18,6 +18,7 @@ import (
"github.com/alcionai/corso/src/internal/common/jwt"
"github.com/alcionai/corso/src/internal/common/ptr"
"github.com/alcionai/corso/src/internal/common/str"
"github.com/alcionai/corso/src/pkg/errs/core"
"github.com/alcionai/corso/src/pkg/fault"
"github.com/alcionai/corso/src/pkg/filters"
"github.com/alcionai/corso/src/pkg/services/m365/custom"
@ -105,6 +106,10 @@ const (
LabelsSkippable = "skippable_errors"
)
// ---------------------------------------------------------------------------
// error sentinels & categorization
// ---------------------------------------------------------------------------
// These errors are graph specific. That means they don't have a clear parallel in
// pkg/errs/core. If these errors need to trickle outward to non-m365 layers, we
// need to find a sufficiently coarse errs/core sentinel to use as transformation.
@ -134,7 +139,26 @@ var (
ErrTokenExpired = clues.New("jwt token expired")
)
func IsErrApplicationThrottled(err error) bool {
func stackWithCoreErr(ctx context.Context, err error, traceDepth int) error {
if err == nil {
return nil
}
switch {
case isErrApplicationThrottled(err):
err = clues.Stack(core.ErrApplicationThrottled, err)
case isErrResourceLocked(err):
err = clues.Stack(core.ErrResourceNotAccessible, err)
case isErrUserNotFound(err):
err = clues.Stack(core.ErrResourceOwnerNotFound, err)
case isErrInsufficientAuthorization(err):
err = clues.Stack(core.ErrInsufficientAuthorization, err)
}
return stackWithDepth(ctx, err, 1+traceDepth)
}
func isErrApplicationThrottled(err error) bool {
return parseODataErr(err).hasErrorCode(err, ApplicationThrottled)
}
@ -142,7 +166,7 @@ func IsErrAuthenticationError(err error) bool {
return parseODataErr(err).hasErrorCode(err, AuthenticationError)
}
func IsErrInsufficientAuthorization(err error) bool {
func isErrInsufficientAuthorization(err error) bool {
return parseODataErr(err).hasErrorCode(err, AuthorizationRequestDenied)
}
@ -189,7 +213,7 @@ func IsErrExchangeMailFolderNotFound(err error) bool {
return parseODataErr(err).hasErrorCode(err, ResourceNotFound, ErrorItemNotFound, MailboxNotEnabledForRESTAPI)
}
func IsErrUserNotFound(err error) bool {
func isErrUserNotFound(err error) bool {
ode := parseODataErr(err)
if ode.hasErrorCode(err, RequestResourceNotFound, invalidUser) {
@ -281,7 +305,7 @@ func IsErrSiteNotFound(err error) bool {
return parseODataErr(err).hasErrorMessage(err, requestedSiteCouldNotBeFound)
}
func IsErrResourceLocked(err error) bool {
func isErrResourceLocked(err error) bool {
ode := parseODataErr(err)
return errors.Is(err, ErrResourceLocked) ||
@ -298,6 +322,10 @@ func IsErrSharingDisabled(err error) bool {
return parseODataErr(err).hasInnerErrorCode(err, sharingDisabled)
}
// ---------------------------------------------------------------------------
// quality of life wrappers
// ---------------------------------------------------------------------------
// Wrap is a helper function that extracts ODataError metadata from
// the error. If the error is not an ODataError type, returns the error.
func Wrap(ctx context.Context, e error, msg string) *clues.Err {
@ -324,6 +352,10 @@ func Wrap(ctx context.Context, e error, msg string) *clues.Err {
// Stack is a helper function that extracts ODataError metadata from
// the error. If the error is not an ODataError type, returns the error.
func Stack(ctx context.Context, e error) *clues.Err {
return stackWithDepth(ctx, e, 1)
}
func stackWithDepth(ctx context.Context, e error, traceDepth int) *clues.Err {
if e == nil {
return nil
}
@ -339,7 +371,7 @@ func Stack(ctx context.Context, e error) *clues.Err {
ce := clues.StackWC(ctx, e).
With("graph_api_err", ode).
WithTrace(1)
WithTrace(1 + traceDepth)
return setLabels(ce, ode)
}

8
src/pkg/services/m365/api/graph/errors_test.go
View File

@ -85,7 +85,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrApplicationThrottled() {
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(suite.T(), IsErrApplicationThrottled(test.err))
test.expect(suite.T(), isErrApplicationThrottled(test.err))
})
}
}
@ -153,7 +153,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrInsufficientAuthorization() {
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(suite.T(), IsErrInsufficientAuthorization(test.err))
test.expect(suite.T(), isErrInsufficientAuthorization(test.err))
})
}
}
@ -481,7 +481,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrUserNotFound() {
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(suite.T(), IsErrUserNotFound(test.err))
test.expect(suite.T(), isErrUserNotFound(test.err))
})
}
}
@ -968,7 +968,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrResourceLocked() {
}
for _, test := range table {
suite.Run(test.name, func() {
test.expect(suite.T(), IsErrResourceLocked(test.err))
test.expect(suite.T(), isErrResourceLocked(test.err))
})
}
}

38
src/pkg/services/m365/api/graph/http_wrapper.go
View File

@ -14,7 +14,6 @@ import (
"github.com/alcionai/corso/src/internal/events"
"github.com/alcionai/corso/src/internal/version"
"github.com/alcionai/corso/src/pkg/count"
"github.com/alcionai/corso/src/pkg/errs/core"
"github.com/alcionai/corso/src/pkg/logger"
)
@ -103,7 +102,9 @@ func (hw httpWrapper) Request(
// See https://learn.microsoft.com/en-us/sharepoint/dev/general-development/how-to-avoid-getting-throttled-or-blocked-in-sharepoint-online#how-to-decorate-your-http-traffic
req.Header.Set("User-Agent", "ISV|Alcion|Corso/"+version.Version)
var resp *http.Response
retriedErrors := []string{}
var e error
// stream errors from http/2 will fail before we reach
// client middleware handling, therefore we don't get to
@ -112,36 +113,41 @@ func (hw httpWrapper) Request(
// retry in the event of a `stream error`, which is not
// a common expectation.
for i := 0; i < hw.config.maxConnectionRetries+1; i++ {
if i > 0 {
time.Sleep(3 * time.Second)
}
ctx = clues.Add(ctx, "request_retry_iter", i)
resp, err = hw.client.Do(req)
resp, err := hw.client.Do(req)
if err == nil {
break
logResp(ctx, resp)
return resp, nil
}
if IsErrApplicationThrottled(err) {
return nil, Stack(ctx, clues.Stack(core.ErrApplicationThrottled, err))
}
err = stackWithCoreErr(ctx, err, 1)
e = err
var http2StreamErr http2.StreamError
if !errors.As(err, &http2StreamErr) {
return nil, Stack(ctx, err)
// exit most errors without retry
break
}
logger.Ctx(ctx).Debug("http2 stream error")
events.Inc(events.APICall, "streamerror")
time.Sleep(3 * time.Second)
retriedErrors = append(retriedErrors, err.Error())
}
if err != nil {
return nil, Stack(ctx, err)
}
e = clues.Stack(e).
With("retried_errors", retriedErrors).
WithTrace(1).
OrNil()
logResp(ctx, resp)
return resp, nil
// no chance of a non-error return here.
// we handle that inside the loop.
return nil, e
}
// ---------------------------------------------------------------------------

1
src/pkg/services/m365/api/graph/http_wrapper_test.go
View File

@ -182,7 +182,6 @@ func (suite *HTTPWrapperUnitSuite) TestNewHTTPWrapper_http2StreamErrorRetries()
_, err := hw.Request(ctx, http.MethodGet, url, nil, nil)
require.ErrorAs(t, err, &http2.StreamError{}, clues.ToCore(err))
require.Equal(t, test.expectRetries, tries, "count of retries")
})
}

49
src/pkg/services/m365/api/graph/service.go
View File

@ -18,7 +18,6 @@ import (
"github.com/alcionai/corso/src/internal/common/idname"
"github.com/alcionai/corso/src/internal/events"
"github.com/alcionai/corso/src/pkg/count"
"github.com/alcionai/corso/src/pkg/errs/core"
"github.com/alcionai/corso/src/pkg/filters"
"github.com/alcionai/corso/src/pkg/logger"
"github.com/alcionai/corso/src/pkg/path"
@ -381,13 +380,15 @@ func (aw *adapterWrap) Send(
requestInfo *abstractions.RequestInformation,
constructor serialization.ParsableFactory,
errorMappings abstractions.ErrorMappings,
) (sp serialization.Parsable, err error) {
) (sp serialization.Parsable, e error) {
defer func() {
if crErr := crash.Recovery(ctx, recover(), "graph adapter request"); crErr != nil {
err = Stack(ctx, crErr)
e = Stack(ctx, crErr)
}
}()
retriedErrors := []string{}
// This external retry wrapper is unsophisticated, but should
// only retry under certain circumstances
// 1. stream errors from http/2, which will fail before we reach
@ -395,41 +396,47 @@ func (aw *adapterWrap) Send(
// 2. jwt token invalidation, which requires a re-auth that's handled
// in the Send() call, before reaching client middleware.
for i := 0; i < aw.config.maxConnectionRetries+1; i++ {
if i > 0 {
time.Sleep(aw.retryDelay)
}
ictx := clues.Add(ctx, "request_retry_iter", i)
sp, err = aw.RequestAdapter.Send(ictx, requestInfo, constructor, errorMappings)
resp, err := aw.RequestAdapter.Send(ictx, requestInfo, constructor, errorMappings)
if err == nil {
break
return resp, nil
}
// force an early exit on throttling issues.
// those retries are well handled in middleware already. We want to ensure
// that the error gets wrapped with the appropriate sentinel here.
if IsErrApplicationThrottled(err) {
return nil, clues.StackWC(ictx, core.ErrApplicationThrottled, err).WithTrace(1)
}
err = stackWithCoreErr(ictx, err, 1)
e = err
// exit most errors without retry
switch {
case IsErrConnectionReset(err) || connectionEnded.Compare(err.Error()):
if IsErrConnectionReset(err) || connectionEnded.Compare(err.Error()) {
logger.Ctx(ictx).Debug("http connection error")
events.Inc(events.APICall, "connectionerror")
case IsErrBadJWTToken(err):
} else if IsErrBadJWTToken(err) {
logger.Ctx(ictx).Debug("bad jwt token")
events.Inc(events.APICall, "badjwttoken")
case requestInfo.Method.String() == http.MethodGet && IsErrInvalidRequest(err):
} else if requestInfo.Method.String() == http.MethodGet && IsErrInvalidRequest(err) {
// Graph may sometimes return a transient 400 response during onedrive
// and sharepoint backup. This is pending investigation on msft end, retry
// for now as it's a transient issue. Restrict retries to GET requests only
// to limit the scope of this fix.
logger.Ctx(ictx).Debug("invalid request")
events.Inc(events.APICall, "invalidrequest")
default:
return nil, clues.StackWC(ictx, err).WithTrace(1)
events.Inc(events.APICall, "invalidgetrequest")
} else {
// exit most errors without retry
break
}
time.Sleep(aw.retryDelay)
retriedErrors = append(retriedErrors, err.Error())
}
return sp, clues.Stack(err).OrNil()
e = clues.Stack(e).
With("retried_errors", retriedErrors).
WithTrace(1).
OrNil()
// no chance of a non-error return here.
// we handle that inside the loop.
return nil, e
}

30
src/pkg/services/m365/api/graph/mock/service.go → src/pkg/services/m365/api/graph/service_mock.go
View File

@ -1,21 +1,21 @@
package mock
package graph
import (
"github.com/alcionai/clues"
"github.com/h2non/gock"
abstractions "github.com/microsoft/kiota-abstractions-go"
msgraphsdkgo "github.com/microsoftgraph/msgraph-sdk-go"
"github.com/alcionai/corso/src/pkg/account"
"github.com/alcionai/corso/src/pkg/count"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
)
func NewService(
func NewGockService(
creds account.M365Config,
counter *count.Bus,
opts ...graph.Option,
) (*graph.Service, error) {
a, err := CreateAdapter(
opts ...Option,
) (*Service, error) {
a, err := CreateGockAdapter(
creds.AzureTenantID,
creds.AzureClientID,
creds.AzureClientSecret,
@ -25,29 +25,31 @@ func NewService(
return nil, clues.Wrap(err, "generating graph adapter")
}
return graph.NewService(a), nil
return NewService(a), nil
}
// CreateAdapter is similar to graph.CreateAdapter, but with option to
// CreateGockAdapter is similar to graph.CreateAdapter, but with option to
// enable interceptions via gock to make it mockable.
func CreateAdapter(
func CreateGockAdapter(
tenant, client, secret string,
counter *count.Bus,
opts ...graph.Option,
) (*msgraphsdkgo.GraphRequestAdapter, error) {
auth, err := graph.GetAuth(tenant, client, secret)
opts ...Option,
) (abstractions.RequestAdapter, error) {
auth, err := GetAuth(tenant, client, secret)
if err != nil {
return nil, err
}
httpClient, _ := graph.KiotaHTTPClient(counter, opts...)
httpClient, cc := KiotaHTTPClient(counter, opts...)
// This makes sure that we are able to intercept any requests via
// gock. Only necessary for testing.
gock.InterceptClient(httpClient)
return msgraphsdkgo.NewGraphRequestAdapterWithParseNodeFactoryAndSerializationWriterFactoryAndHttpClient(
ng, err := msgraphsdkgo.NewGraphRequestAdapterWithParseNodeFactoryAndSerializationWriterFactoryAndHttpClient(
auth,
nil, nil,
httpClient)
return wrapAdapter(ng, cc), clues.Stack(err).OrNil()
}

17
src/pkg/services/m365/api/groups.go
View File

@ -11,6 +11,7 @@ import (
msgraphgocore "github.com/microsoftgraph/msgraph-sdk-go-core"
"github.com/microsoftgraph/msgraph-sdk-go/groups"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/pkg/errors"
"github.com/alcionai/corso/src/internal/common/idname"
"github.com/alcionai/corso/src/internal/common/ptr"
@ -133,10 +134,6 @@ func (c Groups) GetTeamByID(
t, err := c.Stable.Client().Teams().ByTeamId(identifier).Get(ctx, nil)
if err != nil {
if graph.IsErrResourceLocked(err) {
return nil, graph.Stack(ctx, clues.Stack(graph.ErrResourceLocked, err))
}
return nil, graph.Wrap(ctx, err, "finding team by ID")
}
@ -172,8 +169,8 @@ func (c Groups) GetByID(
return group, nil
}
if graph.IsErrResourceLocked(err) {
return nil, graph.Stack(ctx, clues.Stack(graph.ErrResourceLocked, err))
if errors.Is(err, core.ErrResourceNotAccessible) {
return nil, err
}
logger.CtxErr(ctx, err).Info("finding group by id, falling back to secondary identifier")
@ -194,8 +191,8 @@ func (c Groups) GetByID(
return getGroupFromResponse(ctx, resp)
}
if graph.IsErrResourceLocked(err) {
err = clues.Stack(graph.ErrResourceLocked, err)
if errors.Is(err, core.ErrResourceNotAccessible) {
return nil, err
}
logger.CtxErr(ctx, err).Info("finding group by email, falling back to display name")
@ -211,10 +208,6 @@ func (c Groups) GetByID(
resp, err := c.Stable.Client().Groups().Get(ctx, opts)
if err != nil {
if graph.IsErrResourceLocked(err) {
err = clues.Stack(graph.ErrResourceLocked, err)
}
return nil, graph.Wrap(ctx, err, "finding group by display name")
}

58
src/pkg/services/m365/api/groups_test.go
View File

@ -7,6 +7,7 @@ import (
"github.com/google/uuid"
"github.com/h2non/gock"
"github.com/microsoftgraph/msgraph-sdk-go/models"
"github.com/microsoftgraph/msgraph-sdk-go/models/odataerrors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
@ -263,55 +264,28 @@ func (suite *GroupsIntgSuite) TestGroups_GetByID_mockResourceLockedErrs() {
table := []struct {
name string
id string
setup func(t *testing.T)
err *odataerrors.ODataError
}{
{
name: "by name",
id: "g",
setup: func(t *testing.T) {
err := graphTD.ODataErr(string(graph.NotAllowed))
interceptV1Path("groups").
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
interceptV1Path("teams").
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
},
err: graphTD.ODataErr(string(graph.NotAllowed)),
},
{
name: "by id",
id: gID,
setup: func(t *testing.T) {
err := graphTD.ODataErr(string(graph.NotAllowed))
interceptV1Path("groups", gID).
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
interceptV1Path("teams", gID).
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
},
err: graphTD.ODataErr(string(graph.NotAllowed)),
},
{
name: "by id - matches error message",
id: gID,
setup: func(t *testing.T) {
err := graphTD.ODataErrWithMsg(
err: graphTD.ODataErrWithMsg(
string(graph.AuthenticationError),
"AADSTS500014: The service principal for resource 'beefe6b7-f5df-413d-ac2d-abf1e3fd9c0b' "+
"is disabled. This indicate that a subscription within the tenant has lapsed, or that the "+
"administrator for this tenant has disabled the application, preventing tokens from being "+
"issued for it. Trace ID: dead78e1-0830-4edf-bea7-f0a445620100 Correlation ID: "+
"deadbeef-7f1e-4578-8215-36004a2c935c Timestamp: 2023-12-05 19:31:01Z")
interceptV1Path("groups", gID).
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
interceptV1Path("teams", gID).
Reply(403).
JSON(graphTD.ParseableToMap(t, err))
},
"deadbeef-7f1e-4578-8215-36004a2c935c Timestamp: 2023-12-05 19:31:01Z"),
},
}
for _, test := range table {
@ -323,18 +297,32 @@ func (suite *GroupsIntgSuite) TestGroups_GetByID_mockResourceLockedErrs() {
defer flush()
defer gock.Off()
test.setup(t)
interceptV1Path("groups", gID).
Reply(403).
JSON(graphTD.ParseableToMap(t, test.err))
interceptV1Path("groups").
Reply(403).
JSON(graphTD.ParseableToMap(t, test.err))
// Verify both GetByID and GetTeamByID APIs handle this error
_, err := suite.its.gockAC.
Groups().
GetByID(ctx, test.id, CallConfig{})
require.ErrorIs(t, err, graph.ErrResourceLocked, clues.ToCore(err))
require.ErrorIs(t, err, core.ErrResourceNotAccessible, clues.ToCore(err))
interceptV1Path("teams", gID).
Reply(403).
JSON(graphTD.ParseableToMap(t, test.err))
interceptV1Path("teams").
Reply(403).
JSON(graphTD.ParseableToMap(t, test.err))
_, err = suite.its.gockAC.
Groups().
GetTeamByID(ctx, test.id, CallConfig{})
require.ErrorIs(t, err, graph.ErrResourceLocked, clues.ToCore(err))
require.ErrorIs(t, err, core.ErrResourceNotAccessible, clues.ToCore(err))
})
}
}

5
src/pkg/services/m365/api/helper_test.go
View File

@ -15,7 +15,6 @@ import (
"github.com/alcionai/corso/src/pkg/control"
"github.com/alcionai/corso/src/pkg/count"
"github.com/alcionai/corso/src/pkg/services/m365/api/graph"
gmock "github.com/alcionai/corso/src/pkg/services/m365/api/graph/mock"
)
// ---------------------------------------------------------------------------
@ -25,12 +24,12 @@ import (
// GockClient produces a new exchange api client that can be
// mocked using gock.
func gockClient(creds account.M365Config, counter *count.Bus) (Client, error) {
s, err := gmock.NewService(creds, counter)
s, err := graph.NewGockService(creds, counter)
if err != nil {
return Client{}, err
}
li, err := gmock.NewService(creds, counter, graph.NoTimeout())
li, err := graph.NewGockService(creds, counter, graph.NoTimeout())
if err != nil {
return Client{}, err
}

8
src/pkg/services/m365/api/sites.go
View File

@ -157,10 +157,6 @@ func (c Sites) GetByID(
err = clues.Stack(core.ErrResourceOwnerNotFound, err)
}
if graph.IsErrResourceLocked(err) {
err = clues.Stack(graph.ErrResourceLocked, err)
}
return nil, err
}
@ -203,10 +199,6 @@ func (c Sites) GetByID(
err = clues.Stack(core.ErrResourceOwnerNotFound, err)
}
if graph.IsErrResourceLocked(err) {
return nil, graph.Stack(ctx, clues.Stack(graph.ErrResourceLocked, err))
}
return nil, err
}

12
src/pkg/services/m365/api/users.go
View File

@ -137,10 +137,6 @@ func (c Users) GetByID(
ByUserId(identifier).
Get(ctx, options)
if err != nil {
if graph.IsErrResourceLocked(err) {
err = clues.Stack(graph.ErrResourceLocked, err)
}
return nil, graph.Stack(ctx, err)
}
@ -198,12 +194,8 @@ func EvaluateMailboxError(err error) error {
}
// must occur before MailFolderNotFound, due to overlapping cases.
if graph.IsErrUserNotFound(err) {
return clues.Stack(core.ErrResourceOwnerNotFound, err)
}
if graph.IsErrResourceLocked(err) {
return clues.Stack(graph.ErrResourceLocked, err)
if errors.Is(err, core.ErrResourceOwnerNotFound) || errors.Is(err, core.ErrResourceNotAccessible) {
return err
}
if graph.IsErrExchangeMailFolderNotFound(err) || graph.IsErrAuthenticationError(err) {

6
src/pkg/services/m365/api/users_test.go
View File

@ -81,16 +81,16 @@ func (suite *UsersUnitSuite) TestEvaluateMailboxError() {
},
{
name: "mail inbox err - user not found",
err: graphTD.ODataErr(string(graph.RequestResourceNotFound)),
err: core.ErrResourceOwnerNotFound,
expect: func(t *testing.T, err error) {
assert.ErrorIs(t, err, core.ErrResourceOwnerNotFound, clues.ToCore(err))
},
},
{
name: "mail inbox err - resoruceLocked",
err: graphTD.ODataErr(string(graph.NotAllowed)),
err: core.ErrResourceNotAccessible,
expect: func(t *testing.T, err error) {
assert.ErrorIs(t, err, graph.ErrResourceLocked, clues.ToCore(err))
assert.ErrorIs(t, err, core.ErrResourceNotAccessible, clues.ToCore(err))
},
},
{