From 6a31f13e015d1f560b90c5e50cfc936a69f9f8df Mon Sep 17 00:00:00 2001 From: Johan Lindell Date: Thu, 23 Jan 2025 20:54:43 +0100 Subject: [PATCH] fix: retry GraphQL calls the same way for v3 API calls (#524) --- internal/scm/github/github.go | 2 +- internal/scm/github/graphql.go | 11 +++++ internal/scm/github/retry.go | 83 ++++++++++++++++++++++++++-------- 3 files changed, 75 insertions(+), 21 deletions(-) diff --git a/internal/scm/github/github.go b/internal/scm/github/github.go index 49e853f0..62538c03 100755 --- a/internal/scm/github/github.go +++ b/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 } diff --git a/internal/scm/github/graphql.go b/internal/scm/github/graphql.go index d2923106..231cdd10 100644 --- a/internal/scm/github/graphql.go +++ b/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 { diff --git a/internal/scm/github/retry.go b/internal/scm/github/retry.go index 184da782..c9865794 100644 --- a/internal/scm/github/retry.go +++ b/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 +}