Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exposing Errors #509

Open
thunderbiscuit opened this issue Apr 23, 2024 · 6 comments
Open

Exposing Errors #509

thunderbiscuit opened this issue Apr 23, 2024 · 6 comments

Comments

@thunderbiscuit
Copy link
Member

It turns out that errors are not easy to move from Rust to the target languages when using uniffi. In short, it looks like one must choose between a range of options, each with pros and cons, and none particularly perfect. This issue is an attempt at quantifying and categorizing these options and the choices we made for the bdk-ffi language bindings.

@thunderbiscuit
Copy link
Member Author

thunderbiscuit commented Apr 23, 2024

Option 1

One requirement you might have for errors is that their fields be exposed to the bindings. For example, the TxidParseError would like to return to the user not only it's variant's name, but the txid that was not parsable:

#[derive(Debug, thiserror::Error)]
pub enum TxidParseError {
    #[error("invalid txid: {txid}")]
    InvalidTxid { txid: String },
}

In the UDL, this shows up as:

[Error]
interface TxidParseError {
  InvalidTxid(string txid);
};

When building bindings code for this however, the resulting exception thrown will not use the message you might expect (the one built by thiserror). Instead, a custom message field is built which simply prints the names of the fields in the enum and their value:

sealed class TxidParseException: Exception() {
    class InvalidTxid(val txid: String) : TxidParseException() {
        override val message
            get() = "txid=${ `txid` }"
}

This means that we cannot customize the message given to the user; the thiserror library does not help us, and following this approach, one must choose between a custom error message and the fields one might want to expose. For example, instead of a message like

invalid HTTP header value: {value}

You only send back to the Koltin/Swift user an exception with a field message, which would print as

value=${value}

This is inconvenient, and overall doesn't provide the sort of information one might want to convey. We really want to customize our error messages, but we also need to bring in our data! This leads us to option 2.


Option 2

Option 2 relies on using interfaces as error. In this scenario, your errors will be interfaces, which means they will not have any fields. You can instead define a message() method on them, and use the to_string() method in Rust to print your custom message.

impl MyError {
    fn message(&self) -> String> { self.to_string() }
}

This appears better at first glance, but is a little weird on the user side of things, because now your errors don't have the ever so common (at least in Kotlin?) message field. That means that if you have a try/catch block that can handle errors coming from more than just the bdk library, you will have some exceptions that will have the message field and some (the bdk ones) for which this field doesn't exist, and you instead need to call the message() method on it... too messy for general production use IMO.


Option 3

The third option requires using the enum directly without any fields:

[Error]
enum FeeRateError {
  "ArithmeticOverflow"
};

In this situation, the message field on the exception is actually the to_string() in Rust!

sealed class FeeRateException(message: String): Exception(message) {
    class ArithmeticOverflow(message: String) : FeeRateException(message)

This means that we can now leverage the thiserror library to write our custom error messages:

#[derive(Debug, thiserror::Error)]
pub enum FeeRateError {
    #[error("arithmetic overflow on feerate")]
    ArithmeticOverflow,
}

At first glance, the downside of this is that no fields can be passed forward (the enum doesn't have any fields in the target languages). But we can actually use fields on the Rust side to create messages that contain that data once transformed into strings! Not perfect, but it might be better than the other options above.

#[derive(Debug, thiserror::Error)]
pub enum FeeRateError {
    #[error("arithmetic overflow on feerate {e}")]
    ArithmeticOverflow { e: u64 },
}

Option 4

If you use a struct as an error type instead of an enum, your struct can implement the Display trait which will populate the toString() method in Kotlin and Swift, and also contain the full typed data inside. The Mozilla test-fixtures have a good example of this.

This is maybe less applicable for us because it means your errors are not variants of an enum and are simply objects by themselves. It turns out you can combine them into an enum if you want I think, but then you loose the nice Display features.

@reez
Copy link
Collaborator

reez commented Apr 24, 2024

This is an awesome breakdown. 1 or 3 definitely are our best bets.

@thunderbiscuit
Copy link
Member Author

Ok here are a few more data points.

In Swift/Kotlin, the resulting Errors/Exceptions are actually the same whether you choose option 1 or option 2! This is really interesting, because it means that while we think using 2 approaches to errors might feel messy and confuse the end user, this is not the case; the complexity only lies at the ffi layer. We have to know that we build errors in two different ways, but the end user sees the same structures.

Note that for Kotlin/Javausers, they get a message field on all errors, and it sometimes prints it with the inner error_message=message is here structure, and sometimes simply with the message. For Swift users, things change a bit less, and they have to know what fields are available on all errors (which is the same as if they all use option 1 or all use option 2).

Rust code:

#[derive(Debug, thiserror::Error)]
// Using Option 3
pub enum FeeRateError {
    #[error("arithmetic overflow on feerate")]
    ArithmeticOverflow,

    #[error("invalid feerate {e}")]
    InvalidFeeRate { e: u64 },
}

// Using Option 1
#[derive(Debug, thiserror::Error)]
pub enum TxidParseError {
    #[error("invalid txid: {txid}")]
    InvalidTxid { txid: String, error_message: String },
}

UDL file:

[Error]
enum FeeRateError {
  "ArithmeticOverflow",
  "InvalidFeeRate"
};

[Error]
interface TxidParseError {
  InvalidTxid(string txid, string error_message);
};

Kotlin

sealed class FeeRateException(message: String): Exception(message) {
        class ArithmeticOverflow(message: String) : FeeRateException(message)
        class InvalidFeeRate(message: String) : FeeRateException(message)
}

sealed class TxidParseException: Exception() {
    class InvalidTxid(
        val txid: String, 
        val errorMessage: String
    ) : TxidParseException() {
        override val message
            get() = "txid=${txid}, errorMessage=${errorMessage}"
    }
}

Swift

public enum FeeRateError {
    case ArithmeticOverflow(message: String)
    case InvalidFeeRate(message: String)
}
extension FeeRateError: Equatable, Hashable {}
extension FeeRateError: Error { }

public enum TxidParseError {
    case InvalidTxid(
        txid: String, 
        errorMessage: String
    )
}
extension TxidParseError: Equatable, Hashable {}
extension TxidParseError: Error { }

@thunderbiscuit
Copy link
Member Author

So it turns out there is a 4th option. If you use a struct as an error type instead of an enum, your struct can implement the Display trait which will populate the toString() method in Kotlin and Swift, and also contain the full typed data inside. The Mozilla test-fixtures have a good example of this.

This is maybe less applicable for us because it means your errors are not variants of an enum and are simply objects by themselves. It turns out you can combine them into an enum if you want I think, but then you loose the nice Display features.

I'm adding this to the post above just to keep all options in one big comment.

@notmandatory
Copy link
Member

I prefer option 1 because it give apps access to the error data with which they can construct their own user facing errors, as long as we have good error docs that should be enough context for developers. If we go with option 3 we get the nice english error message strings, but those would need to be parsed somehow to get to the data so would be hard to localize and would be less usable in an app UI.

@reez
Copy link
Collaborator

reez commented May 30, 2024

I prefer option 1 because it give apps access to the error data with which they can construct their own user facing errors, as long as we have good error docs that should be enough context for developers. If we go with option 3 we get the nice english error message strings, but those would need to be parsed somehow to get to the data so would be hard to localize and would be less usable in an app UI.

I just added a comment to our ErrorADR a few mins ago that I think aligns with this general concept you're describing: #513 (comment) Libraries return errors and applications decide if and how those errors are formatted and displayed to users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants