Skip to content

Commit

Permalink
Support x-ratelimit-reset handling for secondary rate limits (#2775)
Browse files Browse the repository at this point in the history
  • Loading branch information
@gofri
gofri committed May 14, 2023
1 parent 279eacf commit 53cecba
Show file tree
Hide file tree
Showing 4 changed files with 107 additions and 16 deletions.
14 changes: 7 additions & 7 deletions example/go.mod
View file
Expand Up @@ -4,21 +4,21 @@ go 1.17

require (
github.com/bradleyfalzon/ghinstallation/v2 v2.0.4
github.com/gofri/go-github-ratelimit v1.0.1
github.com/gofri/go-github-ratelimit v1.0.3
github.com/google/go-github/v52 v52.0.0
golang.org/x/crypto v0.1.0
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be
golang.org/x/crypto v0.7.0
golang.org/x/oauth2 v0.7.0
google.golang.org/appengine v1.6.7
)

require (
github.com/golang-jwt/jwt/v4 v4.0.0 // indirect
github.com/golang/protobuf v1.3.2 // indirect
github.com/golang/protobuf v1.5.2 // indirect
github.com/google/go-github/v41 v41.0.0 // indirect
github.com/google/go-querystring v1.1.0 // indirect
golang.org/x/net v0.7.0 // indirect
golang.org/x/sys v0.5.0 // indirect
golang.org/x/term v0.5.0 // indirect
golang.org/x/net v0.9.0 // indirect
golang.org/x/sys v0.7.0 // indirect
golang.org/x/term v0.7.0 // indirect
)

// Use version at HEAD, not the latest published.
Expand Down
12 changes: 12 additions & 0 deletions example/go.sum
View file
Expand Up @@ -2,12 +2,17 @@ github.com/bradleyfalzon/ghinstallation/v2 v2.0.4 h1:tXKVfhE7FcSkhkv0UwkLvPDeZ4k
github.com/bradleyfalzon/ghinstallation/v2 v2.0.4/go.mod h1:B40qPqJxWE0jDZgOR1JmaMy+4AY1eBP+IByOvqyAKp0=
github.com/gofri/go-github-ratelimit v1.0.1 h1:sgefSzxhnvwZ+wR9uZ4l9TnjgLuNiwipJVzJL4YLj9A=
github.com/gofri/go-github-ratelimit v1.0.1/go.mod h1:OnCi5gV+hAG/LMR7llGhU7yHt44se9sYgKPnafoL7RY=
github.com/gofri/go-github-ratelimit v1.0.3 h1:Ocs2jaYokZDzgvqaajX+g04dqFyVqL0JQzoO7d2wmlk=
github.com/gofri/go-github-ratelimit v1.0.3/go.mod h1:OnCi5gV+hAG/LMR7llGhU7yHt44se9sYgKPnafoL7RY=
github.com/golang-jwt/jwt/v4 v4.0.0 h1:RAqyYixv1p7uEnocuy8P1nru5wprCh/MH2BIlW5z5/o=
github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg=
github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs=
github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U=
github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk=
github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY=
github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
Expand All @@ -21,6 +26,7 @@ golang.org/x/crypto v0.0.0-20210817164053-32db794688a5/go.mod h1:GvvjBRRGRdwPK5y
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU=
golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw=
golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
Expand All @@ -29,8 +35,10 @@ golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug
golang.org/x/net v0.1.0/go.mod h1:Cx3nUiGt4eDBEyega/BKRp+/AlGL8hYe7U9odMt2Cco=
golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g=
golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be h1:vEDujvNQGv4jgYKudGeI/+DAX4Jffq6hpD55MmoEvKs=
golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
golang.org/x/oauth2 v0.7.0/go.mod h1:hPLQkd9LyjfXTiRohC/41GhcFqxisoUQ99sCUOHO9x4=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
Expand All @@ -41,11 +49,13 @@ golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU=
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.5.0 h1:n2a8QNdAb0sZNpU9R1ALUXBbY+w51fCQDN+7EdxNBsY=
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
Expand All @@ -59,3 +69,5 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c=
google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc=
google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
34 changes: 27 additions & 7 deletions github/github.go
View file
Expand Up @@ -40,6 +40,7 @@ const (
headerRateRemaining = "X-RateLimit-Remaining"
headerRateReset = "X-RateLimit-Reset"
headerOTP = "X-GitHub-OTP"
headerRetryAfter = "Retry-After"

headerTokenExpiration = "GitHub-Authentication-Token-Expiration"

Expand Down Expand Up @@ -677,6 +678,30 @@ func parseRate(r *http.Response) Rate {
return rate
}

// parseSecondaryRate parses the secondary rate related headers,
// and returns the time to retry after.
func parseSecondaryRate(r *http.Response) *time.Duration {
// According to GitHub support, the "Retry-After" header value will be
// an integer which represents the number of seconds that one should
// wait before resuming making requests.
if v := r.Header.Get(headerRetryAfter); v != "" {
retryAfterSeconds, _ := strconv.ParseInt(v, 10, 64) // Error handling is noop.
retryAfter := time.Duration(retryAfterSeconds) * time.Second
return &retryAfter
}

// According to GitHub support, endpoints might return x-ratelimit-reset instead,
// as an integer which represents the number of seconds since epoch UTC,
// represting the time to resume making requests.
if v := r.Header.Get(headerRateReset); v != "" {
secondsSinceEpoch, _ := strconv.ParseInt(v, 10, 64) // Error handling is noop.
retryAfter := time.Until(time.Unix(secondsSinceEpoch, 0))
return &retryAfter
}

return nil
}

// parseTokenExpiration parses the TokenExpiration related headers.
// Returns 0001-01-01 if the header is not defined or could not be parsed.
func parseTokenExpiration(r *http.Response) Timestamp {
Expand Down Expand Up @@ -1156,13 +1181,8 @@ func CheckResponse(r *http.Response) error {
Response: errorResponse.Response,
Message: errorResponse.Message,
}
if v := r.Header["Retry-After"]; len(v) > 0 {
// According to GitHub support, the "Retry-After" header value will be
// an integer which represents the number of seconds that one should
// wait before resuming making requests.
retryAfterSeconds, _ := strconv.ParseInt(v[0], 10, 64) // Error handling is noop.
retryAfter := time.Duration(retryAfterSeconds) * time.Second
abuseRateLimitError.RetryAfter = &retryAfter
if retryAfter := parseSecondaryRate(r); retryAfter != nil {
abuseRateLimitError.RetryAfter = retryAfter
}
return abuseRateLimitError
default:
Expand Down
63 changes: 61 additions & 2 deletions github/github_test.go
View file
Expand Up @@ -17,6 +17,7 @@ import (
"os"
"path"
"reflect"
"strconv"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -1475,14 +1476,14 @@ func TestDo_rateLimit_abuseRateLimitErrorEnterprise(t *testing.T) {
}
}

// Ensure *AbuseRateLimitError.RetryAfter is parsed correctly.
// Ensure *AbuseRateLimitError.RetryAfter is parsed correctly for the Retry-After header.
func TestDo_rateLimit_abuseRateLimitError_retryAfter(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json; charset=utf-8")
w.Header().Set("Retry-After", "123") // Retry after value of 123 seconds.
w.Header().Set(headerRetryAfter, "123") // Retry after value of 123 seconds.
w.WriteHeader(http.StatusForbidden)
fmt.Fprintln(w, `{
"message": "You have triggered an abuse detection mechanism ...",
Expand Down Expand Up @@ -1528,6 +1529,64 @@ func TestDo_rateLimit_abuseRateLimitError_retryAfter(t *testing.T) {
}
}

// Ensure *AbuseRateLimitError.RetryAfter is parsed correctly for the x-ratelimit-reset header.
func TestDo_rateLimit_abuseRateLimitError_xRateLimitReset(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()

// x-ratelimit-reset value of 123 seconds into the future.
blockUntil := time.Now().Add(time.Duration(123) * time.Second).Unix()

mux.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json; charset=utf-8")
w.Header().Set(headerRateReset, strconv.Itoa(int(blockUntil)))
w.Header().Set(headerRateRemaining, "1") // set remaining to a value > 0 to distinct from a primary rate limit
w.WriteHeader(http.StatusForbidden)
fmt.Fprintln(w, `{
"message": "You have triggered an abuse detection mechanism ...",
"documentation_url": "https://docs.github.com/en/rest/overview/resources-in-the-rest-api#abuse-rate-limits"
}`)
})

req, _ := client.NewRequest("GET", ".", nil)
ctx := context.Background()
_, err := client.Do(ctx, req, nil)

if err == nil {
t.Error("Expected error to be returned.")
}
abuseRateLimitErr, ok := err.(*AbuseRateLimitError)
if !ok {
t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err)
}
if abuseRateLimitErr.RetryAfter == nil {
t.Fatalf("abuseRateLimitErr RetryAfter is nil, expected not-nil")
}
// the retry after value might be a bit smaller than the original duration because the duration is calculated from the expected end-of-cooldown time
if got, want := *abuseRateLimitErr.RetryAfter, 123*time.Second; want-got > 1*time.Second {
t.Errorf("abuseRateLimitErr RetryAfter = %v, want %v", got, want)
}

// expect prevention of a following request
if _, err = client.Do(ctx, req, nil); err == nil {
t.Error("Expected error to be returned.")
}
abuseRateLimitErr, ok = err.(*AbuseRateLimitError)
if !ok {
t.Fatalf("Expected a *AbuseRateLimitError error; got %#v.", err)
}
if abuseRateLimitErr.RetryAfter == nil {
t.Fatalf("abuseRateLimitErr RetryAfter is nil, expected not-nil")
}
// the saved duration might be a bit smaller than Retry-After because the duration is calculated from the expected end-of-cooldown time
if got, want := *abuseRateLimitErr.RetryAfter, 123*time.Second; want-got > 1*time.Second {
t.Errorf("abuseRateLimitErr RetryAfter = %v, want %v", got, want)
}
if got, wantSuffix := abuseRateLimitErr.Message, "not making remote request."; !strings.HasSuffix(got, wantSuffix) {
t.Errorf("Expected request to be prevented because of secondary rate limit, got: %v.", got)
}
}

func TestDo_noContent(t *testing.T) {
client, mux, _, teardown := setup()
defer teardown()
Expand Down

0 comments on commit 53cecba

Please sign in to comment.