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

Remove the last of NativeException's influence. #4144

Closed
wants to merge 1 commit into from

Conversation

headius
Copy link
Member

@headius headius commented Sep 11, 2016

NativeException was replaced by simply rethrowing the actual Java
exception some time ago, and most code did the right thing.
However, a few places still threw NativeException in error along
unlikely call paths (e.g. unrecoverable class lookup errors).
This commit removes the last bits of NativeException use.

  1. NativeException itself is stripped down to just be a simple
    subclass of RuntimeException in Ruby with no special backing
    Java object. Since it will no longer be created, code that
    uses it will never see it anyway. The class and its public
    methods remain to allow such code to compile.
  2. All remaining places that threw NativeException were modified
    to rethrow the original exception.
  3. Tests referencing NativeException were removed, along with any
    test utilities they alone consumed. The only remaining tests
    did so synthetically by constructing NativeException directly.

@headius headius force-pushed the kill_nativeexception branch 3 times, most recently from b026b9f to 95b9ba4 Compare September 11, 2016 19:38
NativeException was replaced by simply rethrowing the actual Java
exception some time ago, and most code did the right thing.
However, a few places still threw NativeException in error along
unlikely call paths (e.g. unrecoverable class lookup errors).
This commit removes the last bits of NativeException use.

1. NativeException itself is stripped down to just be a simple
   subclass of RuntimeException in Ruby with no special backing
   Java object. Since it will no longer be created, code that
   uses it will never see it anyway. The class and its public
   methods remain to allow such code to compile.
2. All remaining places that threw NativeException were modified
   to rethrow the original exception.
3. Tests referencing NativeException were removed, along with any
   test utilities they alone consumed. The only remaining tests
   did so synthetically by constructing NativeException directly.
@headius
Copy link
Member Author

headius commented Sep 30, 2016

I went too far...we do still create NativeException when someone uses it in a rescue, and I broke many things expecting that NativeException would never be in flight. Will have to re-attempt this.

@headius headius closed this Sep 30, 2016
@headius headius added this to the Invalid or Duplicate milestone Sep 30, 2016
@headius headius deleted the kill_nativeexception branch September 30, 2016 05:58
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

1 participant