Testing rate limits along with pagination?

[jlucktay]

Hi there, and thank you for this library, it's super helpful!

I'm trying to test a scenario where my GitHub client hits the rate limit whilst paginating through some responses, listing a considerable number of repositories from a large organisation. However, I can't seem to find the right sequence of mock.MockBackendOption to set things off.

```go func (c Client) ListRepos(ctx context.Context, org string) ([]string, error) { listOpts := &github.RepositoryListByOrgOptions{ Type: "all", Sort: "pushed", Direction: "desc", ListOptions: github.ListOptions{ PerPage: 100, }, } for { repos, resp, err := c.github.Repositories.ListByOrg(ctx, org, listOpts) if err != nil { var rle *github.RateLimitError if !errors.As(err, &rle) { return nil, err } // ... continue } defer resp.Body.Close() for index := range repos { // ... } if resp.NextPage == 0 { break } listOpts.Page = resp.NextPage } return // ... } ```

Abbreviated snippet of my code for listing repos, paginating, and handling rate limit errors

In my test for this function, this is how I have structured my mocked *http.Client:

```go mockedHC := mock.NewMockedHTTPClient( mock.WithRequestMatchPages( mock.GetOrgsReposByOrg, []github.Repository{ // ... first page of repos to return }, ), mock.WithRequestMatchHandler( mock.GetOrgsReposByOrg, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { mock.WriteError( w, http.StatusForbidden, "mock.WriteError rate limit", github.Error{ Message: "github.Error rate limit", }, ) }), ), mock.WithRequestMatch( mock.GetOrgsReposByOrg, []github.Repository{ // ... second page of repos to return }, ), ) ```

"mock.NewMockedHTTPClient" with several "mock.MockBackendOption" arguments

I think it's because the if resp.NextPage == 0 condition always evaluates true from that first mocked page, which is how the go-github example for pagination goes. Is there any way to have that first mocked page give a non-zero value for NextPage? Would I be best served writing up a custom handler with mock.WithRequestMatchHandler wrapped around http.HandlerFunc or is there a more direct way to cover off this test scenario?

Again, thanks very much! 🙏

[migueleliasweb]

Hi @jlucktay , let me have a think about this. There might be two possible ways to do this.

Just a question, do you need to return an error on the first item when you list?

[jlucktay]

Hi @jlucktay , let me have a think about this. There might be two possible ways to do this.

Just a question, do you need to return an error on the first item when you list?

I think the first github.Repositories.ListByOrg call should go through OK and return a page of repos, then the second call should give the error of maxing out the rate limit, then the third call should go through OK and return a second (different) page of repos. I hope that makes sense and that I've understood your question correctly. Thanks for taking a look! 🙏

[migueleliasweb]

The issue seems that this would fundamentally change the underlying behaviour of the mock server as it would have to not just respond a single time with a rate limit (probably a 429), it would have to do that disregards of the mocks you set.

I think the best option is to implement a rate limit in the server level. So there would be no change in the mocks you create. But the server would keep track of the rate of requests.

Also, a rate limiting response might no be treated as an error by github-go. I would have to double check that.

[migueleliasweb]

Actually, Github appears to respond with a 403 :thinking: .

https://docs.github.com/en/rest/overview/resources-in-the-rest-api?apiVersion=2022-11-28#exceeding-the-rate-limit

[jlucktay]

I think the best option is to implement a rate limit in the server level. So there would be no change in the mocks you create. But the server would keep track of the rate of requests.

So, do you think this would require some changes around NewMockedHTTPClient in src/mock/server.go?

Maybe some way to add some rate limit middleware onto the Gorilla router there?

[migueleliasweb]

@jlucktay exactly.

Gorilla allows global middlewares with .Use(). See: https://github.com/gorilla/mux#middleware

What I have in mind is to, in order to keep backwards compatibility, there could be a new option type that you could pass a middlware definition that would hijack requests to specific endpoints and throttle them.

[jlucktay]

Gorilla allows global middlewares with .Use(). See: https://github.com/gorilla/mux#middleware

Yes 👍 I've built around this one before.

What I have in mind is to, in order to keep backwards compatibility, there could be a new option type that you could pass a middlware definition that would hijack requests to specific endpoints and throttle them.

I was hacking around with some ideas already. See what you think of this kind of structure:

func WithRateLimit(window time.Duration, rate int) MockBackendOption {
    // ... create a rate limiter here, using the 'window' and 'rate' arguments

    rateLimitMiddleware := func(next http.Handler) http.Handler {
        return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
            // ... check the rate limit

            if ok {
                next.ServeHTTP(w, r)
            } else {
                // ... write the GitHub rate limit error back to the ResponseWriter somehow
            }
        })
    }

    return func(router *mux.Router) {
        router.Use(rateLimitMiddleware)
    }
}

This layout would not discriminate between endpoints; it would be a blanket limiter.

edit: I think https://pkg.go.dev/golang.org/x/time/rate#Limiter.Allow would be a good fit to use for this.