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

Implement Java::JavaLang::Throwable#full_message #5908

Merged
merged 1 commit into from Oct 8, 2019

Conversation

@davishmcclurg
Copy link
Contributor

davishmcclurg commented Oct 8, 2019

Since Java throwables are treated similarly to regular Ruby exceptions,
I think it makes sense to implement full_message for easier logging
and debugging.

This moves the RubyException implementation to TraceType so that it
can be shared with Throwable. I had to change the exception argument
type in printBacktraceMRI in order to support the exception-like
object that Throwable provides.

Closes: #5906

Since Java throwables are treated similarly to regular Ruby exceptions,
I think it makes sense to implement `full_message` for easier logging
and debugging.

This moves the `RubyException` implementation to `TraceType` so that it
can be shared with `Throwable`. I had to change the `exception` argument
type in `printBacktraceMRI` in order to support the exception-like
object that `Throwable` provides.

Closes: #5906
@headius

This comment has been minimized.

Copy link
Member

headius commented Oct 8, 2019

The original intent of actually propagating Java exceptions was indeed to make them duck type as Ruby exceptions, so this makes sense.

@headius headius added this to the JRuby 9.2.9.0 milestone Oct 8, 2019
@headius

This comment has been minimized.

Copy link
Member

headius commented Oct 8, 2019

Looks good to me. The whitespace changes are a little annoying, but the whitespace that was there shouldn't have been. If you do any future PRs, try to keep the whitespace changes in a separate commit, so they don't show up in git blame as being related to the actual change.

Thank you for the patch!

@headius headius merged commit 80455ae into jruby:master Oct 8, 2019
5 checks passed
5 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jruby.jruby Build #20191008.1 succeeded
Details
jruby.jruby (Job linux) Job linux succeeded
Details
jruby.jruby (Job mac) Job mac succeeded
Details
jruby.jruby (Job windows) Job windows succeeded
Details
@davishmcclurg davishmcclurg deleted the davishmcclurg:throwable-full-message branch Oct 8, 2019
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.

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