Describe the bug
If a project has many PRs with skip release labels then auto quickly starts triggering GitHub's search API rate limit. Even with Octokit's rate limiting protection enabled we're still seeing cases where the rate limits are being exceeded.
See this log as an example: https://circleci.com/gh/artsy/renovate-config/2739
To Reproduce
A repo must have many >20ish unpublished PRs to trigger this effect.
Expected behavior
Process should gracefully recover if rate limits are exceeded. Ideally the search endpoint is called less individual times.
Additional Context
The particular endpoint being hit is
GET https://api.github.com/search/issues?q=repo:artsy/renovate-config 2868a054c07dc4fd36d53a3df1bcc11cbf45e9f4
Where the thing at the end is some commit hash.
I believe this is where the search call is happening:
https://github.com/intuit/auto/blob/221b2931c93859711f8bed5a20f9b572a9ae3054/src/release.ts#L200
I did a test and it looks like you can batch all these hashes together into a single (or at least fewer) search calls.
Try this:
https://api.github.com/search/issues?q=repo%3Aartsy%2Frenovate-config%209b43a6dc2d27967c72914247994afbf7d56167f6%20a76005e3705a4e46d7b7ea6abe544937f5e4e9e4%205b63a5bb1294c15dd2423fce74c119638b0c2716%204dc11106f467c2631374a384db4ef46b4867251e
Edit: Looks like the search itself has a length limit of 256 characters so that'll more likely determine the limit.
I've played with this for a few hours now and I haven't really come up with any good answers. Batching looks like it's somewhat feasible, but I don't know that we can guarantee what results go with what hash so that might be off the table.
Rate limiting helps, but generally it falls through because other things can be calling the search api on that again (other repos running auto for example) which could push it over unintentionally.
We could add a manual try-catch block around the search call and do a 60 second setTimeout between retries. That way if it falls through we just try again after waiting a bit.
I mean, that's a bit redundant on top of the throttling plugin, but if it prevents things from failing 🤷♂️ .
Side note: I was _finally_ able to get one of our failing repos to release. When no other release traffic is happening it seems the throttling plugin works. I'm a little more convinced that some manual intervention would help make sure the process actually finishes... even if it takes _much_ longer.
This search should only be triggered if we can't get a PR from the commit message. Usually on merge it is added. Maybe the PRs are being rebased and have no commit message to parse.
If we can't find a solution we could just add a config option to skip this code.
The code is trying to find a PR for any commit that isn't included included in another PR. It will either get the labels from the associated PR or add the 'push-to-master' label.
I hope that generally just adding some better error handling/retry logic around this one area would likely be sufficient. It's unfortunate that there's such as small rate limit.
yeah hopefully. the config option would kinda suck since the release would be missing a bit of information. This problem really only arises if the commits were rebased in a certain way though.
🙏 ing to the gods we can up the rate limit though https://twitter.com/HipsterSmoothie/status/1121912432480227328
I wonder if we'd be able to contact GitHub support to get them to up the rate limit on auto's search integration. Pretty sure there's already a header set via Octokit. They might not do that in a one-off way though 🤷♂ .
In the mean time I'll work on some extra retry logic.
What would be really awesome is a new API endpoint to match an arbitrary commit to a PR. Then we wouldn't have to search at all
It would indeed. I fear it's unlikely we'll see something like that anytime soon.
Maybe we could wrangle the graphql api to search all the commit hashes in one request. I havent played around with it though. Or ever used graphql
update: I really don't know graphql lol
I think it might work
{
first: search(query: "repo:artsy/renovate-config 9b43a6dc2d27967c72914247994afbf7d56167f6", type: ISSUE, first: 1) {
edges {
node {
... on PullRequest {
number
state
labels(first: 10) {
edges {
node {
name
}
}
}
}
}
}
}
second: search(query: "repo:artsy/renovate-config 87451a9d010500823df3c59f77e559c28c01ab9f", type: ISSUE, first: 1) {
edges {
node {
... on PullRequest {
number
state
labels(first: 10) {
edges {
node {
name
}
}
}
}
}
}
}
}
Ohhh, that's promising.
Oh, yeah! I think this will totally work.
Use this to see the current rate limit status of the query:
{
rateLimit {
limit
cost
remaining
resetAt
}
}
You wanna PR it or do you want me to?
:rocket: Issue was released in v4.9.2 :rocket: