Ok this is pretty easy to understand where we tripped up I think:

begin
  p 1
rescue Foo
rescue Bar
end

AST:

AST:
RootNode 0
  BeginNode 0
    RescueNode 1
      RescueBodyNode 2
        ArrayNode 3
          ConstNode:Foo 3
, 
        RescueBodyNode 3
          ArrayNode 4
            ConstNode:Bar 4
      FCallNode:p 1
        ArrayNode 1
          FixnumNode 1
, null
, null

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 => _ is good enough for now.

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 StandardError will resolve to ahead of time. The exception needs a backtrace if and only if the environment variable PATHOLOGICAL is set to Y:

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 === impl that inspects the backtrace (why?) -- in order to reduce the tremendous cost of generating stack traces when they actually aren't needed.

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 === to have access to the backtrace and then do almost nothing in the rescue body, we'll deal with it then.

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 === operator, the point was that often you will not know what kind of exception is being caught by the rescue clause ahead of time. In the normal case above, rescue StandardError rescues all exceptions of type ::StandardError so we can skip backtraces for most types of exceptions. In the pathological example above, rescue StandardError only rescues exceptions of type M::StandardError (which could be whatever we want it to be), so most typical exceptions will not be caught, and we need to see stack traces for them.

I believe we are willing to break some pathological cases

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!