Skip to content

Commit

Permalink
Make From<UnexpectedUniFFICallbackError> optional
Browse files Browse the repository at this point in the history
This is in preparation of mozilla#1578 -- allowing foreign languages to
implement traits.

One issue there is that if we want to allow foreign languages to
implement traits, then any Result return types need to implement
`LiftReturn`, and that impl currently has a bound on `E:
From<UnexpectedUniFFICallbackError`.  IOW, it's only implemented for
errors that can be converted from UnexpectedUniFFICallbackError.

This makes sense when the trait is actually used by the foreign
languages, however not all traits are intended for that use.  For that
matter, some libraries may be fine panicking in the the face of
unexpected callback errors.

By using the auto-ref specialization hack, we're able to convert the
error if there`s a `From<UnexpectedUniFFICallbackError>` impl, but use a
panicking version if not.  The trait bound is no longer required to
implement `LiftReturn`.
  • Loading branch information
bendk committed Oct 11, 2023
1 parent b2d4714 commit 7f18b82
Show file tree
Hide file tree
Showing 11 changed files with 140 additions and 23 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@
- Updated the async functionality to correctly handle cancellation (#1669)
- Kotlin: Fixed low-level issue with exported async APIs

### What's changed?

- Implementing `From<uniffi::UnexpectedUniFFICallbackError` is now optional for callback interface error types.
If the error type implements that, things will continue to work as before.
If not, then any unexpected callback error will result in a Rust panic.

## v0.24.3 (backend crates: v0.24.3) - (_2023-08-01_)

[All changes in v0.24.3](https://github.com/mozilla/uniffi-rs/compare/v0.24.2...v0.24.3).
Expand Down
17 changes: 9 additions & 8 deletions docs/manual/src/udl/callback_interfaces.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,21 @@ It's currently allowed for callback interface methods to return a regular value
rather than a `Result<>`. However, this is means that any exception from the
foreign bindings will lead to a panic.

### Extra requirements for errors used in callback interfaces
### Errors used in callback interfaces

In order to support errors in callback interfaces, UniFFI must be able to
properly [lift the error](../internals/lifting_and_lowering.md). This means
that the if the error is described by an `enum` rather than an `interface` in
the UDL (see [Errors](./errors.md)) then all variants of the Rust enum must be unit variants.

In addition to expected errors, a callback interface call can result in all kinds of
unexpected errors. Some examples are the foreign code throws an exception that's not part
of the exception type or there was a problem marshalling the data for the call. UniFFI
uses `uniffi::UnexpectedUniFFICallbackError` for these cases. Your code must include a
`From<uniffi::UnexpectedUniFFICallbackError>` impl for your error type to handle those or
the UniFFI scaffolding code will fail to compile. See `example/callbacks` for an
example of how to do this.
### Unexpected errors

When Rust code invokes a callback interface method, that call may result in all kinds of unexpected errors.
Some examples are the foreign code throws an exception that's not part of the exception type or there was a problem marshalling the data for the call.
UniFFI creates an `uniffi::UnexpectedUniFFICallbackError` for these cases.
If you code defines a `From<uniffi::UnexpectedUniFFICallbackError>` impl for your error type, then those errors will be converted into your error type which will be returned to the Rust caller.
If not, then any unexpected errors will result in a panic.
See `example/callbacks` for an example of this.

## 3. Define a callback interface in the UDL

Expand Down
74 changes: 74 additions & 0 deletions uniffi_core/src/ffi/callbackinterface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,77 @@ impl fmt::Display for UnexpectedUniFFICallbackError {
}

impl std::error::Error for UnexpectedUniFFICallbackError {}

// Autoref-based specialization for converting UnexpectedUniFFICallbackError into error types.
//
// For more details, see:
// https://github.com/dtolnay/case-studies/blob/master/autoref-specialization/README.md

// Define two ZST types:
// - One implements `try_convert_unexpected_callback_error` by always returning an error value.
// - The specialized version implements it using `From<UnexpectedUniFFICallbackError>`

#[doc(hidden)]
#[derive(Debug)]
pub struct UnexpectedUniFFICallbackErrorConverterGeneric;

impl UnexpectedUniFFICallbackErrorConverterGeneric {
pub fn try_convert_unexpected_callback_error<E>(
&self,
e: UnexpectedUniFFICallbackError,
) -> anyhow::Result<E> {
Err(e.into())
}
}

#[doc(hidden)]
#[derive(Debug)]
pub struct UnexpectedUniFFICallbackErrorConverterSpecialized;

impl UnexpectedUniFFICallbackErrorConverterSpecialized {
pub fn try_convert_unexpected_callback_error<E>(
&self,
e: UnexpectedUniFFICallbackError,
) -> anyhow::Result<E>
where
E: From<UnexpectedUniFFICallbackError>,
{
Ok(E::from(e))
}
}

// Macro to convert an UnexpectedUniFFICallbackError value for a particular type. This is used in
// the `ConvertError` implementation.
#[doc(hidden)]
#[macro_export]
macro_rules! convert_unexpected_error {
($error:ident, $ty:ty) => {{
// Trait for generic conversion, implemented for all &T.
pub trait GetConverterGeneric {
fn get_converter(&self) -> $crate::UnexpectedUniFFICallbackErrorConverterGeneric;
}

impl<T> GetConverterGeneric for &T {
fn get_converter(&self) -> $crate::UnexpectedUniFFICallbackErrorConverterGeneric {
$crate::UnexpectedUniFFICallbackErrorConverterGeneric
}
}
// Trait for specialized conversion, implemented for all T that implements
// `Into<ErrorType>`. I.e. it's implemented for UnexpectedUniFFICallbackError when
// ErrorType implements From<UnexpectedUniFFICallbackError>.
pub trait GetConverterSpecialized {
fn get_converter(&self) -> $crate::UnexpectedUniFFICallbackErrorConverterSpecialized;
}

impl<T: Into<$ty>> GetConverterSpecialized for T {
fn get_converter(&self) -> $crate::UnexpectedUniFFICallbackErrorConverterSpecialized {
$crate::UnexpectedUniFFICallbackErrorConverterSpecialized
}
}
// Here's the hack. Because of the auto-ref rules, this will use `GetConverterSpecialized`
// if it's implemented and `GetConverterGeneric` if not.
(&$error)
.get_converter()
.try_convert_unexpected_callback_error($error)
}};
}
8 changes: 4 additions & 4 deletions uniffi_core/src/ffi_converter_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
/// "UT" means an abitrary `UniFfiTag` type.
use crate::{
check_remaining, derive_ffi_traits, ffi_converter_rust_buffer_lift_and_lower, metadata,
FfiConverter, ForeignExecutor, Lift, LiftReturn, Lower, LowerReturn, MetadataBuffer, Result,
RustBuffer, UnexpectedUniFFICallbackError,
ConvertError, FfiConverter, ForeignExecutor, Lift, LiftReturn, Lower, LowerReturn,
MetadataBuffer, Result, RustBuffer, UnexpectedUniFFICallbackError,
};
use anyhow::bail;
use bytes::buf::{Buf, BufMut};
Expand Down Expand Up @@ -535,7 +535,7 @@ where
unsafe impl<UT, R, E> LiftReturn<UT> for Result<R, E>
where
R: LiftReturn<UT>,
E: Lift<UT> + From<UnexpectedUniFFICallbackError>,
E: Lift<UT> + ConvertError<UT>,
{
fn lift_callback_return(buf: RustBuffer) -> Self {
Ok(R::lift_callback_return(buf))
Expand All @@ -553,7 +553,7 @@ where
}

fn handle_callback_unexpected_error(e: UnexpectedUniFFICallbackError) -> Self {
Err(E::from(e))
Err(E::try_convert_unexpected_callback_error(e).unwrap_or_else(|e| panic!("{e}")))
}

const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_RESULT)
Expand Down
15 changes: 15 additions & 0 deletions uniffi_core/src/ffi_converter_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,10 @@ pub unsafe trait LiftRef<UT> {
type LiftType: Lift<UT> + Borrow<Self>;
}

pub trait ConvertError<UT>: Sized {
fn try_convert_unexpected_callback_error(e: UnexpectedUniFFICallbackError) -> Result<Self>;
}

/// Derive FFI traits
///
/// This can be used to derive:
Expand All @@ -373,6 +377,7 @@ macro_rules! derive_ffi_traits {
$crate::derive_ffi_traits!(impl<UT> LowerReturn<UT> for $ty);
$crate::derive_ffi_traits!(impl<UT> LiftReturn<UT> for $ty);
$crate::derive_ffi_traits!(impl<UT> LiftRef<UT> for $ty);
$crate::derive_ffi_traits!(impl<UT> ConvertError<UT> for $ty);
};

(local $ty:ty) => {
Expand All @@ -381,6 +386,7 @@ macro_rules! derive_ffi_traits {
$crate::derive_ffi_traits!(impl LowerReturn<crate::UniFfiTag> for $ty);
$crate::derive_ffi_traits!(impl LiftReturn<crate::UniFfiTag> for $ty);
$crate::derive_ffi_traits!(impl LiftRef<crate::UniFfiTag> for $ty);
$crate::derive_ffi_traits!(impl ConvertError<crate::UniFfiTag> for $ty);
};

(impl $(<$($generic:ident),*>)? $(::uniffi::)? Lower<$ut:path> for $ty:ty $(where $($where:tt)*)?) => {
Expand Down Expand Up @@ -448,4 +454,13 @@ macro_rules! derive_ffi_traits {
type LiftType = Self;
}
};

(impl $(<$($generic:ident),*>)? $(::uniffi::)? ConvertError<$ut:path> for $ty:ty $(where $($where:tt)*)?) => {
impl $(<$($generic),*>)* $crate::ConvertError<$ut> for $ty $(where $($where)*)*
{
fn try_convert_unexpected_callback_error(e: $crate::UnexpectedUniFFICallbackError) -> $crate::deps::anyhow::Result<Self> {
$crate::convert_unexpected_error!(e, $ty)
}
}
};
}
2 changes: 1 addition & 1 deletion uniffi_core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub mod metadata;

pub use ffi::*;
pub use ffi_converter_traits::{
FfiConverter, FfiConverterArc, Lift, LiftRef, LiftReturn, Lower, LowerReturn,
ConvertError, FfiConverter, FfiConverterArc, Lift, LiftRef, LiftReturn, Lower, LowerReturn,
};
pub use metadata::*;

Expand Down
4 changes: 2 additions & 2 deletions uniffi_macros/src/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */

use crate::util::{derive_ffi_traits, ident_to_string, mod_path, tagged_impl_header};
use crate::util::{derive_all_ffi_traits, ident_to_string, mod_path, tagged_impl_header};
use proc_macro2::{Ident, TokenStream};
use quote::quote;
use syn::Path;
Expand All @@ -15,7 +15,7 @@ pub(crate) fn expand_ffi_converter_custom_type(
udl_mode: bool,
) -> syn::Result<TokenStream> {
let impl_spec = tagged_impl_header("FfiConverter", ident, udl_mode);
let derive_ffi_traits = derive_ffi_traits(ident, udl_mode);
let derive_ffi_traits = derive_all_ffi_traits(ident, udl_mode);
let name = ident_to_string(ident);
let mod_path = mod_path()?;

Expand Down
4 changes: 2 additions & 2 deletions uniffi_macros/src/enum_.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use quote::quote;
use syn::{Data, DataEnum, DeriveInput, Field, Index};

use crate::util::{
create_metadata_items, derive_ffi_traits, ident_to_string, mod_path, tagged_impl_header,
create_metadata_items, derive_all_ffi_traits, ident_to_string, mod_path, tagged_impl_header,
try_metadata_value_from_usize, try_read_field,
};

Expand Down Expand Up @@ -64,7 +64,7 @@ fn enum_or_error_ffi_converter_impl(
) -> TokenStream {
let name = ident_to_string(ident);
let impl_spec = tagged_impl_header("FfiConverter", ident, udl_mode);
let derive_ffi_traits = derive_ffi_traits(ident, udl_mode);
let derive_ffi_traits = derive_all_ffi_traits(ident, udl_mode);
let mod_path = match mod_path() {
Ok(p) => p,
Err(e) => return e.into_compile_error(),
Expand Down
6 changes: 4 additions & 2 deletions uniffi_macros/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use syn::{
use crate::{
enum_::{rich_error_ffi_converter_impl, variant_metadata},
util::{
chain, create_metadata_items, either_attribute_arg, ident_to_string, kw, mod_path,
parse_comma_separated, tagged_impl_header, try_metadata_value_from_usize,
chain, create_metadata_items, derive_ffi_traits, either_attribute_arg, ident_to_string, kw,
mod_path, parse_comma_separated, tagged_impl_header, try_metadata_value_from_usize,
AttributeSliceExt, UniffiAttributeArgs,
},
};
Expand Down Expand Up @@ -87,6 +87,7 @@ fn flat_error_ffi_converter_impl(
) -> TokenStream {
let name = ident_to_string(ident);
let lower_impl_spec = tagged_impl_header("Lower", ident, udl_mode);
let derive_ffi_traits = derive_ffi_traits(&["ConvertError"], ident, udl_mode);
let mod_path = match mod_path() {
Ok(p) => p,
Err(e) => return e.into_compile_error(),
Expand Down Expand Up @@ -161,6 +162,7 @@ fn flat_error_ffi_converter_impl(
quote! {
#lower_impl
#lift_impl
#derive_ffi_traits
}
}

Expand Down
6 changes: 3 additions & 3 deletions uniffi_macros/src/record.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use syn::{
};

use crate::util::{
create_metadata_items, derive_ffi_traits, either_attribute_arg, ident_to_string, kw, mod_path,
tagged_impl_header, try_metadata_value_from_usize, try_read_field, AttributeSliceExt,
create_metadata_items, derive_all_ffi_traits, either_attribute_arg, ident_to_string, kw,
mod_path, tagged_impl_header, try_metadata_value_from_usize, try_read_field, AttributeSliceExt,
UniffiAttributeArgs,
};

Expand Down Expand Up @@ -41,7 +41,7 @@ pub(crate) fn record_ffi_converter_impl(
udl_mode: bool,
) -> syn::Result<TokenStream> {
let impl_spec = tagged_impl_header("FfiConverter", ident, udl_mode);
let derive_ffi_traits = derive_ffi_traits(ident, udl_mode);
let derive_ffi_traits = derive_all_ffi_traits(ident, udl_mode);
let name = ident_to_string(ident);
let mod_path = mod_path()?;
let write_impl: TokenStream = record.fields.iter().map(write_field).collect();
Expand Down
21 changes: 20 additions & 1 deletion uniffi_macros/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,14 +216,33 @@ pub(crate) fn tagged_impl_header(
}
}

pub(crate) fn derive_ffi_traits(ty: &Ident, udl_mode: bool) -> TokenStream {
pub(crate) fn derive_all_ffi_traits(ty: &Ident, udl_mode: bool) -> TokenStream {
if udl_mode {
quote! { ::uniffi::derive_ffi_traits!(local #ty); }
} else {
quote! { ::uniffi::derive_ffi_traits!(blanket #ty); }
}
}

pub(crate) fn derive_ffi_traits(trait_names: &[&str], ty: &Ident, udl_mode: bool) -> TokenStream {
let trait_idents = trait_names
.iter()
.map(|name| Ident::new(name, Span::call_site()));
if udl_mode {
quote! {
#(
::uniffi::derive_ffi_traits!(impl #trait_idents<crate::UniFfiTag> for #ty);
)*
}
} else {
quote! {
#(
::uniffi::derive_ffi_traits!(impl<UT> #trait_idents<UT> for #ty);
)*
}
}
}

/// Custom keywords
pub mod kw {
syn::custom_keyword!(async_runtime);
Expand Down

0 comments on commit 7f18b82

Please sign in to comment.