EnvResult supporting map_err with Env reference

This provides one possible solution for being able to map errors to
exceptions in native method implementations, after using
`EnvUnowned::with_env`

What I don't particularly like is that you would probably end up
repeating a bunch of boilerplate across lots of different native methods
and it would be nice if you could instead implement a trait for your own
`Error` type that could e.g. have a `to_throwable` or `map_err` method.

Author: rib

src/env.rs

@@ -3856,6 +3856,95 @@ impl<'local> Env<'local> {
     }
 }
 
+/// An opaque wrapper around a `Result<T, Error>` that supports mapping errors
+/// with access to an [`Env`] reference.
+///
+/// This returned by [`EnvUnowned::with_env`] and is designed for use within
+/// native method implementations where unwinding can't be caught by the JVM and
+/// will abort the process.
+///
+/// There is no `.unwrap()` that would panic on error but, if necessary, you can
+/// use [Self::unwrap_result] to get the inner `Result` and handle the error as
+/// you see fit.
+///
+/// In most cases it is recommended to either:
+/// 1. Use [Self::map_err] to convert errors to an Ok value that can be returned
+///    from your native method or to throw an exception using the provided `Env`
+///    reference.
+/// 2. Use [Self::unwrap_or_default] to return a default value (often `null` for
+///    reference types) in case of an error, considering that `null` is often a
+///    valid return value for Java methods.
+#[must_use = "the inner `Result` may be an `Err` variant, which should be handled. `map_err` has access to JNI"]
+#[derive(Debug)]
+pub struct EnvResult<'unowned_frame, T, E>
+where
+    E: From<Error>,
+{
+    env_ptr: *mut jni_sys::JNIEnv,
+    result: std::result::Result<T, E>,
+    _lifetime: std::marker::PhantomData<&'unowned_frame ()>,
+}
+
+impl<'unowned_frame, T, E> EnvResult<'unowned_frame, T, E>
+where
+    E: From<Error>,
+{
+    /// Creates a new [`EnvResult`] from a raw JNI environment pointer and a
+    /// `Result`.
+    ///
+    /// # Safety
+    ///
+    /// The `ptr` must be a valid, non-null pointer to a `JNIEnv`.
+    pub(crate) unsafe fn new(
+        env_ptr: *mut jni_sys::JNIEnv,
+        result: std::result::Result<T, E>,
+    ) -> Self {
+        Self {
+            env_ptr,
+            result,
+            _lifetime: std::marker::PhantomData,
+        }
+    }
+
+    /// Maps the error value of this [`EnvResult`] to another type using the
+    /// provided closure.
+    ///
+    /// The closure is provided with a mutable reference to an `Env` that can
+    /// access JNI and be used to throw exceptions if needed.
+    ///
+    /// The closure effectively runs within a call to [`EnvUnowned::with_env`]
+    /// that will catch any panics and return an [`Error::PanicCaught`] if a
+    /// panic occurs.
+    pub fn map_err<F, O>(self, f: F) -> EnvResult<'unowned_frame, T, O>
+    where
+        F: FnOnce(&mut Env<'unowned_frame>, E) -> O,
+        O: From<Error>,
+    {
+        let mut unowned_env: EnvUnowned<'unowned_frame> =
+            unsafe { EnvUnowned::from_raw(self.env_ptr) };
+        unowned_env.with_env(|env| self.result.map_err(|e| f(env, e)))
+    }
+
+    /// Consumes the `SafeResult`, returning the wrapped `Result`
+    ///
+    /// Be careful within the implementation of native methods to not `.unwrap()`
+    /// or `.expect()` the `Result` and potentially unwind across the FFI boundary.
+    pub fn unwrap_result(self) -> std::result::Result<T, E> {
+        self.result
+    }
+
+    /// Consumes the [`EnvResult`], returning the wrapped `Ok` value or `T::default()`.
+    ///
+    /// Note that all [`Reference`] types, like [`JObject`] or [`JString`] etc implement
+    /// `Default`, which will return a `null` which is often valid for returning to Java.
+    pub fn unwrap_or_default(self) -> T
+    where
+        T: Default,
+    {
+        self.result.unwrap_or_default()
+    }
+}
+
 /// Represents an external (unowned) JNI stack frame and thread attachment that
 /// was passed to a native method call.
 ///
@@ -3896,22 +3985,28 @@ impl<'unowned_frame> EnvUnowned<'unowned_frame> {
     /// method implementations in cases where you have named the lifetime for
     /// the caller's JNI stack frame.
     ///
-    /// Since it would lead to undefined behaviour to allow Rust code to unwind
-    /// across a native method call boundary, this API wraps the closure
-    /// in a [`catch_unwind`] to catch any panics and return an [`Error::PanicCaught`]
-    /// if a panic occurs.
+    /// It returns an [`EnvResult`] that is an opaque wrapper around your
+    /// `Result` that continues to borrow your `EnvUnowned` reference to support
+    /// mapping errors with access to an [`Env`], so you may choose to throw
+    /// errors as exceptions.
+    ///
+    /// To avoid the risk of unwinding into the JVM (which will abort the
+    /// process) this API wraps the closure in a [`catch_unwind`] to catch any
+    /// panics and return an [`Error::PanicCaught`] if a panic occurs.
     ///
     /// Note: This API does not create a new JNI stack frame, which is normally
     /// what you want when implementing a native method, since the JVM will
     /// clean up the JNI stack frame when the native method returns.
-    pub fn with_env<F, T, E>(&mut self, f: F) -> std::result::Result<T, E>
+    ///
+    /// Note: This API returns an [`EnvResult`]
+    pub fn with_env<F, T, E>(&mut self, f: F) -> EnvResult<'unowned_frame, T, E>
     where
         F: FnOnce(&mut Env<'unowned_frame>) -> std::result::Result<T, E>,
         E: From<Error>,
     {
         // Safety: we trust that self.ptr a valid, non-null pointer
         let mut guard = unsafe { AttachGuard::from_unowned(self.ptr) };
-        guard.with_env_current_frame(|env| {
+        let result = guard.with_env_current_frame(|env| {
             // ☠☠☠☠☠☠☠☠☠☠☠☠☠☠  !WARNING!  ☠☠☠☠☠☠☠☠☠☠☠☠☠☠
             //
             // We are casting the Env<'lifetime> here so the closure will
@@ -3955,7 +4050,8 @@ impl<'unowned_frame> EnvUnowned<'unowned_frame> {
                 Ok(ret) => ret,
                 Err(payload) => Err(Error::PanicCaught(panic_payload_to_string(payload)).into()),
             }
-        })
+        });
+        unsafe { EnvResult::new(self.ptr, result) }
     }
 
     /// Runs a closure with a [`Env`] based on an unowned JNI thread attachment
@@ -4006,7 +4102,7 @@ impl<'unowned_frame> EnvUnowned<'unowned_frame> {
     /// know represents a valid JNI attachment for the current thread.
     ///
     /// If you are implementing a native method in Rust though, you should
-    /// prefer to use the `EnvUnowned` type as the first argument to your
+    /// prefer to use the [`EnvUnowned`] type as the first argument to your
     /// native method and avoid the need to use a raw pointer.
     ///
     /// If you have a raw [`crate::sys::JNIEnv`] pointer, this API should be
@@ -4035,6 +4131,16 @@ impl<'unowned_frame> EnvUnowned<'unowned_frame> {
             _lifetime: std::marker::PhantomData,
         }
     }
+
+    /// Returns the raw pointer to the underlying [`crate::sys::JNIEnv`].
+    pub fn as_raw(&self) -> *mut jni_sys::JNIEnv {
+        self.ptr
+    }
+
+    /// Consumes the [`EnvUnowned`] and returns the raw pointer to the underlying [`crate::sys::JNIEnv`].
+    pub fn into_raw(self) -> *mut jni_sys::JNIEnv {
+        self.ptr
+    }
 }
 
 #[derive(Debug)]