Result<(),MyCustomError> instead of anyhow?

[lattice0]

This library is very useful, but having to return a text error instead of a nice struct with custom error information would be much nicer! Is it possible? Is it in the plans?

[fzyzcjy]

Good idea! Feel free to make a PR :)

This should be possible.

flutter_rust_bridge/frb_rust/src/handler.rs

Line 320 in 9c57bf6

Rust2Dart::new(port).error(error.code().to_string(), error.message());

and

flutter_rust_bridge/frb_rust/src/handler.rs

Line 271 in 9c57bf6

ResultError(anyhow::Error),

[lattice0]

Errors are always translated as exceptions on Dart side, right?

I think the best here would be to return Future<Result<T, E>> where Result mimics Rust's Result. However I don't know if it's a good idea to handle errors like this in Dart as it's exception oriented. Maybe Result<T, E> in Rust converts to Future<T> and throws E as an exception? For example, each variant of E would be an Exception.

Currently with anyhow errors there is no way to catch specific exceptions on the Dart side, which is bad for apps that try to recover from errors (like mine).

[fzyzcjy]

I think the best here would be to return Future<Result<T, E>> where Result mimics Rust's Result. However I don't know if it's a good idea to handle errors like this in Dart as it's exception oriented.

Well I seldomly see dart code using Result<T,E>.

Maybe Result<T, E> in Rust converts to Future and throws E as an exception?

That sounds pretty reasonable.

Currently with anyhow errors there is no way to catch specific exceptions on the Dart side, which is bad for apps that try to recover from errors (like mine).

Totally agree. If we throw MyCustomException then it would be much greater to catch

[Desdaemon]

I think the best here would be to return Future<Result<T, E>> where Result mimics Rust's Result. However I don't know if it's a good idea to handle errors like this in Dart as it's exception oriented.

Well I seldomly see dart code using Result<T,E>.

Until we have custom exceptions, I think this might be the only way to preserve errors coming from Rust for now. What we could do is provide a hand-rolled Result in Dart like this:

@freezed
class Result<T, E> {
  const factory Result.ok(T value) = Ok;
  const factory Result.err(E err) = Err;
}

so we can have users return a hypothetical DartResult type to opt out of throwing exceptions:

pub type DartResult<T, E = ()> = Result<T, E>;

I'm not quite sure if this would be more work than supporting custom exceptions by itself.

[fzyzcjy]

I guess custom exceptions are not very hard to implement. Current implementation of "throwing a dart exception from rust exception" is nothing but "transfer a few strings (exception name, message, etc) to dart, and let dart side throw exception". So it may be easy to transfer an extra field indicating which exception is happening.

Or, we can just transfer a normal struct from rust to dart and utilize existing infra. It is only after it goes to the dart side that we throw it instead of return it.

[lattice0]

better to transfer the type cause errors could be nested with other errors, but I don't know how nesting would work on dart side

…

-------- Mensagem Original -------- Em 15 de jul. de 2022 00:58, fzyzcjy escreveu:

I guess custom exceptions are not very hard to implement. Current implementation of "throwing a dart exception from rust exception" is nothing but "transfer a few strings (exception name, message, etc) to dart, and let dart side throw exception". So it may be easy to transfer an extra field indicating which exception is happening. Or, we can just transfer a normal struct from rust to dart and utilize existing infra. It is only after it goes to the dart side that we throw it instead of return it. — Reply to this email directly, [view it on GitHub](#533 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/ABSTHAKYAWP75FFIC55CL7DVUDOVZANCNFSM52CHUSLQ). You are receiving this because you authored the thread.Message ID: ***@***.***>

[fzyzcjy]

cause errors could be nested with other errors

and any arbitrary field, etc :)

but I don't know how nesting would work on dart side

I guess nothing special. frb already supports nesting structs, and the error will be nothing but another struct.

[Desdaemon]

I have an idea: we can have a specialization for Result<T, E> where:

• E is not anyhow::Error; and
• E: IntoDart

Provided these conditions, we can have a new Dart class that extends FfiError and carries this extra error attribute in a type-safe manner.

[lattice0]

@Desdaemon are specializations like this already possible in Rust?

Anyways, I'm beggining to take a look on the error handler and how to do this

[fzyzcjy]

are specializations like this already possible in Rust?

I guess he means we can do such checking at our code generator.

[fzyzcjy]

Anyways, I'm beggining to take a look on the error handler and how to do this

Rough directions:

Firstly, errors are captured at https://github.com/fzyzcjy/flutter_rust_bridge/blob/master/frb_rust/src/handler.rs#L119 and naively handled at

flutter_rust_bridge/frb_rust/src/handler.rs

Line 320 in e841421

Rust2Dart::new(port).error(error.code().to_string(), error.message());

. Users (e.g. me in my internal app) may also modify that error handler and do extra things (e.g. logging / sentry).

Then comes the core error handling:

flutter_rust_bridge/frb_rust/src/rust2dart.rs

Line 46 in e841421

pub fn error_full(

.

The current approach is very very naive. Basically, when we want to return a value, or send a value via Stream, we indeed encode that rust value into some wire data, and then send something like vec![RUST2DART_ACTION_SUCCESS, the_real_result], i.e. vec![0, the_real_result]. When we want to send an error, we send vec![1, error_code, error_message, etc]. And in the Dart side we just simply check whether it is 0 or 1 in the first arg.

[fzyzcjy]

Therefore, the solution may be quite simple: Instead of posting vec![1, errorcode, errormessage] using Rust2Dart, we can simply posting vec![1, arbitrary_rust_object] as long as we use all the existing code to auto-generate converting code for the type of that arbitrary_rust_object.

Then, in Dart side, indeed almost nothing to do. Just mimic how we receive the normal result and deserialize it, we can deserialize the error data, and throw it.

One minor thing may be that, we may need to let our custom exception to implements Exception in dart, to make it more natural to be used in dart side.

[lattice0]

I've just read how Isolate posting works and now I understand it better. Yes, if arbitrary_rust_object implements IntoDart, it should work.

But here:

impl<E: Executor, EH: ErrorHandler> Handler for SimpleHandler<E, EH> {
    fn wrap<PrepareFn, TaskFn, TaskRet>(&self, wrap_info: WrapInfo, prepare: PrepareFn)
    where
        PrepareFn: FnOnce() -> TaskFn + UnwindSafe,
        TaskFn: FnOnce(TaskCallback) -> Result<TaskRet> + Send + UnwindSafe + 'static,
        TaskRet: IntoDart,
    {

the Result is a anyhow::Result. I think it needs to be changed to std lib's Result<TaskRet, E>, where E: IntoDart, and then something like this:

/// Errors that occur from normal code execution.
#[derive(Debug)]
pub enum Error {
    /// Errors from an [anyhow::Error].
    ResultError(anyhow::Error), // <------------------ PS: why not used?
    /// Exceptional errors from panicking.
    Panic(Box<dyn Any + Send>),
    /// Custom errors that implement `IntoDar`
    CustomError(Box<dyn IntoDart + Send>),
}

Then:

impl<E: Executor, EH: ErrorHandler> Handler for SimpleHandler<E, EH> {
    fn wrap<PrepareFn, TaskFn, TaskRet>(&self, wrap_info: WrapInfo, prepare: PrepareFn)
    where
        PrepareFn: FnOnce() -> TaskFn + UnwindSafe,
        TaskFn: FnOnce(TaskCallback) -> std::Result<TaskRet, E> + Send + UnwindSafe + 'static,
        TaskRet: IntoDart,
        E: IntoDart
    {
        let _ = panic::catch_unwind(move || {
            let wrap_info2 = wrap_info.clone();
            if let Err(error) = panic::catch_unwind(move || {
                let task = prepare();
                self.executor.execute(wrap_info2, task);
            }) {
                self.error_handler
                    .handle_error(wrap_info.port.unwrap(), Error::CustomError(error));
            }
        });
    }

Also we need

impl ErrorHandler for ReportDartErrorHandler {
    fn handle_error(&self, port: i64, error: Error) {
        Rust2Dart::new(port).custom_error(error.code().to_string(), error.message());
    }

    fn handle_error_sync(&self, error: Error) -> Vec<u8> {
        //...
    }
}

where

/// Send a custom error message back to the specified port.
    pub fn custom_error<T: IntoDart>(&self, error: T) -> bool {
        self.isolate.post(vec![
            RUST2DART_ACTION_CUSTOM_ERROR.into_dart(),
            error.into_dart(),
        ])
    }

and then do some deserialization on Dart for the new case RUST2DART_ACTION_CUSTOM_ERROR.

Is this approach ok?

[fzyzcjy]

the Result is a anyhow::Result. I think it needs to be changed to std lib's Result<TaskRet, E>, where E: IntoDart

Sure, feel free to change APIs as that is necessary.

But I guess you need impl IntoDart for anyhow::Error? Then maybe PR on upstream: https://github.com/sunshine-protocol/allo-isolate/blob/master/src/into_dart.rs, as IntoDart is not implemented in this crate.

ResultError(anyhow::Error), // <------------------ PS: why not used?

If we are doing CustomError, shall we remove this old error? Since we may directly let anyhow::Error be IntoDart and then it is nothing more special.

pub fn custom_error<T: IntoDart>(&self, error: T) -> bool {

and then do some deserialization on Dart for the new case RUST2DART_ACTION_CUSTOM_ERROR.

Similarly, what about make anyhow::Error nothing special, but just like any other errors.

[lattice0]

Just did this:

pub enum Error {
    /// Errors that implement [IntoDart].
    CustomError(Box<dyn IntoDart>),
    /// Exceptional errors from panicking.
    Panic(Box<dyn Any + Send>),
}

pub trait Executor: RefUnwindSafe {
    /// Executes a Rust function and transforms its return value into a Dart-compatible
    /// value, i.e. types that implement [`IntoDart`].
    fn execute<TaskFn, TaskRet, Er>(&self, wrap_info: WrapInfo, task: TaskFn)
    where
        TaskFn: FnOnce(TaskCallback) -> Result<TaskRet, Er> + Send + UnwindSafe + 'static,
        TaskRet: IntoDart,
        Er: IntoDart + 'static;
//...

and ran on the example

pub enum CustomError{
    Error1(String),
    Error2(u32),
    Error3(i32)
}

pub fn return_custom_error() -> Result<u32, CustomError> {
    Err(CustomError::Error2(3))
}

which generated

#[no_mangle]
pub extern "C" fn wire_return_custom_error(port_: i64) {
    FLUTTER_RUST_BRIDGE_HANDLER.wrap(
        WrapInfo {
            debug_name: "return_custom_error",
            port: Some(port_),
            mode: FfiCallMode::Normal,
        },
        move || move |task_callback| return_custom_error(),
    )
}

but the return type of return_custom_error, which is CustomError, does not implement IntoDart, because the Intermediate Representation ignores the Result<A,B> B portion of the thing and only represents the output A:

IrFuncOutput::ResultType(ty) => ty,

What do you think of creating another field on IrFunc:

pub struct IrFunc {
    pub name: String,
    pub inputs: Vec<IrField>,
    pub output: IrType,
    pub error_output: Option<IrType>, // <------- this
    pub fallible: bool,
    pub mode: IrFuncMode,
    pub comments: Vec<IrComment>,
}

so I can represent the output error? Then later I can visit it and generate its IntoDart thing.

[fzyzcjy]

What do you think of creating another field on IrFunc:

Sure. And then when traversing all types, treat error_output just almost the same as output and inputs.

[fzyzcjy]

pub error_output: Option<IrType>, // <------- this

pub fallible: bool,

Shall we remove fallible (and make it a method instead), since "error_output!=none" is equivalent to fallible==true?

[lattice0]

It's more complicated than we thought. Here, a Result<T, E> is converted into T:

pub fn try_from_syn_type(ty: &syn::Type) -> Option<Self> {
        match ty {
            syn::Type::Path(syn::TypePath { path, .. }) => {
                let last_segment = path.segments.last().unwrap().clone();
                match last_segment.arguments {
                    syn::PathArguments::None => Some(SupportedInnerType::Path(SupportedPathType {
                        ident: last_segment.ident,
                        generic: None,
                    })),
                    syn::PathArguments::AngleBracketed(a) => {

and I can only convert to one of these:

pub enum SupportedInnerType {
    /// Path types with up to 1 generic type argument on the final segment. All segments before
    /// the last segment are ignored. The generic type argument must also be a valid
    /// `SupportedInnerType`.
    Path(SupportedPathType),
    /// Array type
    Array(Box<Self>, usize),
    /// The unit type `()`.
    Unit,
}

the concept of Result is lost after this. I tried adding

pub enum SupportedInnerType {
    /// Path types with up to 1 generic type argument on the final segment. All segments before
    /// the last segment are ignored. The generic type argument must also be a valid
    /// `SupportedInnerType`.
    Path(SupportedPathType),
    /// Path types with up to n generic type argument on the final segment.
    /// The generic type argument must also be a valid `SupportedInnerType`.
    PathMultiple(Vec<SupportedPathType>),
    /// Array type
    Array(Box<Self>, usize),
    /// The unit type `()`.
    Unit,
}

but realized it does not make sense to have a PathMultiple, it could be anything: Path<A,B,C,...> would be converted to Vec<A,B,C,...> and we'd lose the Path info.

What should I do here?

Remember that this is just so the E in Result<T,E> gets a IntoDart implementation. If it weren't for that, we'd not need all of this.

[lattice0]

I'm trying with

#[derive(Debug)]
pub struct SupportedPathType {
    pub ident: syn::Ident,
    pub generic: Vec<Box<SupportedInnerType>>,
}

instead of

#[derive(Debug)]
pub struct SupportedPathType {
    pub ident: syn::Ident,
    pub generic: Option<Box<SupportedInnerType>>,
}

[lattice0]

Well,

#[derive(Debug)]
pub struct SupportedPathType {
    pub ident: syn::Ident,
    pub generic: Vec<Box<SupportedInnerType>>,
}

makes it really hard to

    /// Converts a path type into an `IrType` if possible.
    pub fn convert_path_to_ir_type(&mut self, p: SupportedPathType) -> Vec<IrType> {

I don't know what to do

[fzyzcjy]

What about storing error type in a separate field? Just like what you mentioned:

pub struct IrFunc {
    pub inputs: Vec<IrField>,
    pub output: IrType,
    pub error_output: Option<IrType>, // <------- this

Then, Result<T, E>'s "T" will go into the output, while "E" will go into the error_output.

[lattice0]

in order to get the error_output, I have to get the E from Result<T,E> here:

pub fn try_from_syn_type(ty: &syn::Type) -> Option<Self> {
        match ty {
            syn::Type::Path(syn::TypePath { path, .. }) => {
                let last_segment = path.segments.last().unwrap().clone();
                match last_segment.arguments {
                    syn::PathArguments::None => Some(SupportedInnerType::Path(SupportedPathType {
                        ident: last_segment.ident,
                        generic: None,
                    })),
                    // This part gets only the first argument, the T. To get the E, lots of work need to be done :(
                    syn::PathArguments::AngleBracketed(a) => {

[fzyzcjy]

Shall we do something like pub fn try_from_syn_type(ty: &syn::Type) -> (Option<Self>, Option<Self>) {

[lattice0]

I tried exactly that, but the recursiveness of try_from_syn_type makes it a hell to work with, if not impossible

[fzyzcjy]

Ah I see. What about

pub enum SupportedInnerType {
    Path(SupportedPathType), // unchanged
...
}

pub struct SupportedPathType {
    pub ident: syn::Ident,
    pub generic: Vec<Box<SupportedInnerType>>, // NOTE change from Option to Vec, so support >=2 generic args
}

[lattice0]

wow I tried exactly that afterwards: lattice0@6f5a6fb but it was really hard to create the vec for all parsed types, because now I should do vec![ty] for each one and etc. I rolled back after this

[fzyzcjy]

wow we have had the same thoughts :)

it was really hard to create the vec for all parsed types, because now I should do vec![ty] for each one and etc

Could you please elaborate a bit? If it is nothing but adding vec! to each usage I guess it is just a bit of boilerplate and is not very hard

[lattice0]

Because then convert_path_to_ir_type has to return a Vec, so collecting all the cases into vecs ("SyncReturn", "Vec", "ZeroCopyBuffer", "Box", "Option") and calling self.convert_to_ir_type(*generic) on a generic that is actually a Vec, would make me need to modify convert_to_ir_type as well, etc. It turned out making too many reds on my editor.

[fzyzcjy]

Hmm maybe we can use the only element from the Vec when appropriate?

[Desdaemon]

pub generic: Vec<Box<SupportedInnerType>> might be redundant since Vec already provides a layer of redirection, you might want to change it to Vec<SupportedInnerType> to make it easier to pattern-match on.

[lattice0]

#582

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new issue.

[fzyzcjy]

(Reopen given #1325)