Add support for returning and handling errors

[mozilla]

Add support for returning and handling errors

[linabutler]

That sounds perfect! 💯

On the Rust side, we can use the Error enum variants as names for the error_codes module, and leave it up to the consumer to implement From<MyErrorType> for ExternError—or are you thinking of autogenerating the Rust-side errors, too, and having the crate use the autogenerated errors directly?

Swift has fallible functions, methods and constructors; plumbing the throws Option<T> through to them seems like it would work perfectly, plus generating the Swift-side Error enum. Kotlin doesn't need to specially annotate fallible functions at all; we can transform the RustError into a Kotlin error and throw directly.

[rfk]

Following how Firefox does it, using a Throws annotation on fallible methods?

This was my first instinct as well.

But I also have to be honest, I've been spending my evenings exploring what it would look like to use our own custom variant of an IDL rather than just relying on what's available in WebIDL...which might come to nothing or might change how we want to proceed here. We should not block on that though :-)

Do we want to have structured errors that can be inspected, or are error strings good enough?

I think we need some structure, e.g. in the fxa-client the kotlin code needs to be able to distinguish between a network error and a hard failure.

Strawman: [Throws(EnumName)] would annotate a function/method that can throw, and the resulting error would have both a string and an integer errcode that maps to the specified enum. This is basically how our existing ExternError class works in the hand-written bindings IIUC, except you have to know the possible range of error codes via out-of-band means.

[tarikeshaq]

I'm thinking of taking a look at this, but before I go in over my head, I wanted to dump my thought process in case I have something off (or maybe there are blockers I haven't thought about or something)

The WebIDL would look something like this:

enum Error {
   "NETWORK_ERROR", "TYPE_ERROR"
};

interface InterfaceName {
   constructor();
   [Throws(Error)]
   string func();
};

• From that, we can add a throws Option<T> in here: https://github.com/rfk/uniffi-rs/blob/6a399f8a2e42893fa99d3894ee1d75afb042e73d/uniffi/src/interface/mod.rs#L590-L594 That gets populated with the throws attribute (Haven't looked at the details here)

This should give us all the information needed to form the scaffolding and the bindings (right?)

Edit: Guess we'd need to add the same Option for Function and (maybe) Constructor

From the scaffolding side

• Add the error: &mut ExternError argument to the FFI functions

• Define mappings between our generated Error enum and ffi_support::ExternError, Something along the lines of this: https://github.com/mozilla/application-services/blob/f814698ea973a5f1f2c9b31b1c7a4661b32f6ea3/components/fxa-client/src/ffi.rs#L53-L77

• Edit the templates and place the Result<..> at the right spots, and change the ffi call to use with_resultwhenever we have a fallible function

Bindings

Each binding will be different but the concept is the same

• Detect errors from the ffi call
• appropriately propagate the error

Haven't really looked at the details here and it might be painful, but if the details of the error are in the Method struct, then I think we have all the info needed

Thoughts? Anything large I miss? Or is there a different path you'd rather we go with? Any feedback is welcome!

[tarikeshaq]

Thanks @linacambridge!!

For

On the Rust side, we can use the Error enum variants as names for the error_codes module, and leave it up to the consumer to implement From<MyErrorType> for ExternError

I was playing around and one thing I tried is to let the consumer define their own Rust Error enum. The Rust enum exactly matches the IDL one and has to be named Error. Then, we generate the From<Error> for ExternError. The upside is that the user doesn't have to know about ExternError, but there are a few downsides, a couple off the top of my head:

• We have to find a way to mark errors when we parse the IDL, right now I just assume that there will be exactly one Error enum and its name is Error (similar to how we have only one in fxa_client and such), but we can use attributes or something to allow more flexibility there I guess.
• The consumer has to abide by the limitations of WebIDL enums, (biggest one I think is that they are forced to only have flat non-struct variants with the same names from the IDL).

I guess it's a matter of flexibility vs if we want the consumer to know about ExternError

Anywhoo, I'll put up my WIP in a PR, but I might end up switching to what you said 👀

[rfk]

I was playing around and one thing I tried is to let the consumer define their own Rust Error enum.
The Rust enum exactly matches the IDL one and has to be named Error.

FWIW, in general I like the principal that you should be able to look at the rust code and understand in isolation as plain old rust code, without having to understand the extra things that will get magicked into existence by the scaffolding. I think that aligns broadly with what you describe above - the user defines an Error in rust and uses it for idiomatic rust error-handling stuff, they declare a matching Error in the IDL and we generate all the boilerplate to convert it into an ExternError and pass it over the FFI.

(I haven't looked at your PR yet, that's purely an idea-level reaction based on what you describe above).

We have to find a way to mark errors when we parse the IDL,

We could use an attribute, or we could infer which enums are errors by walking the set of all functions/methods and mark anything that gets thrown as being an error enum. (If we do use an attribute, we'll have to do the opposite check and ensure that anything things marked with the attribute are thrown).

The consumer has to abide by the limitations of WebIDL enums

Yep, which IIUC is similar to the current restrictions on non-error enums between Rust and the IDL.

One of the advantages of this philosophy I think, is that it keeps open the open of moving from a separate IDL to some inline definitions based on macros.