Implement rate limit handling

[rickardl]

Implements a rate-limit function to support #233 and resolve #237

func rateLimited(err error) bool {
	if err == nil {
		return false
	}
	rerr, ok := err.(*github.RateLimitError)
	if ok {
		var d = rerr.Rate.Reset.Time.Sub(time.Now())
		//hit rate limit, sleeping
		time.Sleep(d)
		return true
	}
	aerr, ok := err.(*github.AbuseRateLimitError)
	if ok {
		var d = aerr.GetRetryAfter()
		//hit abuse mechanism, sleeping
		time.Sleep(d)
		return true
	}
	return false
}

implemented as

for {
    err := APIRequest()
    if err != nil {
        if rateLimited(err) {
            continue
        } else {
            return err
        }
    }
}

in related GitHub v3 API requests.

[rickardl]

@itsdalmo I might be a bit pre-emptive in my implementation here, I'll see if we can mock and test this behavior later, do you have an idea on how to proceed?

[d]

OMG, we just hit this today, 👍 for posting this patch!

[rickardl]

@d Please lock down to v0.21.0 until this gets carefully reviewed and merged :-)

[itsdalmo]

I think the change here looks reasonable, but considering that we are hitting the abuse rate limit I take that as an indication that we are doing something we should not be doing here. Looking at the diff between v0.21.0 and v0.22.0 I noticed this line. I.e., when fetching pull requests we no longer filter out only those that are open but instead we always fetch everything, which for repositories with a lot of history and activity will always result in hitting a rate limit no matter how many times we retry.

As such, I think think the best fix here is to either roll back the change I linked above or to do the filtering in the GraphQL query (instead of fetching everything and then filtering in the resource as is currently done). And I suggest we try that before implementing a silent retry for rate limit errors 🤔

[jmccann]

While working on updating #189 and testing it I ran into this exact issue and @itsdalmo hit the nail on the head. Whereas before on initial fetch of PRs to filter on we only queried for OPEN ones we now query for basically ALL of them. This resulted in a check that takes a long time to run. I've been doing manual testing but am sure if I had it running every minute I'd for sure have hit the rate limit ... which doesn't reset for an hour I think.