Kotlin errors being `FooErrorException` is awkward.

[mhammond]

eg, ArithmeticErrorException here - all other bindings have the more natural ArithmeticError as defined in the udl.

It's grating on me in https://github.com/mozilla/application-services/pull/4068 so thought I'd ask if this really is idiomatic?

┆Issue is synchronized with this Jira Task

[rfk]

I agree this is pretty yuck :-/

Naively, it feels like the right thing to do would be to have the exception class named ArithmeticError just like in other bindings. If calling it "FooException" is really important for idiomatic Kotlin reasons, we could also consider translating error classes named FooError into FooException automatically in the bindings.

@grigoryk maybe you can give us a bit of a gut-check here from the PoV of a consumer? Looking at the FxA error class wrappers here, would it be appropriate for us to rename fxaclient.FxaErrorException to just fxaclient.FxaError?

[npars]

In java a "FooError" would typically be expected to extend Error and would be considered fatal. Meanwhile "FooException" would be expected to extend Exception and be non-fatal / catchable. I assume that was the original intention behind tacking on "Exception" at the end.

With that in mind, I would think translating Error -> Exception would be the cleanest solution.

[jhugman]

@npars is correct: from a JVM point of view, Error means something very specific. Removing an Error suffix before appending Exception may be the write thing to do here, even if low priority.

Stymergic hint: the quickest dirtiest path I can think of would be doing this in class_name_kt in gen_kotlin.rs.

[grigoryk]

ditto JVM Error meaning.

I agree with @jhugman's suggestion RE dropping Error before the name conversion, so we end up w/ ArithmeticException, not ArithmeticErrorException.

[rfk]

I support the direction we're going here, and it seems inline with other name-munging that we do in order to make the generated bindings more idiomtic. To sum up, I believe the consensus desired behavior to be:

• Continue to encourage [Error] declarations to be named like FooError in the interface definition.
• When generating the Kotlin bindings for an Error, change FooError into FooException in the public API.

I think this will actually be a pretty self-contained change. The mechanics of our Kotlin error handling are already split into two pieces for any given FooError:

• A FooError class that derives from a base RustError, defining the C FFI struct that gets used when calling over the FFI; this is a JNA Structure.ByReference and is not throwable.
• A wrapping FooErrorException class that extends Exception and is the thing that is actually exposed in the public API.
  • This in turn defines an inner subclass per error variant, e.g. variant FooError::BadThing in the udl shows up in the Kotlin API as the class FooErrorException.BadThingException.

So concretely, I think we can have the change we want here by adjusting only the name-generating logic in the exception wrapper class:

• Make a new Kotlin template filter like exception_name_kt that removes any "Error" suffix from a name and then appends "Exception".
• Use the filter when generating names for both the Exception wrapper class and its inner classes.

I do want to flag though, that this will potentially interact with the work proposed in #460. In fact, it could be a really nice way for someone like @bendk to ease in to the logic for handling errors before taking on that bigger piece of work 😁

[sebastient]

It would be helpful for the documentation on defining error types in UDL clarifies that errors cannot be named Error which is typical in Rust but fails because this munges the name to Exception, which it cannot be.

[mhammond]

Kotlin renaming Error to Exception is expected and should not cause problems. Can you please open a new issue describing your problem in more detail?