-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Exception backtrace is nil when it should be present #4467
Comments
This is a workaround for a JRuby bug: jruby/jruby#4467
This is a workaround for a JRuby bug: jruby/jruby#4467
Ok this is pretty easy to understand where we tripped up I think: begin
p 1
rescue Foo
rescue Bar
end AST:
The RescueBodyNodes are nested and the first one says we don't need the backtrace. Our builder needs to be made a little smarter to see multiple rescue bodies for this case... |
@DavidEGrayson A workaround for now would be to not have the empty case as the first rescue. I will fix this tomorrow and it will be in 9.1.8.0 but if you swap order then it will work with earlier versions of JRuby too. |
Thanks for looking at this so quickly! Swapping the order of rescues is not a general workaround because it can change the behavior of the program, but I thought my workaround of adding |
I think it will be pretty hard to correctly predict whether backtraces are needed ahead of time, before the exception is thrown. Here is a program with a normal-looking rescue block but you don't know what class module M
begin
if ENV['PATHOLOGICAL'] == 'Y'
class StandardError < ::StandardError
end
end
raise 'hello'
rescue StandardError
end
end It seems like the only rescue blocks that are easy to reason about correctly before you run them are the ones that don't specify a class. |
@DavidEGrayson just so I understand you are you saying that the === operator of a derived class would end up asking for the backtrace? I am increasingly thinking we should just use this for RESCUE_MOD cases like 'foo rescue nil' but my mind is not catching what you are saying. |
I believe we are willing to break some pathological cases -- like a rescue with a Generally the only time you will miss the trace is if the error gets reported (as in your case with rspec failures), and I believe there's a flag to disable this behavior (or if my memory is wrong, it would be easy to add). The alternative -- disabling this feature which has only had a couple easily-fixed issues -- would mean that libraries that raise exceptions for flow control (not uncommon) run something like 1000x slower for those cases. The fix for this case is simple and obvious: we need to examine all rescues to see if they're nontrivial. Given that fix, your original issue should be resolved. If a time comes when someone really wants |
FWIW, I can think of one thing that would make the pathological cases pretty benign: instead of no trace at all, we provide a dummy trace that suggests the flag to disable backtrace suppression and links to a wiki information page with explanations and workarounds. |
I wasn't trying to talk about derived classes or the
I thought you would say that. If you can fix RSpec, that would be great. However, after the fix, if there are still some cases where you are sacrificing correctness for speed then I will probably point it out. |
Ahh, I understand what you mean now. The best option for dealing would be as @enebo hinted at: only do this optimization when there's a bare rescue or "rescue nil" style modifier (since those are always predictably StandardError). I believe we only omit the backtrace for StandardError subclasses now anyway, but we may want to turn off the optimization when we can't prove all StandardError are rescued. |
@DavidEGrayson aha..I get it now. yeah RESCUE_MOD case only now. Which was the original intent of the optimization but I got greedy. |
Thanks! |
It looks like the performance improvements mentioned on the JRuby blog probably broke exception backtrace generation in some cases. This affected RSpec, causing the following bug: rspec/rspec-core#2299. I can reproduce the problem by running this very simple script in JRuby:
Here is a shell session showing my environment and the output of that script:
MRI behaves as expected and prints "Looks good" when it runs that same script.
For anyone affected by this, a workaround is to add
=> _
to the first rescue statement.The text was updated successfully, but these errors were encountered: