Regression from 8.8.0 to 8.8.1 with TestData module

[javierjulio]

Describe the bug

We updated from 8.8.0 to 8.8.1 and started to run into a regression with the TestData module. Our test suite would often fail with a flaky test when running 8.8.1. For now we've downgraded to 8.8.0.

We are running Ruby 3.3.5 which reviewing the change in 8.8.1 seemed logical to update as it's just a bug fix although we aren't aware of the issue affecting us. It was surprising to encounter a regression with TestData and figured it's worth to report it.

This is a sample error and stacktrace we encounter with 8.8.1 in our test suite:

     NoMethodError:
       undefined method `namespace' for an instance of String
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/impl/data_store.rb:49:in `eql?'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/expiring_cache.rb:51:in `delete'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/expiring_cache.rb:51:in `block in delete'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/expiring_cache.rb:48:in `synchronize'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/expiring_cache.rb:48:in `delete'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/integrations/util/store_wrapper.rb:106:in `upsert'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/impl/integrations/redis_impl.rb:86:in `upsert'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/impl/store_client_wrapper.rb:43:in `block in upsert'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/impl/store_client_wrapper.rb:70:in `wrapper'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/impl/store_client_wrapper.rb:43:in `upsert'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/impl/integrations/test_data/test_data_source.rb:34:in `upsert'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/integrations/test_data.rb:192:in `block (2 levels) in update_item'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/integrations/test_data.rb:191:in `each'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/integrations/test_data.rb:191:in `block in update_item'
     # /usr/local/bundle/ruby/3.3.0/gems/concurrent-ruby-1.3.4/lib/concurrent-ruby/concurrent/atomic/read_write_lock.rb:79:in `with_read_lock'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/integrations/test_data.rb:190:in `update_item'
     # /usr/local/bundle/ruby/3.3.0/gems/launchdarkly-server-sdk-8.8.1/lib/ldclient-rb/integrations/test_data.rb:127:in `update'

Was there meant to be a change with the TestData integration in the 8.8.1 release? I can provide the module set up and the set-feature-flag test helper method we use for that TestData integration if it helps. We run our rspec tests in parallel using the turbo_tests gem.

I will try to create a reproduction and follow up with that. I figure it would help to share this if any hint with the error and stacktrace.

SDK version
8.8.1

Language version, developer tools
Ruby 3.3.5

OS/platform
Linux Alpine

Additional context
Add any other context about the problem here.

[keelerm84]

@javierjulio sorry to hear you're experiencing this issue, but thank you for bringing it to our attention.

I will try to create a reproduction and follow up with that.

Have you had any luck with a reproduction case yet?

I can provide the module set up and the set-feature-flag test helper method we use for that TestData integration if it helps

Yes please. Any additional insight into how you are interacting with the store will be useful. Thank you.

[keelerm84]

Given the stacktrace you provided, I took a stab at what I think is likely a fix. Can you test against the branch in this PR, and let me know if that resolves your problem?

[javierjulio]

@keelerm84 I'm trying to use that branch now but I'm running into a bundle install error since for some reason the gemspec includes a require "rake" line. Can you please remove that temporarily? There shouldn't be a need to require rake in the gemspec.

I've tried various things to get bundle install to work in Docker, even adding gem "rake" to our Gemfile but I keep running into this error when installing the gem from this repository using your feature branch.

16.66 [!] There was an error while loading `launchdarkly-server-sdk.gemspec`: cannot load such file -- rake. Bundler cannot continue.
16.66 
16.66  #  from /usr/local/bundle/ruby/3.3.0/bundler/gems/ruby-server-sdk-c7080edbad3a/launchdarkly-server-sdk.gemspec:6
16.66  #  -------------------------------------------
16.66  #  require "ldclient-rb/version"
16.66  >  require "rake"
16.66  #  
16.66  #  -------------------------------------------

[javierjulio]

@keelerm84 I forked the repository to install the gem using the fork with two updates. I could not get the gem to install from source with its current setup. I made 2 changes that if you like I can contribute in a PR so the gem can be installed from source in the future.

Delete the following line as it should be unnecessary, otherwise it fails with a bundler error:

ruby-server-sdk/launchdarkly-server-sdk.gemspec

Line 6 in 58275c3

require "rake"

I've shared the error in the previous comment.

Update this line to use Dir instead otherwise it fails with a bundler error:

ruby-server-sdk/launchdarkly-server-sdk.gemspec

Line 19 in 58275c3

spec.files = FileList["lib/**/*", "README.md", "LICENSE.txt"]

[!] There was an error while loading `launchdarkly-server-sdk.gemspec`: uninitialized constant FileList. Bundler cannot continue.

 #  from /Users/j.julio/.rbenv/versions/3.3.5/lib/ruby/gems/3.3.0/bundler/gems/launchdarkly-ruby-server-sdk-b8220416fe3f/launchdarkly-server-sdk.gemspec:18
 #  -------------------------------------------
 #  
 >    spec.files         = FileList["lib/**/*", "README.md", "LICENSE.txt"]
 #    spec.executables   = spec.files.grep(%r{^bin/}) { |f| File.basename(f) }
 #  -------------------------------------------

I don't think I've ever seen FileList used before in a gemspec. If it's not using git, it's using Dir which you can see in popular gems zeitwerk, any of Rails gems, etc. This should be safe to swap.

[javierjulio]

@keelerm84 the fix seems to be working great, thank you! I've run our test suite in CI over 30 times now using a modified feature branch in a fork with the earlier mentioned changes and it works. I no longer encounter that flaky error. We were encountering it on every other test run in CI. Before testing your fix, I confirmed that 8.8.1 would fail again and it did on the first test run in CI. With your fix though it's been all green so thank you.

[keelerm84]

@javierjulio that's fantastic. I'm glad to hear the fix is working for you. I'm still bothered by why this issue was occurring in the first place. Were you able to share a reproduction case prior to the fix I can use to further investigate the underlying issue?

I made 2 changes that if you like I can contribute in a PR so the gem can be installed from source in the future.

Happy to review any changes you want to commit back.

[javierjulio]

@keelerm84 I'm sorry, I don't have a reproduction yet due to what is involved. I'm trying to figure out how to scale it down and replicate. I figured it was still worth reporting considering the level of changes to accommodate a Ruby bug, so thank you. Since it involves some setup, I'd be happy to share what I'm doing with you and see if I can help create a reproduction. Would it be ok if I email you? In the meantime, I'll open a PR with the changes to allow the gem to be installed from source.

[keelerm84]

I went ahead and released the fix as v8.8.2 so you can use an official release for now.

And yes, please feel free to email me directly ([email protected]). I would love to understand the source of this if I can. Thank you so much!

[javierjulio]

@keelerm84 awesome, thank you! I will send you an email now.