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

Rails console crash when returning from a before_save block in Rails / JumpException$ReturnJump #2666

Closed
mvdamme opened this issue Mar 9, 2015 · 5 comments

Comments

@mvdamme
Copy link

mvdamme commented Mar 9, 2015

In a Rails app running under JRuby 1.7.19, I'm having problems when returning from a before_save block. Consider the following dummy rails model:

class Dummy < ActiveRecord::Base
  before_save do |dummy|
    return false
  end
end

Attempting to save such a model should always fail due to the before_save block returning false. Trying this in the rails console causes it to crash without any error message (one just returns to the command prompt) under 1.7.19:

michael@xps:~/Documents/ruby/jruby-test$ rails c
NOTE: ActiveRecord 4.2 is not (yet) fully supported by AR-JDBC, please help us finish 4.2 support - check http://bit.ly/jruby-42 for starters
Loading development environment (Rails 4.2.0)
jruby-1.7.19 :001 > d = Dummy.new(:name => 'aaaa')
   (0.0ms)  SELECT name FROM sqlite_master WHERE type = 'table' AND NOT name = 'sqlite_sequence'
 => #<Dummy id: nil, name: "aaaa", created_at: nil, updated_at: nil> 
jruby-1.7.19 :002 > d.save
michael@xps:~/Documents/ruby/jruby-test$

In my real Rails app (which is Rails 4.1) the same happens in the console. When running the app under trinidad and trying to save the model the result is different: the webbrowser shows a single-line error message (not the normal Rails error page): org.jruby.exceptions.JumpException$ReturnJump. In the Rails logs, no error message is shown, but the status of the request is shown as Completed 401 Unauthorized.

Simply removing the return fixes the problem. Hence, the following works as expected:

class Dummy < ActiveRecord::Base
  before_save do |dummy|
    false
  end
end

For completeness, the output of jruby -v:

jruby 1.7.19 (1.9.3p551) 2015-01-29 20786bd on OpenJDK 64-Bit Server VM 1.6.0_34-b34 +jit [linux-amd64]
@headius
Copy link
Member

headius commented Mar 12, 2015

The way the error manifests isn't great, but I suspect this is probably the wrong flow control. You want to use break or next.

A return within a block will try to return from the method outside that block, rather than returning from the block itself. break will try to return from the method to which the block was passed, and next will actually just do a local return from the block.

I suspect that the difference in MRI is that this block is captured off and turned into a lambda which handles flow control differently (they all just return from the block). That is a valid bug on our end, but you can avoid it by using next here.

@headius
Copy link
Member

headius commented Mar 12, 2015

I have @Who828 looking into this for a smaller reproduction.

@mvdamme
Copy link
Author

mvdamme commented Mar 12, 2015

Thank you for looking into this. I fully agree that the return in the block is not a good idea and I should have used next, thanks for the reminder. The code has been changed in the meantime.

returning from the block worked without any problem under JRuby 1.7.2, though (but that was also with an older Rails version).

@donv
Copy link
Member

donv commented Mar 28, 2015

I believe the behaviour in JRuby 1.7.2 was wrong, and the new behaviour is the right one and also matches MRI.

@enebo enebo added this to the Won't Fix milestone May 16, 2017
@enebo
Copy link
Member

enebo commented May 16, 2017

1.7.x is EOL and this will not get fixed. Marking WONTFIX.

@enebo enebo closed this as completed May 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants