Get external errors working for Swift and Python (#2390)

Does not work with Kotlin - see #2392

CHANGELOG.md

@@ -42,6 +42,8 @@ or use UDL with types from more than one crate.
   to consumers and could have high overhead when lots of FFI calls are made. Instead, the
   `ffi-trace` feature can be used to get tracing-style printouts about the FFI.
 
+- External errors work for Swift and Python. Kotlin does not work - see #2392.
+
 ### What's changed?
 
 - Switching jinja template engine from askama to rinja.

fixtures/ext-types/lib/src/lib.rs

@@ -5,8 +5,8 @@ use ext_types_external_crate::{
 };
 use std::sync::Arc;
 use uniffi_one::{
-    UniffiOneEnum, UniffiOneInterface, UniffiOneProcMacroType, UniffiOneTrait, UniffiOneType,
-    UniffiOneUDLTrait,
+    UniffiOneEnum, UniffiOneError, UniffiOneErrorInterface, UniffiOneInterface,
+    UniffiOneProcMacroType, UniffiOneTrait, UniffiOneType, UniffiOneUDLTrait,
 };
 use uniffi_sublib::SubLibType;
 use url::Url;
@@ -195,6 +195,19 @@ fn get_uniffi_one_proc_macro_type(t: UniffiOneProcMacroType) -> UniffiOneProcMac
     t
 }
 
+#[uniffi::export]
+fn throw_uniffi_one_error() -> Result<(), UniffiOneError> {
+    Err(UniffiOneError::Oops("oh no".to_string()))
+}
+
+// external interface errors don't quite work.
+#[uniffi::export]
+fn throw_uniffi_one_error_interface() -> Result<(), UniffiOneErrorInterface> {
+    Err(UniffiOneErrorInterface {
+        e: "interface oops".to_string(),
+    })
+}
+
 fn get_external_crate_interface(val: String) -> Arc<ExternalCrateInterface> {
     Arc::new(ExternalCrateInterface::new(val))
 }

fixtures/ext-types/lib/tests/bindings/test_imported_types.py

@@ -98,6 +98,13 @@ def test_misc_external_types(self):
         self.assertEqual(get_nested_external_guid(None), "nested-external")
         self.assertEqual(get_nested_external_ouid(None), "nested-external-ouid")
 
+    def test_external_errors(self):
+        with self.assertRaises(UniffiOneError) as cm:
+            throw_uniffi_one_error()
+
+        with self.assertRaises(UniffiOneErrorInterface) as cm:
+            throw_uniffi_one_error_interface()
+
 if __name__=='__main__':
     test_external_callback_interface_impl()
     unittest.main()

fixtures/ext-types/lib/tests/bindings/test_imported_types.swift

@@ -61,5 +61,23 @@ assert(getMaybeUniffiOneEnum(e: nil) == nil)
 assert(getUniffiOneEnums(es: [UniffiOneEnum.one]) == [UniffiOneEnum.one])
 assert(getMaybeUniffiOneEnums(es: [UniffiOneEnum.one, nil]) == [UniffiOneEnum.one, nil])
 
+do {
+    try throwUniffiOneError()
+    fatalError("Should have thrown")
+} catch let e as UniffiOneError {
+    if case let .Oops(reason) = e {
+        assert(reason == "oh no")
+    } else {
+        fatalError("wrong error variant: \(e)")
+    }
+}
+
+do {
+    try throwUniffiOneErrorInterface()
+    fatalError("Should have thrown")
+} catch let e as UniffiOneErrorInterface {
+    assert(e.message() == "interface oops")
+}
+
 assert(ct.ecd.sval == "ecd")
 assert(getExternalCrateInterface(val: "foo").value() == "foo")

fixtures/ext-types/uniffi-one/Cargo.toml

@@ -13,6 +13,7 @@ name = "uniffi_one"
 [dependencies]
 anyhow = "1"
 bytes = "1.3"
+thiserror = "1"
 uniffi = { workspace = true }
 
 [build-dependencies]

fixtures/ext-types/uniffi-one/src/lib.rs

@@ -44,6 +44,38 @@ pub trait UniffiOneTrait: Send + Sync {
     fn hello(&self) -> String;
 }
 
+// A couple of errors used as external types.
+#[derive(thiserror::Error, uniffi::Error, Debug)]
+pub enum UniffiOneError {
+    #[error("{0}")]
+    Oops(String),
+}
+
+#[derive(Debug, uniffi::Object, thiserror::Error)]
+#[uniffi::export(Debug, Display)]
+pub struct UniffiOneErrorInterface {
+    pub e: String,
+}
+
+#[uniffi::export]
+impl UniffiOneErrorInterface {
+    fn message(&self) -> String {
+        self.e.clone()
+    }
+}
+
+impl std::fmt::Display for UniffiOneErrorInterface {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "UniffiOneErrorInterface({})", self.e)
+    }
+}
+
+// We *must* have this so error support is generated. We should fix that and remove this. See #2393.
+#[uniffi::export]
+fn _just_to_get_error_support() -> Result<(), UniffiOneErrorInterface> {
+    Ok(())
+}
+
 // Note `UDL` vs `Udl` is important here to test foreign binding name fixups.
 pub trait UniffiOneUDLTrait: Send + Sync {
     fn hello(&self) -> String;

uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs

@@ -576,6 +576,15 @@ impl<T: AsType> AsCodeType for T {
     }
 }
 
+// A work around for #2392 - we can't handle functions with external errors.
+fn can_render_callable(callable: &dyn Callable, ci: &ComponentInterface) -> bool {
+    // can't handle external errors.
+    callable
+        .throws_type()
+        .map(|t| !ci.is_external(&t))
+        .unwrap_or(true)
+}
+
 mod filters {
     use super::*;
     pub use crate::backend::filters::*;

uniffi_bindgen/src/bindings/kotlin/templates/macros.kt

@@ -29,7 +29,16 @@
 {%- endmacro -%}
 
 {%- macro func_decl(func_decl, callable, indent) %}
+    {%- if self::can_render_callable(callable, ci) %}
+        {%- call render_func_decl(func_decl, callable, indent) %}
+    {%- else %}
+// Sorry, the callable "{{ callable.name() }}" isn't supported.
+    {%- endif %}
+{%- endmacro %}
+
+{%- macro render_func_decl(func_decl, callable, indent) %}
     {%- call docstring(callable, indent) %}
+
     {%- match callable.throws_type() -%}
     {%-     when Some(throwable) %}
     @Throws({{ throwable|type_name(ci) }}::class)

uniffi_bindgen/src/bindings/python/templates/ExternalTemplate.py

@@ -6,5 +6,15 @@
 {{ self.add_import_of(module, ffi_converter_name) }}
 {{ self.add_import_of(module, name) }} {#- import the type alias itself -#}
 
+# External type {{ name }} is an error.
+{%- if ci.is_name_used_as_error(name) %}
+{%-     match type_ %}
+{%-         when Type::Object { .. } %}
+{%-             let err_ffi_converter_name = "{}__as_error"|format(ffi_converter_name) %}
+{{ self.add_import_of(module, err_ffi_converter_name) }}
+{%-         else %}
+{%-     endmatch %}
+{%- endif %}
+
 {%- let rustbuffer_local_name = "_UniffiRustBuffer{}"|format(name) %}
 {{ self.add_import_of_as(module, "_UniffiRustBuffer", rustbuffer_local_name) }}

uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift

@@ -175,6 +175,24 @@ public struct {{ ffi_converter_name }}: FfiConverter {
     }
 }
 
+{#
+We always write these public functions just in case the object is used as
+an external type by another crate.
+#}
+#if swift(>=5.8)
+@_documentation(visibility: private)
+#endif
+public func {{ ffi_converter_name }}_lift(_ pointer: UnsafeMutableRawPointer) throws -> {{ type_name }} {
+    return try {{ ffi_converter_name }}.lift(pointer)
+}
+
+#if swift(>=5.8)
+@_documentation(visibility: private)
+#endif
+public func {{ ffi_converter_name }}_lower(_ value: {{ type_name }}) -> UnsafeMutableRawPointer {
+    return {{ ffi_converter_name }}.lower(value)
+}
+
 {# Objects as error #}
 {%- if is_error %}
 
@@ -208,22 +226,20 @@ public struct {{ ffi_converter_name }}__as_error: FfiConverterRustBuffer {
         fatalError("not implemented")
     }
 }
-{%- endif %}
 
-{#
-We always write these public functions just in case the object is used as
-an external type by another crate.
-#}
+{# Error FFI converters also need these public functions. #}
 #if swift(>=5.8)
 @_documentation(visibility: private)
 #endif
-public func {{ ffi_converter_name }}_lift(_ pointer: UnsafeMutableRawPointer) throws -> {{ type_name }} {
-    return try {{ ffi_converter_name }}.lift(pointer)
+public func {{ ffi_converter_name }}__as_error_lift(_ buf: RustBuffer) throws -> {{ type_name }} {
+    return try {{ ffi_converter_name }}__as_error.lift(buf)
 }
 
 #if swift(>=5.8)
 @_documentation(visibility: private)
 #endif
-public func {{ ffi_converter_name }}_lower(_ value: {{ type_name }}) -> UnsafeMutableRawPointer {
-    return {{ ffi_converter_name }}.lower(value)
+public func {{ ffi_converter_name }}__as_error_lower(_ value: {{ type_name }}) -> RustBuffer {
+    return {{ ffi_converter_name }}__as_error.lower(value)
 }
+
+{%- endif %}

uniffi_bindgen/src/bindings/swift/templates/macros.swift

@@ -7,7 +7,7 @@
 {%- macro to_ffi_call(func) -%}
     {%- call is_try(func) -%}
     {%- if let(Some(e)) = func.throws_type() -%}
-        rustCallWithError({{ e|ffi_error_converter_name }}.lift) {
+        rustCallWithError({{ e|ffi_error_converter_name }}_lift) {
     {%- else -%}
         rustCall() {
     {%- endif %}