properly declare resourceNotFound for sites (#3728)

Missing sites return an ItemNotFound error code,
which we aren't currently handling when doing
queries for individual sites.  This change catches that error code and stacks the error with a
resourceOwnerNotFound sentinel.

---

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

- [x] ⛔ No

#### Type of change

- [x] 🐛 Bugfix

#### Test Plan

- [x] 💪 Manual
- [x] 💚 E2E

src/internal/m365/graph/errors.go

@@ -29,8 +29,8 @@ const (
 	activityLimitReached        errorCode = "activityLimitReached"
 	emailFolderNotFound         errorCode = "ErrorSyncFolderNotFound"
 	errorAccessDenied           errorCode = "ErrorAccessDenied"
-	itemNotFound                errorCode = "ErrorItemNotFound"
+	errorItemNotFound           errorCode = "ErrorItemNotFound"
-	itemNotFoundShort           errorCode = "itemNotFound"
+	itemNotFound                errorCode = "itemNotFound"
 	MailboxNotEnabledForRESTAPI errorCode = "MailboxNotEnabledForRESTAPI"
 	malwareDetected             errorCode = "malwareDetected"
 	// nameAlreadyExists occurs when a request with
@@ -111,8 +111,8 @@ func IsErrDeletedInFlight(err error) bool {
 
 	if hasErrorCode(
 		err,
+		errorItemNotFound,
 		itemNotFound,
-		itemNotFoundShort,
 		syncFolderNotFound,
 	) {
 		return true
@@ -121,6 +121,10 @@ func IsErrDeletedInFlight(err error) bool {
 	return false
 }
 
+func IsErrItemNotFound(err error) bool {
+	return hasErrorCode(err, itemNotFound)
+}
+
 func IsErrInvalidDelta(err error) bool {
 	return hasErrorCode(err, syncStateNotFound, resyncRequired, syncStateInvalid) ||
 		errors.Is(err, ErrInvalidDelta)
@@ -133,7 +137,7 @@ func IsErrQuotaExceeded(err error) bool {
 func IsErrExchangeMailFolderNotFound(err error) bool {
 	// Not sure if we can actually see a resourceNotFound error here. I've only
 	// seen the latter two.
-	return hasErrorCode(err, ResourceNotFound, itemNotFound, MailboxNotEnabledForRESTAPI)
+	return hasErrorCode(err, ResourceNotFound, errorItemNotFound, MailboxNotEnabledForRESTAPI)
 }
 
 func IsErrUserNotFound(err error) bool {

src/internal/m365/graph/errors_test.go

@@ -101,7 +101,7 @@ func (suite *GraphErrorsUnitSuite) TestIsErrDeletedInFlight() {
 		},
 		{
 			name:   "not-found oDataErr",
-			err:    odErr(string(itemNotFound)),
+			err:    odErr(string(errorItemNotFound)),
 			expect: assert.True,
 		},
 		{
@@ -531,3 +531,42 @@ func (suite *GraphErrorsUnitSuite) TestGraphStack_labels() {
 		})
 	}
 }
+
+func (suite *GraphErrorsUnitSuite) TestIsErrItemNotFound() {
+	table := []struct {
+		name   string
+		err    error
+		expect assert.BoolAssertionFunc
+	}{
+		{
+			name:   "nil",
+			err:    nil,
+			expect: assert.False,
+		},
+		{
+			name:   "non-matching",
+			err:    assert.AnError,
+			expect: assert.False,
+		},
+		{
+			name:   "as",
+			err:    ErrInvalidDelta,
+			expect: assert.False,
+		},
+		{
+			name:   "non-matching oDataErr",
+			err:    odErr("fnords"),
+			expect: assert.False,
+		},
+		{
+			name:   "item nott found oDataErr",
+			err:    odErr(string(itemNotFound)),
+			expect: assert.True,
+		},
+	}
+	for _, test := range table {
+		suite.Run(test.name, func() {
+			test.expect(suite.T(), IsErrItemNotFound(test.err))
+		})
+	}
+}

src/pkg/services/m365/api/sites.go

@@ -117,9 +117,21 @@ func (c Sites) GetByID(ctx context.Context, identifier string) (models.Siteable,
 	ctx = clues.Add(ctx, "given_site_id", identifier)
 
 	if siteIDRE.MatchString(identifier) {
-		resp, err = c.Stable.Client().Sites().BySiteId(identifier).Get(ctx, nil)
+		resp, err = c.Stable.
+			Client().
+			Sites().
+			BySiteId(identifier).
+			Get(ctx, nil)
 		if err != nil {
-			return nil, graph.Wrap(ctx, err, "getting site by id")
+			err := graph.Wrap(ctx, err, "getting site by id")
+
+			// a 404 when getting sites by ID returns an itemNotFound
+			// error code, instead of something more sensible.
+			if graph.IsErrItemNotFound(err) {
+				err = clues.Stack(graph.ErrResourceOwnerNotFound, err)
+			}
+
+			return nil, err
 		}
 
 		return resp, err
@@ -147,7 +159,15 @@ func (c Sites) GetByID(ctx context.Context, identifier string) (models.Siteable,
 		NewItemSitesSiteItemRequestBuilder(rawURL, c.Stable.Adapter()).
 		Get(ctx, nil)
 	if err != nil {
-		return nil, graph.Wrap(ctx, err, "getting site by weburl")
+		err := graph.Wrap(ctx, err, "getting site by weburl")
+
+		// a 404 when getting sites by ID returns an itemNotFound
+		// error code, instead of something more sensible.
+		if graph.IsErrItemNotFound(err) {
+			err = clues.Stack(graph.ErrResourceOwnerNotFound, err)
+		}
+
+		return nil, err
 	}
 
 	return resp, err

src/pkg/services/m365/api/sites_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/suite"
 
 	"github.com/alcionai/corso/src/internal/common/ptr"
+	"github.com/alcionai/corso/src/internal/m365/graph"
 	"github.com/alcionai/corso/src/internal/tester"
 	"github.com/alcionai/corso/src/pkg/fault"
 	"github.com/alcionai/corso/src/pkg/services/m365/api"
@@ -112,7 +113,7 @@ func TestSitesIntgSuite(t *testing.T) {
 	suite.Run(t, &SitesIntgSuite{
 		Suite: tester.NewIntegrationSuite(
 			t,
-			[][]string{tester.M365AcctCredEnvs, tester.AWSStorageCredEnvs}),
+			[][]string{tester.M365AcctCredEnvs}),
 	})
 }
 
@@ -126,7 +127,9 @@ func (suite *SitesIntgSuite) TestGetAll() {
 	ctx, flush := tester.NewContext(t)
 	defer flush()
 
-	sites, err := suite.its.ac.Sites().GetAll(ctx, fault.New(true))
+	sites, err := suite.its.ac.
+		Sites().
+		GetAll(ctx, fault.New(true))
 	require.NoError(t, err)
 	require.NotZero(t, len(sites), "must have at least one site")
 
@@ -137,11 +140,12 @@ func (suite *SitesIntgSuite) TestGetAll() {
 
 func (suite *SitesIntgSuite) TestSites_GetByID() {
 	var (
-		t       = suite.T()
+		t               = suite.T()
-		siteID  = tester.M365SiteID(t)
+		siteID          = tester.M365SiteID(t)
-		host    = strings.Split(siteID, ",")[0]
+		host            = strings.Split(siteID, ",")[0]
-		shortID = strings.TrimPrefix(siteID, host+",")
+		shortID         = strings.TrimPrefix(siteID, host+",")
-		siteURL = tester.M365SiteURL(t)
+		siteURL         = tester.M365SiteURL(t)
+		modifiedSiteURL = siteURL + "foo"
 	)
 
 	sitesAPI := suite.its.ac.Sites()
@@ -149,26 +153,81 @@ func (suite *SitesIntgSuite) TestSites_GetByID() {
 	table := []struct {
 		name      string
 		id        string
-		expectErr assert.ErrorAssertionFunc
+		expectErr func(*testing.T, error)
 	}{
-		{"3 part id", siteID, assert.NoError},
+		{
-		{"2 part id", shortID, assert.NoError},
+			name: "3 part id",
-		{"malformed id", uuid.NewString(), assert.Error},
+			id:   siteID,
-		{"random id", uuid.NewString() + "," + uuid.NewString(), assert.Error},
+			expectErr: func(t *testing.T, err error) {
-		{"url", siteURL, assert.NoError},
+				assert.NoError(t, err, clues.ToCore(err))
-		{"host only", host, assert.NoError},
+			},
-		{"malformed url", "barunihlda", assert.Error},
+		},
-		{"non-matching url", "https://test/sites/testing", assert.Error},
+		{
+			name: "2 part id",
+			id:   shortID,
+			expectErr: func(t *testing.T, err error) {
+				assert.NoError(t, err, clues.ToCore(err))
+			},
+		},
+		{
+			name: "malformed id",
+			id:   uuid.NewString(),
+			expectErr: func(t *testing.T, err error) {
+				assert.Error(t, err, clues.ToCore(err))
+			},
+		},
+		{
+			name: "random id",
+			id:   uuid.NewString() + "," + uuid.NewString(),
+			expectErr: func(t *testing.T, err error) {
+				assert.ErrorIs(t, err, graph.ErrResourceOwnerNotFound, clues.ToCore(err))
+			},
+		},
+		{
+			name: "url",
+			id:   siteURL,
+			expectErr: func(t *testing.T, err error) {
+				assert.NoError(t, err, clues.ToCore(err))
+			},
+		},
+		{
+			name: "host only",
+			id:   host,
+			expectErr: func(t *testing.T, err error) {
+				assert.NoError(t, err, clues.ToCore(err))
+			},
+		},
+		{
+			name: "malformed url",
+			id:   "barunihlda",
+			expectErr: func(t *testing.T, err error) {
+				assert.Error(t, err, clues.ToCore(err))
+			},
+		},
+		{
+			name: "well formed url, invalid hostname",
+			id:   "https://test/sites/testing",
+			expectErr: func(t *testing.T, err error) {
+				assert.Error(t, err, clues.ToCore(err))
+			},
+		},
+		{
+			name: "well formed url, no sites match",
+			id:   modifiedSiteURL,
+			expectErr: func(t *testing.T, err error) {
+				assert.ErrorIs(t, err, graph.ErrResourceOwnerNotFound, clues.ToCore(err))
+			},
+		},
 	}
 	for _, test := range table {
 		suite.Run(test.name, func() {
+			t := suite.T()
+
 			ctx, flush := tester.NewContext(t)
 			defer flush()
 
-			t := suite.T()
-
 			_, err := sitesAPI.GetByID(ctx, test.id)
-			test.expectErr(t, err, clues.ToCore(err))
+			test.expectErr(t, err)
 		})
 	}
 }