fix: retry GraphQL calls the same way for v3 API calls (#524)

internal/scm/github/github.go

@@ -690,7 +690,7 @@ func (g *Github) getPullRequests(ctx context.Context, branchName string, repos [
 	)
 
 	result := map[string]graphqlRepo{}
-	err := g.makeGraphQLRequest(ctx, query, queryVariables, &result)
+	err := g.makeGraphQLRequestWithRetry(ctx, query, queryVariables, &result)
 	if err != nil {
 		return nil, err
 	}

internal/scm/github/graphql.go

@@ -12,6 +12,12 @@ import (
 	"github.com/pkg/errors"
 )
 
+func (g *Github) makeGraphQLRequestWithRetry(ctx context.Context, query string, data interface{}, res interface{}) error {
+	return retryAPIRequest(ctx, func() error {
+		return g.makeGraphQLRequest(ctx, query, data, res)
+	})
+}
+
 func (g *Github) makeGraphQLRequest(ctx context.Context, query string, data interface{}, res interface{}) error {
 	type reqData struct {
 		Query string      `json:"query"`
@@ -46,6 +52,11 @@ func (g *Github) makeGraphQLRequest(ctx context.Context, query string, data inte
 	}
 	defer resp.Body.Close()
 
+	retryAfterErr := retryAfterFromHTTPResponse(resp)
+	if retryAfterErr != nil {
+		return retryAfterErr
+	}
+
 	resultData := struct {
 		Data   json.RawMessage `json:"data"`
 		Errors []struct {

internal/scm/github/retry.go

@@ -2,7 +2,9 @@ package github
 
 import (
 	"context"
+	"fmt"
 	"math"
+	"net/http"
 	"strconv"
 	"strings"
 	"time"
@@ -24,6 +26,12 @@ var sleep = func(ctx context.Context, d time.Duration) error {
 	}
 }
 
+type retryAfterError time.Duration
+
+func (r retryAfterError) Error() string {
+	return fmt.Sprintf("rate limit exceeded, waiting for %s)", time.Duration(r))
+}
+
 // retry runs a GitHub API request and retries it if a temporary error occurred
 func retry[K any](ctx context.Context, fn func() (K, *github.Response, error)) (K, *github.Response, error) {
 	var val K
@@ -38,44 +46,79 @@ func retry[K any](ctx context.Context, fn func() (K, *github.Response, error)) (
 
 // retryWithoutReturn runs a GitHub API request with no return value and retries it if a temporary error occurred
 func retryWithoutReturn(ctx context.Context, fn func() (*github.Response, error)) (*github.Response, error) {
+	var response *github.Response
+	err := retryAPIRequest(ctx, func() error {
+		var err error
+		response, err = fn()
+
+		if response != nil {
+			httpResponse := response.Response
+			retryAfterErr := retryAfterFromHTTPResponse(httpResponse)
+			if retryAfterErr != nil {
+				return retryAfterErr
+			}
+		}
+
+		return err
+	})
+	return response, err
+}
+
+func retryAPIRequest(ctx context.Context, fn func() error) error {
 	tries := 0
 
 	for {
 		tries++
 
-		githubResp, err := fn()
+		err := fn()
 		if err == nil { // NB!
-			return githubResp, nil
-		}
-
-		// Get the number of retry seconds (if any)
-		retryAfter := 0
-		if githubResp != nil && githubResp.Header != nil {
-			retryAfterStr := githubResp.Header.Get(retryHeader)
-			if retryAfterStr != "" {
-				var err error
-				if retryAfter, err = strconv.Atoi(retryAfterStr); err != nil {
-					return githubResp, errors.WithMessage(err, "could not convert Retry-After header")
-				}
-			}
+			return nil
 		}
 
+		var retryAfter retryAfterError
 		switch {
 		// If GitHub has specified how long we should wait, use that information
-		case retryAfter != 0:
-			err := sleep(ctx, time.Duration(retryAfter)*time.Second)
+		case errors.As(err, &retryAfter):
+			err := sleep(ctx, time.Duration(retryAfter))
 			if err != nil {
-				return githubResp, err
+				return err
 			}
 		// If secondary rate limit error, use an exponential back-off to determine the wait
 		case strings.Contains(err.Error(), "secondary rate limit"):
-			err := sleep(ctx, time.Duration(math.Pow(float64(tries), 3))*10*time.Second)
+			err := sleep(ctx, exponentialBackoff(tries))
 			if err != nil {
-				return githubResp, err
+				return err
 			}
 		// If any other error, return the error
 		default:
-			return githubResp, err
+			return err
 		}
 	}
 }
+
+func retryAfterFromHTTPResponse(response *http.Response) error {
+	if response == nil {
+		return nil
+	}
+
+	retryAfterStr := response.Header.Get(retryHeader)
+	if retryAfterStr == "" {
+		return nil
+	}
+
+	retryAfterSeconds, err := strconv.Atoi(retryAfterStr)
+	if err != nil {
+		return nil
+	}
+
+	if retryAfterSeconds <= 0 {
+		return nil
+	}
+
+	return retryAfterError(time.Duration(retryAfterSeconds) * time.Second)
+}
+
+func exponentialBackoff(tries int) time.Duration {
+	// 10, 80, 270... seconds
+	return time.Duration(math.Pow(float64(tries), 3)) * 10 * time.Second
+}