Tests fail on master - rspec ./spec/paper_trail/request_spec.rb:120

[professor]

Thank you for your contribution!

Due to limited volunteers, issues that do not follow these instructions will be
closed without comment.

Check the following boxes:

• This is not a usage question, this is a bug report
• This bug can be reproduced with the script I provide below
• This bug can be reproduced in the latest release of the paper_trail gem

Due to limited volunteers, we cannot answer usage questions. Please ask such
questions on StackOverflow.

Bug reports must use the following template:

git co master
DB=sqlite bundle exec appraisal rails-7.1 rspec ./spec/paper_trail/request_spec.rb:120

  1) PaperTrail::Request.with with a block given with all allowed options sets options only for the current thread
     Failure/Error: Thread.new { expect(described_class.whodunnit).to be_nil }.join

       expected: nil
            got: "foo"

ruby version	rails version	status	RequestStore.scope
3.0.1	7.1	green
3.1.3	7.1	green
3.1.5	7.1	green	Thread
3.2.0	7.1	red	Fiber
3.2.4	7.1	red
3.3.1	7.1	red

There are three thread features in Ruby 3.2 (Dec 24, 2022)

• TracePoint for block default to trace the current thread
• Thread::Queue: timeouts for pop and push
• Thread.each_caller_location

Diff between 3.2.0 and 3.1.3

Current Theory

Looks like we are getting a shared or duplicated request_store:

Ruby 3.1.x
described_class:
{:enabled=>true, :whodunnit=>"foo", :controller_info=>{}, :enabled_for_Widget=>false}
Thread.new:
{:enabled=>true}

Ruby 3.2.x
described_class:
{:enabled=>true, :whodunnit=>"foo", :controller_info=>{}, 
:enabled_for_Widget=>false}
Thread.new:
{:enabled=>true, :whodunnit=>"foo", :controller_info=>{}, :enabled_for_Widget=>false}

Papertrail is on the latest version of RequestStore.

[professor]

I'm curious how "bad" is it if PaperTrail is leaking whodunnit between threads.

[professor]

new(blocking: false, storage: true) 
If the storage is unspecified, the default is to inherit a copy of the 
storage from the current fiber. This is the same as specifying 
storage: true.

This seems like it could be a problem in using RequestStore and Fibers. It's up to puma, thin, sidekiq to make sure that the storage is set to nil, otherwise the RequestStore will have cross Fiber pollution.

I've created this issue: steveklabnik/request_store#98

[professor]

Once this is merged: steveklabnik/request_store#99, I'm hoping that paper trail master will be green again!

[professor]

With the new release of request store 1.7.0, paper_trial is now green on master!