Cache and parse only a successful response of /rate_limit

lib/fbe/middleware/rate_limit.rb

@@ -77,6 +77,7 @@ def handle_rate_limit_request(env)
     stale = @lock.synchronize { @cached.nil? || @counter >= 100 }
     return @lock.synchronize { Faraday::Response.new(response_env(env, @cached)) } unless stale
     response = @app.call(env)
+    return response unless response.success?
     @lock.synchronize do
       @cached = response
       @remaining = extract_remaining_count(response)

test/fbe/middleware/test_rate_limit.rb

@@ -495,6 +495,45 @@ def test_cached_body_is_not_leaked_to_callers
     assert_equal(30, second.body['resources']['search']['remaining'])
   end
 
+  def test_asks_again_after_server_error
+    seed = Random.new_seed
+    status = Random.new(seed).rand(500..599)
+    stub_request(:get, 'https://api.github.com/rate_limit')
+      .to_return(status:, body: '{}', headers: { 'Content-Type' => 'application/json' })
+      .then
+      .to_return(status: 200, body: '{"rate":{"remaining":4000}}', headers: { 'Content-Type' => 'application/json' })
+    conn = create_connection
+    conn.get('/rate_limit')
+    assert_equal(200, conn.get('/rate_limit').status, "error #{status} is served from cache, seed #{seed}")
+  end
+
+  def test_asks_again_after_client_error
+    seed = Random.new_seed
+    status = [401, 403, 404, 422].sample(random: Random.new(seed))
+    stub_request(:get, 'https://api.github.com/rate_limit')
+      .to_return(status:, body: '{"message":"Ω"}', headers: { 'Content-Type' => 'application/json' })
+      .then
+      .to_return(status: 200, body: '{"rate":{"remaining":4000}}', headers: { 'Content-Type' => 'application/json' })
+    conn = create_connection
+    conn.get('/rate_limit')
+    assert_equal(200, conn.get('/rate_limit').status, "error #{status} is served from cache, seed #{seed}")
+  end
+
+  def test_passes_on_error_with_html_body
+    stub_request(:get, 'https://api.github.com/rate_limit')
+      .to_return(status: 502, body: '<html>Bad gateway</html>', headers: { 'Content-Type' => 'text/html' })
+    assert_equal(502, create_connection.get('/rate_limit').status, 'error with html body is not passed on')
+  end
+
+  def test_leaves_remaining_unknown_after_error
+    stub_request(:get, 'https://api.github.com/rate_limit').to_return(
+      status: 503, body: '{"rate":{"remaining":0}}', headers: { 'Content-Type' => 'application/json' }
+    )
+    tracker = {}
+    create_connection(tracker).get('/rate_limit')
+    assert_nil(tracker[:rate_limit].remaining, 'remaining is taken from an error response')
+  end
+
   private
 
   def create_connection(tracker = nil)

test/fbe/test_octo.rb

@@ -1120,6 +1120,22 @@ def test_octo_not_trace_cached_requests
     refute_match('/repos/zerocracy/baza.rb: 25', output)
   end
 
+  def test_reaches_github_after_one_failed_quota_request
+    WebMock.disable_net_connect!
+    stub_request(:get, 'https://api.github.com/rate_limit')
+      .to_return(status: 502, body: '<html>Bad gateway</html>', headers: { 'Content-Type' => 'text/html' })
+      .then
+      .to_return(
+        status: 200, body: '{"rate":{"remaining":4000}}',
+        headers: { 'Content-Type' => 'application/json', 'X-RateLimit-Remaining' => '4000' }
+      )
+    stub_request(:get, 'https://api.github.com/repos/foo/bar').to_return(
+      body: '{"id":42}', headers: { 'Content-Type' => 'application/json', 'X-RateLimit-Remaining' => '3999' }
+    )
+    o = Fbe.octo(loog: Loog::NULL, global: {}, options: Judges::Options.new({ 'github_token' => 'fake-token' }))
+    assert_equal(42, o.repository('foo/bar')[:id], 'one failed quota request blocks the client')
+  end
+
   def test_trace_gets_cleared_after_print
     WebMock.disable_net_connect!
     stub_request(:get, 'https://api.github.com/rate_limit').to_return(