PR Pagination Fails

[dermidgen]

Related to:

• Report rate-limiting errors clearly #1440
• https://github.com/google-github-actions/release-please-action/issues/506

TLDR

• Release Please PR iterator is not graceful when hitting errors & API rate limits
• Release Please PR iteration exhausts API rate limits very quickly for non enterprise accounts

Here's the details of this bug:

• PR pagination fails for larger repos with >= 200 closed/merged pull requests
  • It looks for PRs against the target branch so after enough releases you hit this on even a small repo
  • It iterates through 200 PRs every single time
• Paging PRs and some resources from the GH APIs may hit occasional 502's reported as "Server Error"
• RP is using the REST pagination tool from octokit for finding merged PRs
• The REST iterator does not use the throttle/retry plugin in octokit (the GraphQL iterator does)
• When searching for merged PRs you get unhandled 502's and die
• When searching for 200 PRs, iterating many commits between releases, etc you hit actual API rate limits pretty quickly
• The longer you wait between releases, the more commits you iterate, the higher the likelihood of failure once you get to PRs

Attempts to mitigate

• Dedicated user & token to isolate API requests (nope)
  • Doesn't help - you hit 502's anyways even if you're not getting "rate limited"
  • Still blasts through soft rate limits pretty quickly, which are not handled gracefully
• Dedicated release branch (nope)
  • Once you merge enough release PRs, you're in the same spot again
  • Sucks if you're trunk-based
• Switch to GraphQL (BETTER!)
  • Modified local sources for Release Please in node_modules to switch to the GraphQL strategy; HELPED!
  • It's graceful! It can handle the 502's AND API soft rate limiting
  • It still runs so many requests you get rate limited pretty quickly

Deeper detail

Switch to using GraphQL and you get:

I locally modified https://github.com/googleapis/release-please/blob/main/src/github.ts#L619 to use pullRequestIteratorWithFiles instead of pullRequestIteratorWithoutFiles. Since pullRequestIteratorWithFiles uses the GraphQL API through octokit - you get fault tolerance and retries. Technically, the REST iterator through octokit should do the same thing; but it doesn't.

Further, octokit violates Github's Docs by sending the application/vnd.github.v3+json accept header - which should be application/vnd.github+json.

Solution Path

Switch to GraphQL iterator for PRs

• Solves failures on soft API limits & 502 "Server Error"

I will likely submit a PR to switch away from the REST iterator for PRs. This is a vastly improved experience, fault tolerant and more verbose. This does NOT resolve the overall issue of Release Please exhausting API request limits through aggressively paging through 200 hundred PRs at multiple stages. It would be nice if that 200 PR lookup limit was configurable.

[dermidgen]

@chingor13 wanted to check and see if you've had a chance to review this issue. Currently, we're working off of forks, which sucks. Do you want a PR with the change I described? It's rather fugly, but it's at least making this a usable tool on our monorepo.

[chingor13]

Sorry for the delay in responding. We would be willing to possibly switch to graphql queries if it's faster or more consistent. We have noticed though that graphql can also be flaky and is not really transparent about what fails.

[dermidgen]

Gosh, bummer. At least the GraphQL SDK has retries & back-offs. It's far more gracious. We haven't seen any bad behavior from GraphQL so far.

[Djiit]

Hey team, how can we help prioritize or implement this ? Thx