diff --git a/CHANGELOG.md b/CHANGELOG.md index e795f86641..1324422276 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/Cargo.lock b/Cargo.lock index 7dffc9d6b0..cba9d727f7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1817,6 +1817,7 @@ version = "0.22.0" dependencies = [ "anyhow", "bytes", + "thiserror", "uniffi", ] diff --git a/fixtures/ext-types/lib/src/lib.rs b/fixtures/ext-types/lib/src/lib.rs index ea0e3807b1..f00d4a0759 100644 --- a/fixtures/ext-types/lib/src/lib.rs +++ b/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 { Arc::new(ExternalCrateInterface::new(val)) } diff --git a/fixtures/ext-types/lib/tests/bindings/test_imported_types.py b/fixtures/ext-types/lib/tests/bindings/test_imported_types.py index 098b80c8da..561524466a 100644 --- a/fixtures/ext-types/lib/tests/bindings/test_imported_types.py +++ b/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() diff --git a/fixtures/ext-types/lib/tests/bindings/test_imported_types.swift b/fixtures/ext-types/lib/tests/bindings/test_imported_types.swift index 4fd2cae21f..fa4b33ee7a 100644 --- a/fixtures/ext-types/lib/tests/bindings/test_imported_types.swift +++ b/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") diff --git a/fixtures/ext-types/uniffi-one/Cargo.toml b/fixtures/ext-types/uniffi-one/Cargo.toml index a9cf7b188b..659a8f3351 100644 --- a/fixtures/ext-types/uniffi-one/Cargo.toml +++ b/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] diff --git a/fixtures/ext-types/uniffi-one/src/lib.rs b/fixtures/ext-types/uniffi-one/src/lib.rs index 90ecc191e8..f3a24a0e3d 100644 --- a/fixtures/ext-types/uniffi-one/src/lib.rs +++ b/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; diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index d409febd15..8e4fd785c1 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -576,6 +576,15 @@ impl 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::*; diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt b/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt index 7d818ca6dc..f854fbe37e 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/macros.kt +++ b/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) diff --git a/uniffi_bindgen/src/bindings/python/templates/ExternalTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ExternalTemplate.py index 36fb052ea6..313c0bc13e 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ExternalTemplate.py +++ b/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) }} diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index 4522c5fba2..6051d7a3b9 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/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 %} diff --git a/uniffi_bindgen/src/bindings/swift/templates/macros.swift b/uniffi_bindgen/src/bindings/swift/templates/macros.swift index 91abadaf45..c20557bdc4 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/macros.swift +++ b/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 %}