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

autorestore: can't modify frozen Hash #482

Closed
bkeepers opened this issue Feb 13, 2024 · 7 comments · Fixed by #483 or #504
Closed

autorestore: can't modify frozen Hash #482

bkeepers opened this issue Feb 13, 2024 · 7 comments · Fixed by #483 or #504
Labels

Comments

@bkeepers
Copy link
Owner

bkeepers commented Feb 13, 2024

After perusing dependabot updates to dotenv 3.0, I'm seeing a handful of builds breaking with this error:

can't modify frozen Hash: {"SOME_ENV_VAR"=>"true"}
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv.rb:87:in `replace'
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv.rb:87:in `block in restore'
/app/vendor/bundle/ruby/3.2.0/gems/activesupport-7.1.3/lib/active_support/notifications.rb:206:in `block in instrument'
/app/vendor/bundle/ruby/3.2.0/gems/activesupport-7.1.3/lib/active_support/notifications/instrumenter.rb:58:in `instrument'
/app/vendor/bundle/ruby/3.2.0/gems/activesupport-7.1.3/lib/active_support/notifications.rb:206:in `instrument'
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv.rb:132:in `instrument'
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv.rb:87:in `restore'
/app/vendor/bundle/ruby/3.2.0/gems/dotenv-3.0.0/lib/dotenv/autorestore.rb:9:in `block (2 levels) in <top (required)>'

This is caused by autorestore and is happening because ENV is somehow frozen, but the head scratcher is that ENV.freeze raises cannot freeze ENV (TypeError).

Examples:

@bkeepers bkeepers added the bug label Feb 13, 2024
@dbackeus
Copy link

We're hitting this issue in a Rails 7 app for all specs using climate_control to modify ENV.

@bkeepers
Copy link
Owner Author

@dbackeus thanks, I'll work up a PR to disable autorestore if using climate_control or ice_age. In the mean time, do you have any idea how ENV is getting frozen? I can't figure out what is freezing it or how they're doing it, so any help with that would be appreciated.

@dbackeus
Copy link

I can confirm that your PR makes our specs green. But no idea about the frozen issue.

@rgarner
Copy link

rgarner commented Feb 21, 2024

We're not using Climate Control, but we are using stub_const in RSpec, e.g. before { stub_const('ENV', ENV.to_hash.merge('SOME_KEY' => 'some-value')) } and our dependabot build against dotenv-rails still breaks in this way.

We're using

  config.dotenv.autorestore = false

in config/environments/test.rb if it helps someone else.

(but it's better to just set the ENV var and use this autorestore instead, because that's the actual feature 🤦🏻 )

@kajetanowicz
Copy link

kajetanowicz commented Feb 23, 2024

I can't figure out what is freezing it or how they're doing it, so any help with that would be appreciated.

Hi @bkeepers 👋 I wasn't able to fully figure out the exact chain of events but by trial and error it seems that the issue is caused by this bit https://github.com/bkeepers/dotenv/blob/main/lib/dotenv/diff.rb#L53. When Dotenv is used together with rspec's stub_const('ENV', 'foo' -> 'bar') it seems that the latter is doing it's own housekeeping after the test run and later when the autorestore kicks in it's trying to call replace on something that was previously frozen by the Diff class. One way to go around this issue is to remove the freeze part from Diff but it's not really solving the underlying problem.

mpw5 added a commit to ministryofjustice/Claim-for-Crown-Court-Defence that referenced this issue Apr 15, 2024
Upgrading to version 3 of the dotenv-rails gem causes tests in
zendesk_sender_spec to fail with the error `FrozenError: can't modify
frozen Hash: [...]`.

This is because the latest version of dotenv-rails introduces an
Autorestore feature which automatically restores the state of ENV
between rspec tests. In zendesk_sender_spec we stub the value of
ENV['ENV'] using `stub_const`, which also resets between tests and leads
to a conflict: bkeepers/dotenv#482.

This is resolved by replacing the use of `stub_const` with the
`Dotenv.modify` method (also introduced in v3), which makes dotenv-rails
fully responsible for the stubbing of environment variables and avoids
conflict.
@sk-
Copy link

sk- commented May 6, 2024

@bkeepers as others have already mentioned this issue is not exclusive to certain CI environments, but rather to the fact that using rspec's stub_const messes with the autorestore functionality.

Would it be possible to reopen this issue and provide guidelines on the impact of disabling autorestore?

@bkeepers
Copy link
Owner Author

bkeepers commented May 6, 2024

@rgarner, @kajetanowicz and @sk-, thanks for the stub_const pointers. I was able to make a test case that demonstrated the issue and fix it in #504.

The problem is happens when you replace ENV with an instance of Hash. ENV.to_h returns a copy, but Hash#to_h just returns itself, which was then getting frozen.

This should be fixed in 3.1.2, which will be out shortly.

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