signalapp / libsignal

Home to the Signal Protocol as well as other cryptographic primitives which make Signal possible.
GNU Affero General Public License v3.0
3.63k stars 420 forks source link

Error handling and panic catching in jni-rs 0.21 #515

Closed AshleySchaeffer closed 1 year ago

AshleySchaeffer commented 1 year ago

Historically I took some heavy inspiration from libsignal to handle errors/panics to throw Java exceptions. I recently updated to jni-rs 0.21 which introduces some breaking changes, and it broke my spin on your error handling approach. Therefore you will most likely also be affected if/when you upgrade.

Here's your code on error handling that this applies to: https://github.com/signalapp/libsignal/blob/c98ed9cb6dcc87613d31cf5acc7d330bc399c4c4/rust/bridge/shared/src/jni/mod.rs

Here's the issue I raised explaining the breaking change, with a workaround for my use case that may help you in the comments: https://github.com/jni-rs/jni-rs/issues/432

PS. Signal is awesome, feel free to close this issue. If you prefer being contacted in a different way sorry for raising an issue, please let me know.

jrose-signal commented 1 year ago

Thanks for the heads-up! I've subscribed to the jni-rs issue, at least. I'm going to close this since it's an implementation detail for us and we track work items outside of GitHub, but we do appreciate it.