Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveSupport::CurrentAttributes Compatibility #1341

Closed
JonathanFrias opened this issue Apr 27, 2024 · 5 comments
Closed

ActiveSupport::CurrentAttributes Compatibility #1341

JonathanFrias opened this issue Apr 27, 2024 · 5 comments

Comments

@JonathanFrias
Copy link
Contributor

I have an interesting bug report for you.

Let's say you use ActiveSupport::CurrentAttributes:

class Current < ActiveSupport::CurrentAttributes
  attribute :user
end

And you want to use it with GoodJob:

class MyGoodJob < ApplicationJob
  def perform(user_id)
    Current.user = User.find(id)
    # .. some work
    GoodJob::Batch.enqueue do
       MyGoodJob.perform_later(Current.user.manager_id) # raises error
       # Current.user # => nil here
    end
  end
end

I looked into this and the reason is the good job calls the function Rails.application.executor.wrap twice in this code which does work to ensure isolated execution state. Part of that work includes calling reset on all descendants of ActiveSupport::CurrentAttributes. This resets all of the attributes.

I'm not sure if this is an issue or expected behavior. I think it should at least be documented. This is the workaround I've come up with so far

class Current < ActiveSupport::CurrentAttributes
  attribute :user, :_locked_attributes
  def locked_attributes
    Current._locked_attributes = true
    yield
    Current._locked_attributes = nil
  end

  def reset
    return if Current._locked_attributes
    super
  end
end

Thanks for your again for your work on this project! @bensheldon

@Earlopain
Copy link
Collaborator

This is a bug in rails: rails/rails#49227, there's also a bit more info at #1320 and its linked issues/prs/blogpost.

I agree it's very unexpected.

@bensheldon
Copy link
Owner

That's the correct issue @Earlopain that explains the problem. Thank you!

In this example, it's somewhat worse because Batch.enqueue also has its own Rails.application.executor.wrap because it takes an advisory lock when enqueuing and thus requires a wrapping executor to ensure the connection is persisted:

Rails.application.executor.wrap do
record.with_advisory_lock(function: "pg_advisory_lock") do

Is this example code you're running in a test or console and is there an active Rails executor? (puts Rails.application.executor.active?). The underlying solution is to always have a wrapping Rails executor, which usually is the case in a real request or Active Job perform.

Rails now wraps all test cases with an executor (though I think you must opt in). RSpec has not done so yet, though there are workarounds: rspec/rspec-rails#2713

Background:

@Earlopain
Copy link
Collaborator

Ah, right. Yeah, I wasn't thinking about potential executor things going on under the hood for batches.

Rails made the executor around test cases the default for 7.0. So, if you do load_defaults 7.0 or higher then that will be the case: https://guides.rubyonrails.org/configuring.html#config-active-support-executor-around-test-case

@JonathanFrias
Copy link
Contributor Author

JonathanFrias commented Apr 29, 2024

Okay that makes sense. Yes, I was running it directly in the console to test the job. Perhaps console sessions should also be wrapped with Rails.application.executor.wrap.

Consider the example:

$ rails console
Loading development environment (Rails 7.0.8.1)

# Fail
Current.user = User.find(id)
MyGoodJob.new.perform(...)

# Success
Rails.application.executor.wrap do
  Current.user = User.find(id)
  MyGoodJob.new.perform(...) 
end

@JonathanFrias
Copy link
Contributor Author

Closing this as I'm finding many other discussions about this elsewhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

No branches or pull requests

3 participants