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

Show that ArgumentError from encoding error is recursive #4117

Closed
wants to merge 1 commit into from

Conversation

nightscape
Copy link
Contributor

In general Java exceptions should not have themselves as cause and JRuby in most cases handles this correctly.
We just ran into a problem where an ArgumentError raised due to an encoding problem does have itself as cause which leads to a problem in an ActiveSupport method.
The ActiveSupport method is a little dangerous/questionable by itself, but the recursive cause should be prevented in JRuby nonetheless and there is also a test that checks this for RuntimeErrors.
We're not sure if this is the most general reproduction, so feel free to ignore this PR if you find a more general test.

@nightscape
Copy link
Contributor Author

Issue #4024 might be related to this.

@headius
Copy link
Member

headius commented Sep 30, 2016

I think this is simply us being too eager to use the in-flight exception when re-raising, not realizing we're stuffing the same exception into itself again. Looking.

@headius
Copy link
Member

headius commented Sep 30, 2016

This only appears to affect re-raising using the raise ex form, and I have a fix coming.

$ jruby -e 'begin; "hi \255".split; rescue => e; p e.cause; raise e; end rescue p $!.cause'
nil
#<ArgumentError: invalid byte sequence in UTF-8>

@headius
Copy link
Member

headius commented Sep 30, 2016

I've fixed the issue, but the test would be better going into ruby/spec (spec/ruby/core/exception or spec/ruby/core/kernel/raise_spec.rb).

@headius headius closed this Sep 30, 2016
@headius headius added this to the JRuby 9.1.6.0 milestone Sep 30, 2016
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