Java: Handle panics and errors in the Java FFI layer

java/client/build.gradle

@@ -219,6 +219,8 @@ tasks.withType(Test) {
         events "started", "skipped", "passed", "failed"
         showStandardStreams true
     }
+    // This is needed for the FFI tests
+    jvmArgs "-Djava.library.path=${projectDir}/../target/debug"
 }
 
 jar {

java/client/src/test/java/glide/ffi/FfiTest.java

@@ -5,6 +5,7 @@
 import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import glide.ffi.resolvers.RedisValueResolver;
@@ -42,6 +43,18 @@ public class FfiTest {
 
     public static native long createLeakedLongSet(long[] value);
 
+    // This tests that panics do not cross the FFI boundary and an exception is thrown if a panic is
+    // caught
+    public static native long handlePanics(
+            boolean shouldPanic, boolean errorPresent, long value, long defaultValue);
+
+    // This tests that Rust errors are properly converted into Java exceptions and thrown
+    public static native long handleErrors(boolean isSuccess, long value, long defaultValue);
+
+    // This tests that a Java exception is properly thrown across the FFI boundary
+    public static native void throwException(
+            boolean throwTwice, boolean isRuntimeException, String message);
+
     @Test
     public void redisValueToJavaValue_Nil() {
         long ptr = FfiTest.createLeakedNil();
@@ -141,4 +154,52 @@ public void redisValueToJavaValue_Set() {
                 () -> assertTrue(result.contains(2L)),
                 () -> assertEquals(result.size(), 2));
     }
+
+    @Test
+    public void handlePanics_panic() {
+        long expectedValue = 0L;
+        long value = FfiTest.handlePanics(true, false, 1L, expectedValue);
+        assertEquals(expectedValue, value);
+    }
+
+    @Test
+    public void handlePanics_returnError() {
+        long expectedValue = 0L;
+        long value = FfiTest.handlePanics(false, true, 1L, expectedValue);
+        assertEquals(expectedValue, value);
+    }
+
+    @Test
+    public void handlePanics_returnValue() {
+        long expectedValue = 2L;
+        long value = FfiTest.handlePanics(false, false, expectedValue, 0L);
+        assertEquals(expectedValue, value);
+    }
+
+    @Test
+    public void handleErrors_success() {
+        long expectedValue = 0L;
+        long value = FfiTest.handleErrors(true, expectedValue, 1L);
+        assertEquals(expectedValue, value);
+    }
+
+    @Test
+    public void handleErrors_error() {
+        assertThrows(Exception.class, () -> FfiTest.handleErrors(false, 0L, 1L));
+    }
+
+    @Test
+    public void throwException() {
+        assertThrows(Exception.class, () -> FfiTest.throwException(false, false, "My message"));
+    }
+
+    @Test
+    public void throwException_throwTwice() {
+        assertThrows(Exception.class, () -> FfiTest.throwException(true, false, "My message"));
+    }
+
+    @Test
+    public void throwException_throwRuntimeException() {
+        assertThrows(RuntimeException.class, () -> FfiTest.throwException(false, true, "My message"));
+    }
 }

java/src/errors.rs

@@ -0,0 +1,108 @@
+/**
+ * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
+ */
+use jni::{errors::Error as JNIError, JNIEnv};
+use log::error;
+use std::string::FromUtf8Error;
+
+pub enum FFIError {
+    Jni(JNIError),
+    Uds(String),
+    Utf8(FromUtf8Error),
+}
+
+impl From<jni::errors::Error> for FFIError {
+    fn from(value: jni::errors::Error) -> Self {
+        FFIError::Jni(value)
+    }
+}
+
+impl From<FromUtf8Error> for FFIError {
+    fn from(value: FromUtf8Error) -> Self {
+        FFIError::Utf8(value)
+    }
+}
+
+impl std::fmt::Display for FFIError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            FFIError::Jni(err) => write!(f, "{}", err),
+            FFIError::Uds(err) => write!(f, "{}", err),
+            FFIError::Utf8(err) => write!(f, "{}", err),
+        }
+    }
+}
+
+#[derive(Copy, Clone)]
+pub enum ExceptionType {
+    Exception,
+    RuntimeException,
+}
+
+impl std::fmt::Display for ExceptionType {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ExceptionType::Exception => write!(f, "java/lang/Exception"),
+            ExceptionType::RuntimeException => write!(f, "java/lang/RuntimeException"),
+        }
+    }
+}
+
+// This handles `FFIError`s by converting them to Java exceptions and throwing them.
+pub fn handle_errors<T>(env: &mut JNIEnv, result: Result<T, FFIError>) -> Option<T> {
+    match result {
+        Ok(value) => Some(value),
+        Err(err) => {
+            match err {
+                FFIError::Utf8(utf8_error) => throw_java_exception(
+                    env,
+                    ExceptionType::RuntimeException,
+                    &utf8_error.to_string(),
+                ),
+                error => throw_java_exception(env, ExceptionType::Exception, &error.to_string()),
+            };
+            // Return `None` because we need to still return a value after throwing.
+            // This signals to the caller that we need to return the default value.
+            None
+        }
+    }
+}
+
+// This function handles Rust panics by converting them into Java exceptions and throwing them.
+// `func` returns an `Option<T>` because this is intended to wrap the output of `handle_errors`.
+pub fn handle_panics<T, F: std::panic::UnwindSafe + FnOnce() -> Option<T>>(
+    func: F,
+    ffi_func_name: &str,
+) -> Option<T> {
+    match std::panic::catch_unwind(func) {
+        Ok(value) => value,
+        Err(_err) => {
+            // Following https://github.com/jni-rs/jni-rs/issues/76#issuecomment-363523906
+            // and throwing a runtime exception is not feasible here because of https://github.com/jni-rs/jni-rs/issues/432
+            error!("Native function {} panicked.", ffi_func_name);
+            None
+        }
+    }
+}
+
+pub fn throw_java_exception(env: &mut JNIEnv, exception_type: ExceptionType, message: &str) {
+    match env.exception_check() {
+        Ok(true) => (),
+        Ok(false) => {
+            env.throw_new(exception_type.to_string(), message)
+                .unwrap_or_else(|err| {
+                    error!(
+                        "Failed to create exception with string {}: {}",
+                        message,
+                        err.to_string()
+                    );
+                });
+        }
+        Err(err) => {
+            error!(
+                "Failed to check if an exception is currently being thrown: {}",
+                err.to_string()
+            );
+        }
+    }
+}

java/src/ffi_test.rs

@@ -1,9 +1,10 @@
 /**
  * Copyright GLIDE-for-Redis Project Contributors - SPDX Identifier: Apache-2.0
  */
+use crate::errors::{handle_errors, handle_panics, throw_java_exception, ExceptionType, FFIError};
 use jni::{
-    objects::{JClass, JLongArray},
-    sys::jlong,
+    objects::{JByteArray, JClass, JLongArray, JString},
+    sys::{jboolean, jdouble, jlong},
     JNIEnv,
 };
 use redis::Value;
@@ -21,7 +22,7 @@ pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedNil<'local>(
 pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedSimpleString<'local>(
     mut env: JNIEnv<'local>,
     _class: JClass<'local>,
-    value: jni::objects::JString<'local>,
+    value: JString<'local>,
 ) -> jlong {
     let value: String = env.get_string(&value).unwrap().into();
     let redis_value = Value::SimpleString(value);
@@ -51,7 +52,7 @@ pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedInt<'local>(
 pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedBulkString<'local>(
     env: JNIEnv<'local>,
     _class: JClass<'local>,
-    value: jni::objects::JByteArray<'local>,
+    value: JByteArray<'local>,
 ) -> jlong {
     let value = env.convert_byte_array(&value).unwrap();
     let value = value.into_iter().collect::<Vec<u8>>();
@@ -88,7 +89,7 @@ pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedMap<'local>(
 pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedDouble<'local>(
     _env: JNIEnv<'local>,
     _class: JClass<'local>,
-    value: jni::sys::jdouble,
+    value: jdouble,
 ) -> jlong {
     let redis_value = Value::Double(value.into());
     Box::leak(Box::new(redis_value)) as *mut Value as jlong
@@ -98,7 +99,7 @@ pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedDouble<'local>(
 pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedBoolean<'local>(
     _env: JNIEnv<'local>,
     _class: JClass<'local>,
-    value: jni::sys::jboolean,
+    value: jboolean,
 ) -> jlong {
     let redis_value = Value::Boolean(value != 0);
     Box::leak(Box::new(redis_value)) as *mut Value as jlong
@@ -108,7 +109,7 @@ pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedBoolean<'local>(
 pub extern "system" fn Java_glide_ffi_FfiTest_createLeakedVerbatimString<'local>(
     mut env: JNIEnv<'local>,
     _class: JClass<'local>,
-    value: jni::objects::JString<'local>,
+    value: JString<'local>,
 ) -> jlong {
     use redis::VerbatimFormat;
     let value: String = env.get_string(&value).unwrap().into();
@@ -144,3 +145,68 @@ fn java_long_array_to_value<'local>(
         .map(|value| Value::Int(*value))
         .collect::<Vec<Value>>()
 }
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_handlePanics<'local>(
+    _env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    should_panic: jboolean,
+    error_present: jboolean,
+    value: jlong,
+    default_value: jlong,
+) -> jlong {
+    let should_panic = should_panic != 0;
+    let error_present = error_present != 0;
+    handle_panics(
+        || {
+            if should_panic {
+                panic!("Panicking")
+            } else if error_present {
+                None
+            } else {
+                Some(value)
+            }
+        },
+        "handlePanics",
+    )
+    .unwrap_or(default_value)
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_handleErrors<'local>(
+    mut env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    is_success: jboolean,
+    value: jlong,
+    default_value: jlong,
+) -> jlong {
+    let is_success = is_success != 0;
+    let error = FFIError::Uds("Error starting socket listener".to_string());
+    let result = if is_success { Ok(value) } else { Err(error) };
+    handle_errors(&mut env, result).unwrap_or(default_value)
+}
+
+#[no_mangle]
+pub extern "system" fn Java_glide_ffi_FfiTest_throwException<'local>(
+    mut env: JNIEnv<'local>,
+    _class: JClass<'local>,
+    throw_twice: jboolean,
+    is_runtime_exception: jboolean,
+    message: JString<'local>,
+) {
+    let throw_twice = throw_twice != 0;
+    let is_runtime_exception = is_runtime_exception != 0;
+
+    let exception_type = if is_runtime_exception {
+        ExceptionType::RuntimeException
+    } else {
+        ExceptionType::Exception
+    };
+
+    let message: String = env.get_string(&message).unwrap().into();
+    throw_java_exception(&mut env, exception_type, &message);
+
+    if throw_twice {
+        throw_java_exception(&mut env, exception_type, &message);
+    }
+}