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.

@ujihisa ujihisa force-pushed the ujihisa:master branch from 9358962 to 3e3dae5 Apr 12, 2018
@headius
Copy link
Member

@headius 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
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@ujihisa
Copy link
Contributor Author

@ujihisa ujihisa commented Apr 12, 2018

thx!

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.