search

timob / jnigi

Golang Java JNI library
BSD 2-Clause "Simplified" License
166 stars 44 forks source link

Expand exception handling #42

Closed andrewmostello closed 4 years ago

andrewmostello commented 4 years ago

After working with this package for the last few weeks, I was hoping to improve the way exceptions are handled. I saw that handleException had a TODO line, and wanted to take a crack at fulfilling the need.

What I wanted to do here is make sure that any enhanced handling is opt-in, and that it is flexible/customizable. To do that, I created a new ExceptionHandler interface, and added an optional ExceptionHandler field to Env. I think that's the best approach here, especially if it turns out the robust ThrowableErrorExceptionHandler (see below) has problems in production.

Please let me know what you think - thanks!

Details in the commit message:

Expand Env exception handling by adding the ExceptionHandlerInterface and ExceptionHandler field to Env. When this field is set in Env, exceptions encountered and run through handleException are processed using the provided handler, and the error it creates is returned. This enables custom error handling / exception conversion.

In addition to providing custom handling, this patch includes three handlers available for use:

  • DescribeExceptionHandler provides the previous behavior, printing with describeException.
  • ThrowableToStringExceptionHandler calls the "toString" method of the exception, and returns an error with that message.
  • ThrowableErrorExceptionHandler retrieves the values in the exception, storing them in the new ThrowableError error, and returning that value. This conversion includes the stack trace, converting those values into a slice of the new StackTraceElement struct. This is recursive to convert any underlying cause.

By default, the existing behavior is preserved, and adding one of the new handlers (or a custom one) is opt-in.

timob commented 4 years ago

That looks great, this will make using Java libraries much more robust. ā˜•ā˜•ā˜•

I'll try this out ASAP, looking forward to it!

andrewmostello commented 4 years ago

Great! Thanks for taking a look!

timob commented 4 years ago

That is comprehensive exception handling there. Should I add you to the AUTHORS file?

andrewmostello commented 4 years ago

Great - thanks for merging this PR. Sure! It'd be great to be part of the AUTHORS file.

timob commented 4 years ago

No problem, thanks for the contribution! šŸ™Œ I am doing other work ATM but when I get back to doing work with Java and Go this is going to be golden. I already have an app that could benefit from this, it needs to handle network errors from Java.

When I do get back to Java and Go probably will be to creating a wrapper around some of the Android API. This will be generated code. This generated code uses reflection to convert between Go and general generic Java classes, for example like ArrayList<T> (https://github.com/timob/javabind/blob/master/runtime.go#L343).

Is there anything in particular that you are using JNIGI for?