Rate limit status race condition

[derrickreimer]

My application deals with multiple instances of Twitter::Client that can run simultaneous requests from different threads, on behalf of different Twitter users. Here's some code to demonstrate:

# Global config
Twitter.configure do |config|
  config.consumer_key = "xxx"
  config.consumer_secret = "yyy"
end

# On thread 1:

begin
  client = Twitter::Client.new(user_a_credentials)
  results = client.home_timeline
rescue Twitter::Error::BadRequest => e
  # Thread 2 may overwrite rate limit data before I can call...
  rate_limit = Twitter::RateLimit.instance
end


# On thread 2:

begin
  client = Twitter::Client.new(user_b_credentials)
  results = client.home_timeline
rescue Twitter::Error::BadRequest => e
  # Thread 1 may overwrite rate limit data before I can call...
  rate_limit = Twitter::RateLimit.instance
end

Unless I'm missing something, there doesn't seem to be a way to retrieve the rate limit status from the error object directly like there used to be. That would solve the race condition.

Any thoughts?

P.S. Major thanks to the maintainers for creating such a solid gem!

[derrickreimer]

Overall, I'm curious about the decision to store rate limit status globally. Perhaps it's more appropriate as a property of Twitter::Client instances, since different instances can have different credentials?

[sferik]

Thanks for the detailed bug report. This is a legitimate issue.

The decision to have a global Twitter::RateLimit object was driven by the fact that users are typically more interested in the current rate limit status than the status at a particular point in time. I hadn't considered the fact that multiple clients using different credentials would clobber each other's rate-limit data.

There are a few possible solutions:

1. Go back the the way things were in version 2, where Twitter::Error objects included rate limit information.
2. Convert the global Twitter::RateLimit singleton class to a non-singleton and initialize one Twitter::RateLimit object per client.
3. Add the rate-limit attributes and accessors to the Twitter::Client class itself.

I'd like to spend a little more time thinking about which of these solutions is best before I begin working on the implementation.

Thanks again for the detailed bug report.

[derrickreimer]

Word. I like the overall idea of what the singleton was trying to accomplish, so option 2 or 3 seem reasonable.

Something also to consider is the way Search API rate limiting is handled. Right now, the #retry_after attribute is set on the rate limit singleton, but that is somewhat ambiguous since it does not apply to the REST API rate limits. Not sure if that warrants a separate object to maintain the state of the search API rate limit.

[sferik]

Yeah, I suppose we could have a Twitter::SearchRateLimit object that inherits from Twitter::RateLimit and extends the #retry_after method.

On second thought, #retry_after and #reset_in represent the same value, so we should probably just write a method than handles both cases and then alias the other name for backwards compatibility.

[sferik]

The final solution turned out to be a combination of 1 and 2. I'm pretty happy with the implementation. This fix will be included in version 3.3, which could ship as early as tomorrow evening. If you'd like to use it before then, you can specify your twitter dependency from a git source in your Gemfile, like so:

gem 'twitter', :git => 'git://github.com/sferik/twitter.git'

[derrickreimer]

Nice work!