bugsnag / bugsnag-api-ruby

BugSnag API toolkit for Ruby
Other
21 stars 14 forks source link

auto_paginate does not gracefully handle being rate limited. #40

Open BobbyMcWho opened 2 years ago

BobbyMcWho commented 2 years ago

We were writing a script to pull error events from the api, to parse some associated information so that we could backfill some data that needed fixed. Upon using the ruby gem, all was going well until I realized that Bugsnag::Api.error_events does not gracefully handle being rate-limited, which ultimately came down to the paginate method.

Our solution was to override that method on Bugsnag::Api::Client, but it's something that I would expect the client library to handle, or at least call out in the docs.

module Bugsnag::Client::GracefulRateLimit
  def paginate(url, options = {}, &block)
    opts = parse_query_and_convenience_headers(options.dup)
    if configuration.auto_paginate || configuration.per_page
      opts[:query][:per_page] ||=  configuration.per_page || (configuration.auto_paginate ? 100 : nil)
    end

    data = request(:get, url, opts)

    if configuration.auto_paginate
      while @last_response.rels[:next]
        begin
          @last_response = @last_response.rels[:next].get
        rescue Bugsnag::Api::RateLimitExceeded => e
          sleep 60 and redo
        else
          if block_given?
            yield(data, @last_response)
          else
            data.concat(@last_response.data) if @last_response.data.is_a?(Array)
          end
        end
      end
    end

    data
  end
end

Bugsnag::Api::Client.prepend Bugsnag::Client::GracefulRateLimit
luke-belton commented 2 years ago

Hi @BobbyMcWho - thanks for raising this and sharing your solution. There's already an item on our product roadmap to look at this, however I don't have an ETA for when that work will be completed. I'll keep this thread updated with progress 👍

javierjulio commented 1 year ago

Just to point out, that the first request (outside the auto_paginate conditional) also needs to handle the rate limit exceeded error.

This is the edit I used in an irb console since the original module didn't work for me.

Bugsnag::Api::Client.class_eval do
  def paginate(url, options = {}, &block)
    opts = parse_query_and_convenience_headers(options.dup)
    if configuration.auto_paginate || configuration.per_page
      opts[:query][:per_page] ||=  configuration.per_page || (configuration.auto_paginate ? 100 : nil)
    end

    begin
      data = request(:get, url, opts)
    rescue Bugsnag::Api::RateLimitExceeded => e
      sleep 60 and retry
    end

    if configuration.auto_paginate
      while @last_response.rels[:next]
        begin
          @last_response = @last_response.rels[:next].get
        rescue Bugsnag::Api::RateLimitExceeded => e
          sleep 60 and redo
        else
          if block_given?
            yield(data, @last_response)
          else
            data.concat(@last_response.data) if @last_response.data.is_a?(Array)
          end
        end
      end
    end

    data
  end
end

I'll make a separate issue to note adding the auto_paginate configuration to the README. I started to build this out before noticing this issue. It's immensely helpful to have this auto paginate feature for debugging (thank you!) as we often need to collect the data in our event tabs.

In addition to this it would help to have some logging or some optional hook here to log output when the paginate method handles the rate limit exceeded error and is sleeping. Perhaps an optional block that is set on the configuration object that is called in the rescue block?

Bugsnag::Api.configure do |config|
  # ...
  config.auto_paginate = true
  config.on_auto_paginate_rate_limit = -> { puts "Rate Limit Exceeded. Sleeping for 60 seconds." }
end

# in client object
def paginate(url, options = {}, &block)
  begin
    data = request(:get, url, opts)
  rescue Bugsnag::Api::RateLimitExceeded => e
    configuration.on_auto_paginate_rate_limit.call if config.on_auto_paginate_rate_limit
    sleep 60 and retry
  end
end
yousif-bugsnag commented 1 year ago

Hi @javierjulio , thanks for sharing your edit! I've noted this and your logging suggestion on our internal ticket. I still don't have an ETA for when this will be prioritised but we will keep this thread updated with any progress.

javierjulio commented 1 year ago

@yousif-bugsnag thank you! No worries. I figured out afterwards it's just easier to work around this by overriding the Sawyer client instead.

module CallWithRetryOnRateLimitExceeded
  def call(method, url, data = nil, options = nil)
    super
  rescue Bugsnag::Api::RateLimitExceeded => error
    puts "RateLimitExceeded: Retrying in 60 seconds"
    sleep 60 and retry
  end
end

Sawyer::Agent.send :prepend, CallWithRetryOnRateLimitExceeded
rtymchyk commented 9 months ago

@yousif-bugsnag thank you! No worries. I figured out afterwards it's just easier to work around this by overriding the Sawyer client instead.

module CallWithRetryOnRateLimitExceeded
  def call(method, url, data = nil, options = nil)
    super
  rescue Bugsnag::Api::RateLimitExceeded => error
    puts "RateLimitExceeded: Retrying in 60 seconds"
    sleep 60 and retry
  end
end

Sawyer::Agent.send :prepend, CallWithRetryOnRateLimitExceeded

Just an FYI you can use the response header to determine how long to sleep (instead of the fixed 60 seconds)

retry_after = error.instance_variable_get(:@response)[:response_headers]["Retry-After"]