From c9bd9ac5f9a578e6c62e3136e14a672a6f03611e Mon Sep 17 00:00:00 2001 From: Valentine Frolov Date: Tue, 29 Sep 2026 10:29:05 +0300 Subject: [PATCH] fix(#1224): take the trace out under the lock and ask for the quota outside it --- lib/fbe/octo.rb | 62 +++++++++++++++++++++---------------------- test/fbe/test_octo.rb | 34 ++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 32 deletions(-) diff --git a/lib/fbe/octo.rb b/lib/fbe/octo.rb index ee0ce178..00d17487 100644 --- a/lib/fbe/octo.rb +++ b/lib/fbe/octo.rb @@ -145,38 +145,36 @@ def Fbe.octo(options: $options, global: $global, loog: $loog) # rubocop:disable o = decoor(o, loog:, trace:, limits:, mutex:) do # rubocop:disable Metrics/BlockLength def print_trace!(all: false, max: 5) - @mutex.synchronize do - if @trace.empty? - @loog.debug('GitHub API trace is empty') - else - shown = @trace.select { |e| e[:duration] > 0.05 || all } - grouped = - shown.group_by do |entry| - uri = URI.parse(entry[:url]) - query = uri.query - query = "?#{query.ellipsized(40)}" if query - "#{uri.scheme}://#{uri.host}#{uri.path}#{query}" - end - message = grouped - .sort_by { |_path, entries| -entries.count } - .map do |path, entries| - [ - ' ', - path.gsub(%r{^https://api.github.com/}, '/'), - ': ', - entries.count, - " (#{entries.sum { |e| e[:duration] }.seconds})" - ].join - end - .take(max) - .join("\n") - @loog.info( - "GitHub API trace (#{grouped.count} URLs vs #{shown.count} requests, " \ - "#{@trace.count - shown.count} fast ones skipped, " \ - "#{@origin.rate_limit!.remaining} quota left):\n#{message}" - ) - @trace.clear - end + trace = @mutex.synchronize { @trace.slice!(0..) } + if trace.empty? + @loog.debug('GitHub API trace is empty') + else + shown = trace.select { |e| e[:duration] > 0.05 || all } + grouped = + shown.group_by do |entry| + uri = URI.parse(entry[:url]) + query = uri.query + query = "?#{query.ellipsized(40)}" if query + "#{uri.scheme}://#{uri.host}#{uri.path}#{query}" + end + message = grouped + .sort_by { |_path, entries| -entries.count } + .map do |path, entries| + [ + ' ', + path.gsub(%r{^https://api.github.com/}, '/'), + ': ', + entries.count, + " (#{entries.sum { |e| e[:duration] }.seconds})" + ].join + end + .take(max) + .join("\n") + @loog.info( + "GitHub API trace (#{grouped.count} URLs vs #{shown.count} requests, " \ + "#{trace.count - shown.count} fast ones skipped, " \ + "#{@origin.rate_limit!.remaining} quota left):\n#{message}" + ) end end # rubocop:disable Elegant/GoodMethodName, Style/OptionalBooleanParameter diff --git a/test/fbe/test_octo.rb b/test/fbe/test_octo.rb index 9efa8b8a..6b70d73d 100644 --- a/test/fbe/test_octo.rb +++ b/test/fbe/test_octo.rb @@ -1120,6 +1120,40 @@ def test_octo_not_trace_cached_requests refute_match('/repos/zerocracy/baza.rb: 25', output) end + def test_prints_trace_after_hundred_requests + WebMock.disable_net_connect! + seed = Random.new_seed + total = Random.new(seed).rand(100..130) + stub_request(:get, 'https://api.github.com/rate_limit').to_return( + body: '{"rate":{"remaining":4000}}', headers: { 'X-RateLimit-Remaining' => '4000' } + ) + stub_request(:get, %r{https://api.github.com/repos/foo/bar/issues/\d+}).to_return( + body: '{"number":1}', headers: { 'X-RateLimit-Remaining' => '4000' } + ) + loog = Loog::Buffer.new + octo = Fbe.octo(loog:, global: {}, options: Judges::Options.new) + total.times { |i| octo.issue('foo/bar', i + 1) } + octo.print_trace!(all: true) + assert_includes(loog.to_s, 'GitHub API trace (', "trace is not printed after #{total} requests, seed #{seed}") + end + + def test_prints_trace_twice_past_two_quota_refreshes + WebMock.disable_net_connect! + stub_request(:get, 'https://api.github.com/rate_limit').to_return( + body: '{"rate":{"remaining":4000}}', headers: { 'X-RateLimit-Remaining' => '4000' } + ) + stub_request(:get, %r{https://api.github.com/repos/foo/bar/issues/\d+}).to_return( + body: '{"number":1}', headers: { 'X-RateLimit-Remaining' => '4000' } + ) + loog = Loog::Buffer.new + octo = Fbe.octo(loog:, global: {}, options: Judges::Options.new) + 2.times do |r| + 120.times { |i| octo.issue('foo/bar', (r * 1000) + i + 1) } + octo.print_trace!(all: true) + end + assert_equal(2, loog.to_s.scan('GitHub API trace (').size, 'trace is not printed after each of two refreshes') + end + def test_trace_gets_cleared_after_print WebMock.disable_net_connect! stub_request(:get, 'https://api.github.com/rate_limit').to_return(