Ensure core exceptions get raised with $! as cause by headius · Pull Request #9409 · jruby/jruby

headius · 2026-05-01T22:03:01Z

Many places where we raise exceptions from JRuby core code, we do not attach $! as a cause. This leads to several issues, including part of the issue reported in #9398. This PR adds specs and fixes known cases.

headius · 2026-05-02T19:56:36Z

@eregon There are some modifications to mspec's raise_error to support matching causes, modifications to existing specs to use that functionality, and some additional cause-checking specs here, if you want to review.

headius · 2026-05-02T20:01:43Z

Failures appear to be GHA issues or bad timing in specs, unrelated to these changes.

eregon · 2026-05-03T11:17:43Z

Thank you for adding more cause specs.

My initial thought here is whether making raise_error more complicated for this is worth it:

-> { 1 / 0 }.should raise_error(ZeroDivisionError, cause:)
# vs
-> { 1 / 0 }.should raise_error(ZeroDivisionError) { |e| e.cause.should == cause }

It looks nice and it's expressive but OTOH it's not used in that many files.

The main concern is MSpec tries to be as simple as possible so it can be used by early Ruby implementations.
That means so far we don't use keyword arguments in MSpec (git grep -F '**' lib has none).
That's easily solved though, see https://github.com/ruby/mspec/blob/31e8f11c0bf18cf85fa3d3643ea8fc5a395fc3fe/lib/mspec/matchers/complain.rb#L4-L7 for example.

If you want to modify raise_error, could you make a PR https://github.com/ruby/mspec and add spec for it in https://github.com/ruby/mspec/blob/master/spec/matchers/raise_error_spec.rb? Or you could also do it in this PR and run the mspec specs locally.

headius · 2026-05-06T02:20:46Z

It looks nice and it's expressive but OTOH it's not used in that many files.

I think it will be used in many more as we continue to specify language-level exceptions that should have causes. I've only added a few cases here.

And even without those additional cases, there were many places doing the block form of raise_error just to check causes. Those are almost all one-liners now.

This adds a cause kwarg to raise_error for matching the cause of a captured exception. The error output for failed cases is also modified to show both message and cause if they are being matched.

headius · 2026-05-06T03:43:19Z

@eregon I cleaned up the commit that adds the cause: functionality, added specs for it, and updated others to support the expanded error output. Also fixed all style issues.

eregon

Thank you, looks good, just needs a few more tweaks and it's good to go

eregon · 2026-05-06T20:43:23Z

+      -> { raise RuntimeError, "the consequence" }
+        .should raise_error(RuntimeError, "the consequence", cause:)


Suggested change

-> { raise RuntimeError, "the consequence" }

.should raise_error(RuntimeError, "the consequence", cause:)

-> {

raise RuntimeError, "the consequence"

}.should raise_error(RuntimeError, "the consequence", cause:)

This is more consistent with ruby/spec style

eregon · 2026-05-06T20:46:01Z

-      "#{exception_class} (#{message})"
-    else
-      "#{exception_class}"
+      string << "#{prefix.()}message: #{message.inspect}"


Suggested change

string << "#{prefix.()}message: #{message.inspect}"

string << "#{prefix.()}#{message.inspect}"

A message: prefix every time seems quite noisy, and it's a clear a String is the message and not the cause, so let's omit that.
The #{message.inspect} sounds fine.

eregon · 2026-05-06T20:53:47Z

  attr_writer :block

-  def initialize(exception, message, &block)
+  def initialize(exception, message = nil, options = { }, &block)


Suggested change

def initialize(exception, message = nil, options = { }, &block)

def initialize(exception, message = nil, options = nil, &block)

This is consistent with what raise_error below passes.
Also it avoids allocating an extra Hash when it's not used in most cases.

Below it becomes

@cause = options ? options.fetch(:cause, UNDEF_CAUSE) : UNDEF_CAUSE

which seems simple enough to avoid the extra Hash allocation.

eregon · 2026-05-06T20:55:25Z

+    matcher = RaiseErrorMatcher.new(RuntimeError, "foo", cause:)
+    begin
+      matcher.matches?(proc)
+    rescue RuntimeError
+      expect(matcher.failure_message).to eq(
+        ["Expected RuntimeError(message: \"foo\", cause: #<RuntimeError: bar>)", "but got: RuntimeError(message: \"foo\", cause: nil)"]
+      )
+    end


It would be good to test the case with cause but not message too

eregon · 2026-05-06T20:56:04Z

    rescue UnexpectedException => e
      expect(matcher.failure_message).to eq(
-        ["Expected ExpectedException (message)", "but got: UnexpectedException (message)"]
+        ["Expected ExpectedException(message: \"message\")", "but got: UnexpectedException(message: \"message\")"]


Minor: these would read nicer (and close to what gets printed) if using single-quote string now that we have " inside.

headius added this to the JRuby 10.1.1.0 milestone May 1, 2026

headius force-pushed the core_exception_causes branch from 1855854 to 9804bb8 Compare May 2, 2026 19:44

eregon reviewed May 3, 2026

View reviewed changes

Comment thread spec/mspec/lib/mspec/matchers/raise_error.rb Outdated

Comment thread spec/mspec/lib/mspec/matchers/raise_error.rb Outdated

headius added 5 commits May 5, 2026 22:40

Add logic for matching exception cause

e0fb96d

This adds a cause kwarg to raise_error for matching the cause of a captured exception. The error output for failed cases is also modified to show both message and cause if they are being matched.

Simplify exception cause expectations

9df82c6

Add specs for core Name/NoMethodError including cause

6144a97

Check that cause is set for NoMethodError here

7320279

Also set cause for NoMethodError

7ae3e91

headius force-pushed the core_exception_causes branch from 9804bb8 to 7ae3e91 Compare May 6, 2026 03:41

eregon approved these changes May 6, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Ensure core exceptions get raised with $! as cause#9409

Ensure core exceptions get raised with $! as cause#9409
headius wants to merge 5 commits intojruby:masterfrom
headius:core_exception_causes

headius commented May 1, 2026

Uh oh!

headius commented May 2, 2026

Uh oh!

headius commented May 2, 2026

Uh oh!

eregon commented May 3, 2026

Uh oh!

Uh oh!

Uh oh!

headius commented May 6, 2026

Uh oh!

headius commented May 6, 2026

Uh oh!

eregon left a comment

Uh oh!

eregon May 6, 2026

Uh oh!

eregon May 6, 2026

Uh oh!

eregon May 6, 2026

Uh oh!

eregon May 6, 2026

Uh oh!

eregon May 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		-> { raise RuntimeError, "the consequence" }
		.should raise_error(RuntimeError, "the consequence", cause:)

	string << "#{prefix.()}message: #{message.inspect}"
	string << "#{prefix.()}#{message.inspect}"

	def initialize(exception, message = nil, options = { }, &block)
	def initialize(exception, message = nil, options = nil, &block)

Uh oh!

Conversation

headius commented May 1, 2026

Uh oh!

headius commented May 2, 2026

Uh oh!

headius commented May 2, 2026

Uh oh!

eregon commented May 3, 2026

Uh oh!

Uh oh!

Uh oh!

headius commented May 6, 2026

Uh oh!

headius commented May 6, 2026

Uh oh!

eregon left a comment

Choose a reason for hiding this comment

Uh oh!

eregon May 6, 2026

Choose a reason for hiding this comment

Uh oh!

eregon May 6, 2026

Choose a reason for hiding this comment

Uh oh!

eregon May 6, 2026

Choose a reason for hiding this comment

Uh oh!

eregon May 6, 2026

Choose a reason for hiding this comment

Uh oh!

eregon May 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants