[Rust] return Error instead of panic when deserialize/serialize failed

[chaokunyang]

Feature Request

Currently fory rust use expect in many places, if the expect doesn't succed, the program will panic. We should return use Result/Error as return value instead if panic directly. And we can shortpath the codeflow by ?

This is necessary, especially when deserialization can come from untrusted source.

Is your feature request related to a problem? Please describe

No response

Describe the solution you'd like

No response

Describe alternatives you've considered

No response

Additional context

No response

[urlyy]

@chaokunyang sorry, too busy right now, please let someone else to address this issue.

[urlyy]

btw, this is a small suggestion I have for refactoring error.rs. This kind of refactoring can reduce the usage of anyhow in the code, and using crate::error::Error and crate::{bail, ensure} instead of an external library feels more conformable.

use std::borrow::Cow;

use thiserror::Error;

#[derive(Error, Debug)]
#[non_exhaustive]
pub enum Error {
    // not use this, just as a example.
    // You can create new error enums for specific cases in the future.
    #[error("Fory on Rust not support Ref type")]
    Ref,

    // don't use this directly, use Error::from() instead
    #[error(transparent)]
    Wrapped(#[from] anyhow::Error),

    // don't use this directly, use Error::msg(str or String) instead
    #[error("{0}")]
    Msg(Cow<'static, str>),
}

impl Error {
    pub fn msg<S: Into<Cow<'static, str>>>(s: S) -> Self {
        Error::Msg(s.into())
    }
}

#[macro_export]
macro_rules! ensure {
    ($cond:expr, $msg:literal) => {
        if !$cond {
            return Err($crate::error::Error::msg($msg));
        }
    };
    ($cond:expr, $err:expr) => {
        if !$cond {
            return Err($crate::error::Error::Wrapped($err));
        }
    };
    ($cond:expr, $fmt:expr, $($arg:tt)*) => {
        if !$cond {
            return Err($crate::error::Error::msg(format!($fmt, $($arg)*)));
        }
    };
}

#[macro_export]
macro_rules! bail {
    ($err:expr) => {
        return Err($crate::error::Error::msg($err))
    };
    ($fmt:expr, $($arg:tt)*) => {
        return Err($crate::error::Error::msg(format!($fmt, $($arg)*)))
    };
}

you can:

use fory_core::error::Error;

Error::msg(format!(
    "TypeId {:?} not found in type_info_map, maybe you forgot to register some types",
    type_id
))

Error::msg("named_ext type must be registered in both peers")

Error::from(other_error)

Error::Ref

The usage of bail! and ensure! is the same as anyhow::{bail, ensure}.

It's also useful if we implement a crate::error:Result like anyhow:

use anyhow::{Result, bail};

fn do_something(x: i32) -> Result<()> {
    if x < 0 {
        bail!("x is smaller than 0, current value: {}", x);
    }
    println!("x = {}", x);
    Ok(())
}

[urlyy]

btw, I found that this issue will change Serializer api once again. Like fn fory_write_type_info will return Result<(),Error>, or fory_type_id_dyn will return Result<u32,Error>. This will have a significant impact.

[urlyy]

It’s important to determine which parts of the code can safely panic.

There are many unwrap()/expect() calls in the code—some occur during the initialization phase, some during the registration phase, and some during serialization/deserialization.

Some are better left to panic, while others should return an Error so the user can handle them.

Since this change is quite significant, it’s best to clarify this first.