Read absent RateLimit headers as unknown, not as 0 and the epoch
What does this MR do and why?
When a response carries no RateLimit-Remaining and RateLimit-Reset headers, the HTTParty adapter turns both into numbers with to_i, so it reports 0 requests remaining and a reset at the epoch. RestAPINetwork then takes every such response for a rate limit about to run out: with --debug it prints Rate limit remaining: 0 (reset at 1970-01-01 00:00:00 +0000) and Rate limit almost exceeded, sleeping for a negative number of seconds after each request, which is the log quoted in the issue, and with --summary it counts a rate-limit wait for each request. It never actually sleeps, because a reset in 1970 is already past, so the damage is the output and the count rather than a stall.
The GraphQL query path already reads missing headers correctly: GraphqlAdapter has rate_limit_remaining and rate_limit_reset_at, which read a missing RateLimit-Remaining as no limit (Float::INFINITY) and a missing RateLimit-Reset as no reset time (nil), and GraphqlNetwork#rate_limit_wait returns early when there is no reset time. The REST adapter and GraphqlAdapter#mutate still coerced both headers, so I brought them in line with that path. The project merges without squashing, so each commit is meant to stand on its own:
- Pin the rate-limit values the HTTParty adapter returns. Specs for all five verbs against a stubbed response carrying both headers. Nothing checked these values before; the specs pass before and after the change.
- Skip the REST rate-limit wait when no reset time is known. The same early return
GraphqlNetwork#rate_limit_waithas. Commit 4 makes a response withRateLimit-Remainingbut noRateLimit-Resetcome back with anilreset, and when that count is below 25 the network would raise on it without this return (NoMethodErrorbuilding the debug message,ArgumentErrorcomparingTime.nowwithnilwhen debug is off). - Move the rate-limit header readers to the base adapter. The two methods move unchanged from
GraphqlAdaptertoBaseAdapter, so the HTTParty adapter can use them too. - Read missing RateLimit headers as unknown in REST responses. All five verbs of the HTTParty adapter read the headers through those methods. Besides the adapter specs, an integration spec runs the engine with
--debug --summaryagainst an instance stub that sends no rate-limit headers and checks that nothing reports the limit as exhausted, announces a sleep or counts a wait. - Read missing RateLimit headers as unknown in GraphQL mutations.
#mutatebuilds its response throughbuild_graphql_response, as the comment above that method already says it does. Thework_item_statusaction and the health status part of theupdateaction send mutations.
A response that does carry the headers is read exactly as before (the specs of commit 1 and the existing GraphQL adapter specs). With the headers absent, the debug line now reads Rate limit remaining: Infinity (reset at ), which is what the GraphQL query path prints for the same response today.
Output before and after, and the checks
The integration scenario of commit 4 (one GET of issues and one POST of a comment, no rate-limit headers), run with --debug --summary on Ruby 3.2.4, keeping the request, rate-limit and summary-count lines.
On master (e0710dec):
[DEBUG] query_api: https://gitlab.com/api/v4/projects/123/issues?per_page=100
[DEBUG] rate_limit_infos: Rate limit remaining: 0 (reset at 1970-01-01 00:00:00 +0000)
[DEBUG] Rate limit almost exceeded, sleeping for -1791798442.4866385 seconds
[DEBUG] post_api: https://gitlab.com/api/v4/projects/123/issues/6/notes?
[DEBUG] rate_limit_infos: Rate limit remaining: 0 (reset at 1970-01-01 00:00:00 +0000)
[DEBUG] Rate limit almost exceeded, sleeping for -1791798442.496981 seconds
Rate limit waits: 2With this branch:
[DEBUG] query_api: https://gitlab.com/api/v4/projects/123/issues?per_page=100
[DEBUG] rate_limit_infos: Rate limit remaining: Infinity (reset at )
[DEBUG] post_api: https://gitlab.com/api/v4/projects/123/issues/6/notes?
[DEBUG] rate_limit_infos: Rate limit remaining: Infinity (reset at )
Rate limit waits: 0Each fix commit's new specs, run against that commit's tree with its lib/ changes put back to the parent's version:
- commit 2: 1 of 34 examples in
rest_api_network_spec.rbfails, the new one (undefined method '-' for nil:NilClass). - commit 4: 13 of 31 examples in
httparty_adapter_spec.rband the new integration spec fail, the 13 new ones (the adapter returnsratelimit_remaining: 0andratelimit_reset_at: 1970-01-01). - commit 5: 1 of 19 examples in
graphql_adapter_spec.rbfails, the new one (same values from#mutate).
On every commit, Ruby 3.2.4: bundle exec rake spec passes (1044, 1045, 1045, 1058 and 1059 examples, 0 failures) and bundle exec rubocop --parallel reports no offenses. On the last commit, the suite also passes on Ruby 3.0.7, 3.1.7 and 3.3.12 (1059 examples, 0 failures).
Closes #385 (closed)