fix: Retry when there are 429s on flag metadata fetches (#446)

Retry when there are 429s

Author: Mike Zorn

internal/dev_server/adapters/api.go

@@ -4,9 +4,8 @@ import (
 	"context"
 	"fmt"
 	"log"
-	"net/url"
-	"strconv"
 
+	"github.com/launchdarkly/ldcli/internal/dev_server/adapters/internal"
 	"github.com/pkg/errors"
 
 	ldapi "github.com/launchdarkly/api-client-go/v14"
@@ -65,7 +64,8 @@ func (a apiClientApi) GetProjectEnvironments(ctx context.Context, projectKey str
 }
 
 func (a apiClientApi) getFlags(ctx context.Context, projectKey string, href *string) ([]ldapi.FeatureFlag, error) {
-	return getPaginatedItems(ctx, projectKey, href, func(ctx context.Context, projectKey string, limit, offset *int64) (*ldapi.FeatureFlags, error) {
+	return internal.GetPaginatedItems(ctx, projectKey, href, func(ctx context.Context, projectKey string, limit, offset *int64) (flags *ldapi.FeatureFlags, err error) {
+		// loop until we do not get rate limited
 		query := a.apiClient.FeatureFlagsApi.GetFeatureFlags(ctx, projectKey).Limit(100)
 
 		if limit != nil {
@@ -75,10 +75,7 @@ func (a apiClientApi) getFlags(ctx context.Context, projectKey string, href *str
 		if offset != nil {
 			query = query.Offset(*offset)
 		}
-
-		flags, _, err := query.
-			Execute()
-		return flags, err
+		return internal.Retry429s(query.Execute)
 	})
 }
 
@@ -105,60 +102,3 @@ func (a apiClientApi) getEnvironments(ctx context.Context, projectKey string, hr
 
 	return envs.Items, nil
 }
-
-func getPaginatedItems[T any, R interface {
-	GetItems() []T
-	GetLinks() map[string]ldapi.Link
-}](ctx context.Context, projectKey string, href *string, fetchFunc func(context.Context, string, *int64, *int64) (R, error)) ([]T, error) {
-	var result R
-	var err error
-
-	if href == nil {
-		result, err = fetchFunc(ctx, projectKey, nil, nil)
-		if err != nil {
-			return nil, err
-		}
-	} else {
-		limit, offset, err := parseHref(*href)
-		if err != nil {
-			return nil, errors.Wrapf(err, "unable to parse href for next link: %s", *href)
-		}
-		result, err = fetchFunc(ctx, projectKey, &limit, &offset)
-		if err != nil {
-			return nil, err
-		}
-	}
-
-	items := result.GetItems()
-
-	if links := result.GetLinks(); links != nil {
-		if next, ok := links["next"]; ok && next.Href != nil {
-			newItems, err := getPaginatedItems(ctx, projectKey, next.Href, fetchFunc)
-			if err != nil {
-				return nil, err
-			}
-			items = append(items, newItems...)
-		}
-	}
-
-	return items, nil
-}
-
-func parseHref(href string) (limit, offset int64, err error) {
-	parsedUrl, err := url.Parse(href)
-	if err != nil {
-		return
-	}
-	l, err := strconv.Atoi(parsedUrl.Query().Get("limit"))
-	if err != nil {
-		return
-	}
-	o, err := strconv.Atoi(parsedUrl.Query().Get("offset"))
-	if err != nil {
-		return
-	}
-
-	limit = int64(l)
-	offset = int64(o)
-	return
-}

internal/dev_server/adapters/internal/api_util.go

@@ -0,0 +1,108 @@
+package internal
+
+import (
+	"context"
+	"log"
+	"net/http"
+	"net/url"
+	"strconv"
+	"time"
+
+	"github.com/launchdarkly/api-client-go/v14"
+	"github.com/pkg/errors"
+)
+
+func GetPaginatedItems[T any, R interface {
+	GetItems() []T
+	GetLinks() map[string]ldapi.Link
+}](ctx context.Context, projectKey string, href *string, fetchFunc func(context.Context, string, *int64, *int64) (R, error)) ([]T, error) {
+	var result R
+	var err error
+
+	if href == nil {
+		result, err = fetchFunc(ctx, projectKey, nil, nil)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		limit, offset, err := parseHref(*href)
+		if err != nil {
+			return nil, errors.Wrapf(err, "unable to parse href for next link: %s", *href)
+		}
+		result, err = fetchFunc(ctx, projectKey, &limit, &offset)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	items := result.GetItems()
+
+	if links := result.GetLinks(); links != nil {
+		if next, ok := links["next"]; ok && next.Href != nil {
+			newItems, err := GetPaginatedItems(ctx, projectKey, next.Href, fetchFunc)
+			if err != nil {
+				return nil, err
+			}
+			items = append(items, newItems...)
+		}
+	}
+
+	return items, nil
+}
+
+func parseHref(href string) (limit, offset int64, err error) {
+	parsedUrl, err := url.Parse(href)
+	if err != nil {
+		return
+	}
+	l, err := strconv.Atoi(parsedUrl.Query().Get("limit"))
+	if err != nil {
+		return
+	}
+	o, err := strconv.Atoi(parsedUrl.Query().Get("offset"))
+	if err != nil {
+		return
+	}
+
+	limit = int64(l)
+	offset = int64(o)
+	return
+}
+
+//go:generate go run go.uber.org/mock/mockgen -destination mocks.go -package internal . MockableTime
+type MockableTime interface {
+	Sleep(duration time.Duration)
+	Now() time.Time
+}
+
+type realTime struct{}
+
+func (realTime) Sleep(duration time.Duration) {
+	time.Sleep(duration)
+}
+
+func (realTime) Now() time.Time {
+	return time.Now()
+}
+
+var timeImpl MockableTime = realTime{}
+
+func Retry429s[T any](requester func() (T, *http.Response, error)) (result T, err error) {
+	for {
+		var res *http.Response
+		result, res, err = requester()
+		if res.StatusCode == 429 {
+			resetUnixMillisString := res.Header.Get("X-Ratelimit-Reset")
+			resetUnixMillis, strconvErr := strconv.ParseInt(resetUnixMillisString, 10, 64)
+			if strconvErr != nil {
+				err = errors.Wrapf(err, `unable to retry rate limited request: X-RateLimit-Reset: "%s" was not parsable`, resetUnixMillisString)
+				return
+			}
+			sleep := resetUnixMillis - timeImpl.Now().UnixMilli()
+			log.Printf("Got 429 in API response. Retrying in %d milliseconds.", sleep)
+			timeImpl.Sleep(time.Duration(sleep) * time.Millisecond)
+		} else {
+			return
+		}
+	}
+}

internal/dev_server/adapters/api_test.go …erver/adapters/internal/api_util_test.gointernal/dev_server/adapters/api_test.go renamed to internal/dev_server/adapters/internal/api_util_test.go internal/dev_server/adapters/api_test.go renamed to internal/dev_server/adapters/internal/api_util_test.go

@@ -1,11 +1,14 @@
-package adapters
+package internal
 
 import (
 	"context"
+	"net/http"
 	"testing"
+	"time"
 
 	ldapi "github.com/launchdarkly/api-client-go/v14"
 	"github.com/stretchr/testify/assert"
+	"go.uber.org/mock/gomock"
 )
 
 type testItem struct {
@@ -131,7 +134,7 @@ func TestGetPaginatedItems(t *testing.T) {
 				return result, nil
 			}
 
-			items, err := getPaginatedItems(ctx, projectKey, nil, fetchFunc)
+			items, err := GetPaginatedItems(ctx, projectKey, nil, fetchFunc)
 
 			if tc.expectedError {
 				assert.Error(t, err)
@@ -143,6 +146,44 @@ func TestGetPaginatedItems(t *testing.T) {
 	}
 }
 
+func TestRetry429s(t *testing.T) {
+	t.Run("it should call exactly once if not a 429", func(t *testing.T) {
+		called := 0
+		res, err := Retry429s(func() (string, *http.Response, error) {
+			called++
+			return "lol", &http.Response{StatusCode: 200}, nil
+		})
+		assert.Equal(t, "lol", res)
+		assert.NoError(t, err)
+		assert.Equal(t, 1, called)
+	})
+
+	t.Run("it should retry when a 429 is received", func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		timeMock := NewMockMockableTime(ctrl)
+		defer func() { ctrl.Finish() }()
+		timeImpl = timeMock
+		defer func() { timeImpl = realTime{} }()
+		timeMock.EXPECT().Now().Return(time.UnixMilli(0))
+		timeMock.EXPECT().Sleep(time.Duration(1000) * time.Millisecond)
+
+		called := 0
+		res, err := Retry429s(func() (string, *http.Response, error) {
+			called++
+			if called > 1 {
+				return "lol", &http.Response{StatusCode: 200}, nil
+			} else {
+				header := make(http.Header)
+				header.Set("X-Ratelimit-Reset", "1000")
+				return "", &http.Response{StatusCode: 429, Header: header}, nil
+			}
+		})
+		assert.Equal(t, "lol", res)
+		assert.NoError(t, err)
+		assert.Equal(t, 2, called)
+	})
+}
+
 func strPtr(s string) *string {
 	return &s
 }