if ratelimited during pagination, silently truncated results

[edmund-huber]

It looks like https://github.com/octokit/octokit.rb/blob/master/lib/octokit/client.rb#L205 if you get a stroke of bad luck and hit your ratelimit while auto_paginate is doing its thing, you'll get an incomplete set of results and nothing will alert you to that.

[pengwynn]

Indeed. Since it returns a bare array, there's not a great way to surface that. Perhaps you could check client.rate_limit.remaining > 0 to know if you've got complete results?

[edmund-huber]

I would argue that if auto_paginate is on, and ratelimit is hit, you really want to know if your results are incomplete, and it would be reasonable for the code to raise an exception. I realize this is a major API change, but for the sake of discussion..

[joeyw]

When using autopaginate, it is kind of implied that it is sort of a long running task and could take some time for all the results to be returned. Taking that into consideration, maybe it is reasonable to have it wait until more requests are available to continue finishing the requests. We could also add a timeout on autopaginate to raise an error if it took too long with the possibility of customizing the timeout.

[pengwynn]

@joeyw I'd be more inclined to raise and have the partial results available on the exception than try to do any auto-retry because methods like GET /repositories would never return.

[brynjargr]

I was dealing with this same issue where I was getting incomplete results and it wasn't clear why. Eventually I figured out that it was because I was hitting the rate limit while using auto_paginate. I eventually figured out that I could handle this by adding a sleep that would wait until the rate limit is reset before making more requests. I accomplished this by doing something like this:

def rate_limit_handling(client = Octokit::Client.new, floor = 0)
  while client.rate_limit.remaining <= floor do
    sleep client.rate_limit.resets_in
  end
end

commits = client.commits(repository.full_name) do |data, last_response|
  data.concat(last_response.data) if last_response.data.is_a?(Array)
  rate_limit_handling(client, 10)
end

I think this could easily be supported directly in Octokit by adding an option like auto_paginate_rate_limit_waiting or something like that and if that option is enabled then this sleep would be triggered any time the rate limit is hit while auto_paginate is also enabled. I would be open to creating a pull request making that change if others think that would be valuable. Otherwise I can keep using my workaround and doing my own sleeping in the block that I pass in.

[github-actions[bot]]

👋 Hey Friends, this issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please add the Status: Pinned label if you feel that this issue needs to remain open/active. Thank you for your contributions and help in keeping things tidy!