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

Improve Rails 7.2 compatibility #207

Merged
merged 3 commits into from
Nov 11, 2024

Conversation

jamiecobbett
Copy link
Contributor

On trying to upgrade to Rails 7.2, I was experiencing

[SystemStackError: stack level too deep (SystemStackError)]

On debugging, I found the object triggering this was an instance of ActiveRecord::Transaction:

=> #<ActiveRecord::Transaction:0x00000001211be2a8
 @internal_transaction=
  #<ActiveRecord::ConnectionAdapters::RealTransaction:0x0000000121b1a298
   @callbacks=nil,
   @connection=#<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x0000000000ec40 env_name="development" role=:writing>,
   @dirty=false,
   @instrumenter=
    #<ActiveRecord::ConnectionAdapters::TransactionInstrumenter:0x00000001211be258
     @base_payload=
      {:connection=>#<ActiveRecord::ConnectionAdapters::PostgreSQLAdapter:0x0000000000ec40 env_name="development" role=:writing>,
       :transaction=>#<ActiveRecord::Transaction:0x00000001211be2a8 ...>},
     @handle=nil,
     @payload=nil,
     @started=false>,
   @isolation_level=nil,
   @joinable=true,
   @lazy_enrollment_records=nil,
   @materialized=false,
   @records=
    [#<User REDACTED>],
   @run_commit_callbacks=true,
   @state=#<ActiveRecord::ConnectionAdapters::TransactionState:0x00000001211be348 @children=nil, @state=nil>,
   @user_transaction=#<ActiveRecord::Transaction:0x00000001211be2a8 ...>,
   @written=true>,
 @uuid=nil>

Adding this condition in appears to fix the problem.

I think this fixes #206

This kind of problem seems to be a perennial issue in the gem. I know that super_diff has similar issues - they have a "RecursionGuard", maybe that pattern could be applied to meta_request too?

On trying to upgrade to Rails 7.2, I was experiencing

    [SystemStackError: stack level too deep (SystemStackError)]

On debugging, I found the object triggering this was an instance of
ActiveRecord::Transaction.

Adding this condition in appears to fix the problem.
The Dockerfile was added in dejan#199
but not added to this file, so I don't think it is actually used.
@jamiecobbett jamiecobbett force-pushed the improve-rails-7-2-compatibility branch from 383eadb to d88fabf Compare October 23, 2024 14:17
@jamiecobbett
Copy link
Contributor Author

@dejan let me know if there's anything I can do to make merging this easier - we are so grateful that this gem and extension exist ❤️

@mahmoudimus
Copy link

@dejan and @jamiecobbett this fix works for us. I'm grateful for your contribution and this gem. We will need to relax the railties restriction as well if we want this to work for rails 8.0.

@dejan dejan merged commit 2fb30a5 into dejan:master Nov 11, 2024
@dejan
Copy link
Owner

dejan commented Nov 11, 2024

@jamiecobbett thanks for the contribution!

@jamiecobbett
Copy link
Contributor Author

@dejan no problem 👍

I thought I'd have a go at rails 8, and I opened a PR here cc @mahmoudimus

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

Successfully merging this pull request may close these issues.

Rails 7.2 compatibility issue
3 participants