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

Support overriding Exception#message #5136

Merged
merged 1 commit into from Apr 12, 2018
Merged

Support overriding Exception#message #5136

merged 1 commit into from Apr 12, 2018

Conversation

ujihisa
Copy link
Contributor

@ujihisa ujihisa commented Apr 12, 2018

In CRuby, an uncaught exception messages users with the exception's message, considering if it's overriden or not.

e.g.

class A < StandardError
  def message
    'hello'
  end
end

raise A
  • CRuby stderr: /tmp/vu4TBd5/647:7:in `<main>': hello (A)
  • JRuby stderr: A: A\n <main> at /tmp/vu4TBd5/648:7

This pull request changes JRuby behaviour similar to CRuby's.

  • (patched) JRuby stderr: A: hello\n <main> at /tmp/vu4TBd5/650:7

Note that if the overriden message method raises an exception, it shouldn't message the internal exception, but the external exception, according to CRuby Bug #14566 https://bugs.ruby-lang.org/issues/14566

This pullreq already considers that.

e.g.

class A < StandardError
  def message
    raise 'boom boom!'
  end
end

raise A
  • CRuby stderr: /tmp/vu4TBd5/655:7:in `<main>': A
  • (patched) JRuby stderr: A: A\n <main> at /tmp/vu4TBd5/656:7"

I'm not fluent in Java, so please let me know if there's somewhere you don't feel good! I'm more than happy to update my commit.

@headius
Copy link
Member

headius commented Apr 12, 2018

The patch looks just fine to me, thank you @ujihisa!

@headius headius added this to the JRuby 9.2.0.0 milestone Apr 12, 2018
@headius headius merged commit 34bec6e into jruby:master Apr 12, 2018
@ujihisa
Copy link
Contributor Author

ujihisa commented Apr 12, 2018

thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants