Use a max retry after duration for secondary rate limit if specified

[Letiste]

The current logic to pick a retry after duration in case of a secondary rate limit is to first check for the retry after header, and if not found, use the primary rate limit reset time. As the primary rate limit reset every hour, we can be blocked for as long as 1 hour when GitHub returns a secondary rate limit error without a retry after header.

According to GitHub documentation, the rate limit reset header should be used when the remaining allowed requests reached 0. Otherwise, retry in 1 min (doc):

If you receive a rate limit error, you should stop making requests temporarily according to these guidelines:

• If the retry-after response header is present, you should not retry your request until after that many seconds has elapsed.
• If the x-ratelimit-remaining header is 0, you should not make another request until after the time specified by the x-ratelimit-reset header. The x-ratelimit-reset header is in UTC epoch seconds.
• Otherwise, wait for at least one minute before retrying. If your request continues to fail due to a secondary rate limit, wait for an exponentially increasing amount of time between retries, and throw an error after a specific number of retries.

This PR adds an optional max wait time for the secondary rate limit that will, if specified, prevents the client to block up to 1h in some cases.

Given that the GitHub documentation specifies to use the rate limit reset header only if there is no more requests remaining in the quota, I was tempted to make this change by default but it would change the current behaviour. Should we make it by default regardless?

[Letiste]

I'm letting users specify their desired max duration. We could enforce a minimum or a default value here if we want to make sure users are not using this field incorrectly

[Letiste]

I wanted to enforce the max duration in the parseSecondaryRate function but we don't have access to the client there, and it was tedious to pass a parameter downstream so I'm mutating the error here instead. Let me know if you would prefer another solution

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.26%. Comparing base (2b8c7fa) to head (b810986).
Report is 245 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3438      +/-   ##
==========================================
- Coverage   97.72%   92.26%   -5.46%     
==========================================
  Files         153      174      +21     
  Lines       13390    15026    +1636     
==========================================
+ Hits        13085    13864     +779     
- Misses        215     1068     +853     
- Partials       90       94       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[gmlewis]

I am extremely leery of changing the default behavior regarding rate limiting.
If you want to provide an option for your feature, I'm fine with that.

@wmouchere - please comment on this PR because of your recent work on #3411.

[gmlewis]

Thank you, @Letiste.
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

[stevehipwell]

LGTM

[gmlewis]

Thank you, @stevehipwell!
Merging.