diff --git a/lib/fbe/middleware/rate_limit.rb b/lib/fbe/middleware/rate_limit.rb index 849b1f90..d66a9bc0 100644 --- a/lib/fbe/middleware/rate_limit.rb +++ b/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) diff --git a/test/fbe/middleware/test_rate_limit.rb b/test/fbe/middleware/test_rate_limit.rb index eb932c1d..611ac293 100644 --- a/test/fbe/middleware/test_rate_limit.rb +++ b/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: 'Bad gateway', 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) diff --git a/test/fbe/test_octo.rb b/test/fbe/test_octo.rb index 9efa8b8a..fea728fd 100644 --- a/test/fbe/test_octo.rb +++ b/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: 'Bad gateway', 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(