merit-gem / merit

Reputation engine for Rails apps
Other
1.52k stars 197 forks source link

Error every time code changes #360

Closed ephracis closed 2 years ago

ephracis commented 2 years ago

How to recreate the issue:

  1. Start the app with bin/rails server
  2. Make a change in the controller. Like adding a space and then remove it (so there is actually no change but we can still save the file)
  3. Save the file.
  4. Go to a page, any page

Expected result:

Actual result:

Ambry::AmbryError Id must be unique

ambry (1.0.0) lib/ambry/active_model.rb:107:in `save!'
ambry (1.0.0) lib/ambry/active_model.rb:48:in `create!'
config/initializers/merit.rb:41:in `block (2 levels) in <main>'
config/initializers/merit.rb:39:in `each'
config/initializers/merit.rb:39:in `each_with_index'
config/initializers/merit.rb:39:in `block in <main>'

The merit.rb file:

Rails.application.reloader.to_prepare do
  Merit::Badge.create!(id: 1, name: 'test')
end

If the page is reloaded it works. The error only occurs once and then goes away.

The error occurs if a controller or model is changed, but not if a view is changed.

This is not an issue in production since the app is restarted every time a code change is uploaded. But it is a real nuisance during development since it means every single code change must be followed by one error before the app works fine.

ephracis commented 2 years ago

The error occurs if a controller or model is changed, but not if a view is changed.

Seems to be any .rb file inside app/ directory triggers the error. So JavaScript, views, assets, etc. will not trigger the error. Outside of app/ the only file where I could trigger the error was in config/routes.rb. Changes to any other files such as initializers or environments, will not trigger the error.

tute commented 2 years ago

Thanks for reporting! While we find a good fix, does surrounding your code in a rescue Ambry::AmbryError work?

Rails.application.reloader.to_prepare do
  Merit::Badge.create!(id: 1, name: 'test')
rescue Ambry::AmbryError
end

We shouldn't reload that file.

tute commented 2 years ago

I couldn't replicate this issue. In order to test it, on latest master, I did:

cd test/dummy
rails server
# load a page
# apply edits to views
# reload page
# see no error

I did it with Rails.application.reloader.to_prepare do block in test/dummy/config/initializers/merit.rb and without it. How can I see this bug?

ephracis commented 2 years ago

Hm, well that's both good and bad news. Good news it's not affecting everyone, bad that it is not easy to replicate and fix. I am not sure why this is affecting me, but I will try the solution in #362 this weekend and see if it resolves the issue.

tute commented 2 years ago

Let me know how it went! :)

ephracis commented 2 years ago

I cannot reproduce this anymore. One thing that has changed is that ruby got updated to 3.0 a few weeks ago (and a few gems but I don't think they are relevant).

While I could try to downgrade and dig further into this I would rather just keep on truckin'. So I will close this.

tute commented 2 years ago

Glad it seems to working well now!