Use filters.In instead of contains. Add tests

src/pkg/services/m365/api/graph/service.go

@@ -174,9 +174,15 @@ func KiotaHTTPClient(
 const (
 	defaultDelay             = 3 * time.Second
 	defaultHTTPClientTimeout = 1 * time.Hour
+
+	// Default retry count for retry middlewares
+	defaultMaxRetries = 3
+	// Retry count for graph adapter
+	//
 	// Bumping retries to 6 since we have noticed that auth token expiry errors
 	// may continue to persist even after 3 retries.
-	defaultMaxRetries = 6
+	adapterMaxRetries = 6
+
 	// FIXME: This should ideally be 0, but if we set to 0, graph
 	// client with automatically set the context timeout to 0 as
 	// well which will make the client unusable.
@@ -203,7 +209,7 @@ type Option func(*clientConfig)
 // populate constructs a clientConfig according to the provided options.
 func populateConfig(opts ...Option) *clientConfig {
 	cc := clientConfig{
-		maxConnectionRetries: defaultMaxRetries,
+		maxConnectionRetries: adapterMaxRetries,
 		maxRetries:           defaultMaxRetries,
 		minDelay:             defaultDelay,
 		timeout:              defaultHTTPClientTimeout,
@@ -376,7 +382,7 @@ func wrapAdapter(gra *msgraphsdkgo.GraphRequestAdapter, cc *clientConfig) *adapt
 	}
 }
 
-var connectionEnded = filters.Contains([]string{
+var connectionEnded = filters.In([]string{
 	"connection reset by peer",
 	"client connection force closed",
 	"read: connection timed out",

src/pkg/services/m365/api/graph/service_test.go

@@ -93,7 +93,7 @@ func (suite *GraphIntgSuite) TestHTTPClient() {
 			checkConfig: func(t *testing.T, c *clientConfig) {
 				assert.Equal(t, defaultDelay, c.minDelay, "default delay")
 				assert.Equal(t, defaultMaxRetries, c.maxRetries, "max retries")
-				assert.Equal(t, defaultMaxRetries, c.maxConnectionRetries, "max connection retries")
+				assert.Equal(t, adapterMaxRetries, c.maxConnectionRetries, "max connection retries")
 			},
 		},
 		{
@@ -221,43 +221,92 @@ func (suite *GraphIntgSuite) TestAdapterWrap_catchesPanic() {
 	require.Contains(t, err.Error(), "panic", clues.ToCore(err))
 }
 
-func (suite *GraphIntgSuite) TestAdapterWrap_retriesConnectionClose() {
+func (suite *GraphIntgSuite) TestAdapterWrap_retriesConnectionInterruptions() {
-	t := suite.T()
+	table := []struct {
-
+		name             string
-	ctx, flush := tester.NewContext(t)
+		providedErr      error
-	defer flush()
+		expectRetryCount int
-
+		expectErr        assert.ErrorAssertionFunc
-	retryInc := 0
+	}{
-
+		{
-	// the panics should get caught and returned as errors
+			name:             "ECONNRESET",
-	alwaysECONNRESET := mwForceResp{
+			providedErr:      syscall.ECONNRESET,
-		err: syscall.ECONNRESET,
+			expectRetryCount: 7,
-		alternate: func(req *http.Request) (bool, *http.Response, error) {
+			expectErr:        assert.Error,
-			retryInc++
+		},
-			return false, nil, nil
+		{
+			name:             "connection reset by peer",
+			providedErr:      clues.New("connection reset by peer"),
+			expectRetryCount: 7,
+			expectErr:        assert.Error,
+		},
+		{
+			name:             "read: connection timed out",
+			providedErr:      clues.New("read: connection timed out"),
+			expectRetryCount: 7,
+			expectErr:        assert.Error,
+		},
+		{
+			name:             "unexpected EOF",
+			providedErr:      io.ErrUnexpectedEOF,
+			expectRetryCount: 7,
+			expectErr:        assert.Error,
+		},
+		{
+			name:             "nil error",
+			providedErr:      nil,
+			expectRetryCount: 1,
+			expectErr:        assert.NoError,
+		},
+		{
+			name:             "non retriable error",
+			providedErr:      clues.New("non retriable error"),
+			expectRetryCount: 1,
+			expectErr:        assert.Error,
 		},
 	}
 
-	adpt, err := CreateAdapter(
+	for _, test := range table {
-		suite.credentials.AzureTenantID,
+		suite.Run(test.name, func() {
-		suite.credentials.AzureClientID,
+			t := suite.T()
-		suite.credentials.AzureClientSecret,
-		count.New(),
-		appendMiddleware(&alwaysECONNRESET),
-		// Configure retry middlewares so that they don't retry on connection reset.
-		// Those middlewares have their own tests to verify retries.
-		MaxRetries(-1))
-	require.NoError(t, err, clues.ToCore(err))
 
-	// Retry delay doesn't really matter here since all requests will be intercepted
+			ctx, flush := tester.NewContext(t)
-	// by the test middleware. Set it to 0 to reduce test runtime.
+			defer flush()
-	aw := adpt.(*adapterWrap)
-	aw.retryDelay = 0
 
-	// the query doesn't matter
+			retryInc := 0
-	_, err = NewService(adpt).Client().Users().Get(ctx, nil)
+			forceErrMW := mwForceResp{
-	require.ErrorIs(t, err, syscall.ECONNRESET, clues.ToCore(err))
+				err: test.providedErr,
-	require.Equal(t, 4, retryInc, "number of retries")
+				// send some dummy response, because kiota retry handler panics
+				// if err is nil and resp is nil.
+				resp: &http.Response{},
+				alternate: func(req *http.Request) (bool, *http.Response, error) {
+					retryInc++
+					return false, nil, nil
+				},
+			}
+
+			adpt, err := CreateAdapter(
+				suite.credentials.AzureTenantID,
+				suite.credentials.AzureClientID,
+				suite.credentials.AzureClientSecret,
+				count.New(),
+				appendMiddleware(&forceErrMW),
+				// Configure retry middlewares so that they don't retry on connection reset.
+				// Those middlewares have their own tests to verify retries.
+				MaxRetries(-1))
+			require.NoError(t, err, clues.ToCore(err))
+
+			// Retry delay doesn't really matter here since all requests will be intercepted
+			// by the test middleware. Set it to 0 to reduce test runtime.
+			aw := adpt.(*adapterWrap)
+			aw.retryDelay = 0
+
+			// the query doesn't matter
+			_, err = NewService(adpt).Client().Users().Get(ctx, nil)
+			test.expectErr(t, err, clues.ToCore(err))
+			require.Equal(t, test.expectRetryCount, retryInc, "number of retries")
+		})
+	}
 }
 
 func (suite *GraphIntgSuite) TestAdapterWrap_retriesBadJWTToken() {
@@ -315,14 +364,14 @@ func (suite *GraphIntgSuite) TestAdapterWrap_retriesBadJWTToken() {
 		NewItemCalendarsItemEventsDeltaRequestBuilder("https://graph.microsoft.com/fnords/beaux/regard", adpt).
 		Get(ctx, nil)
 	assert.ErrorIs(t, err, core.ErrAuthTokenExpired, clues.ToCore(err))
-	assert.Equal(t, 4, retryInc, "number of retries")
+	assert.Equal(t, adapterMaxRetries+1, retryInc, "number of retries")
 
 	retryInc = 0
 
 	// the query doesn't matter
 	_, err = NewService(adpt).Client().Users().Get(ctx, nil)
 	assert.ErrorIs(t, err, core.ErrAuthTokenExpired, clues.ToCore(err))
-	assert.Equal(t, 4, retryInc, "number of retries")
+	assert.Equal(t, adapterMaxRetries+1, retryInc, "number of retries")
 }
 
 // TestAdapterWrap_retriesInvalidRequest tests adapter retries for graph 400
@@ -398,7 +447,7 @@ func (suite *GraphIntgSuite) TestAdapterWrap_retriesInvalidRequest() {
 				_, err = NewService(adpt).Client().Users().Get(ctx, nil)
 				return err
 			},
-			expectedRetries: 4,
+			expectedRetries: adapterMaxRetries + 1,
 		},
 		{
 			name: "POST request, no retry",