Provide decent error message for wrapped errors

[ivmarkov]

Provide decent error message for wrapped errors

[chungwong]

Just like the backtrace member, it should be enabled only when both std and backtrace features are enabled, as it would be bringing non-trivial cost to the unhappy path.

Does it mean this feature will not work on branch no_std? That is, running the no_std version of onoff_light will not have a nice error report.

[ivmarkov]

It will work on branch no_std. "std" is enabled on that branch by default as well.

[chungwong]

It will work on branch no_std. "std" is enabled on that branch by default as well.

Oh, I think I am confused. So the no_std branch is not actually no std(It does use the std library conditionally)

Given no_std branch has the std feature enabled by default, does it mean to be really no_std, it is necessary to turn off the std feature manually?

[ivmarkov]

Yeah. I don't see a big deal here. This is a standard pattern for a bunch of no_std-compatible crates.

[ssnover]

@ivmarkov what are your thoughts on an API like this to resolve this issue?

pub struct Error {
    code: ErrorCode,
    #[cfg(all(feature = "std", feature = "backtrace"))]
    backtrace: std::backtrace::Backtrace,
    #[cfg(feature = "std")]
    inner: Option<Box<dyn std::error::Error>>,
}

impl Error {
    pub fn new(code: ErrorCode) -> Self {
        Self {
            code,
            #[cfg(all(feature = "std", feature = "backtrace"))]
            backtrace: std::backtrace::Backtrace::capture(),
            #[cfg(feature = "std")]
            inner: None,
        }
    }

    #[cfg(feature = "std")]
    pub fn new_with_details(code: ErrorCode, detailed_err: Box<dyn std::error::Error>) -> Self {
        Self {
            code,
            #[cfg(all(feature = "std", feature = "backtrace"))]
            backtrace: std::backtrace::Backtrace::capture(),
            #[cfg(feature = "std")]
            inner: Some(detailed_err),
        }
    }

    #[cfg(feature = "std")]
    pub fn details(&self) -> Option<&dyn std::error::Error> {
        self.inner.as_ref().map(|err| err.as_ref())
    }
} // impl Error

#[cfg(feature = "std")]
impl From<std::io::Error> for Error {
    fn from(e: std::io::Error) -> Self {
        // Keep things simple for now
        Self::new_with_details(ErrorCode::StdIoError, Box::new(e))
    }
}

I would implement the above for any error type which implements std::error::Error but leave the current new method for no_std use cases where it might not be implemented for the concrete error type.

[ssnover]

With the above, I get a log like this:

[2023-08-17T23:57:44Z WARN  on_off_light] mDNS exited: Os { code: 98, kind: AddrInUse, message: "Address already in use" }

Generated by:

res = mdns_runner => {
    log::warn!("mDNS exited: {:?}", res.unwrap_err().details().unwrap());
},

[ivmarkov]

With the above, I get a log like this:

[2023-08-17T23:57:44Z WARN  on_off_light] mDNS exited: Os { code: 98, kind: AddrInUse, message: "Address already in use" }

Generated by:

res = mdns_runner => {
    log::warn!("mDNS exited: {:?}", res.unwrap_err().details().unwrap());
},

Would work, however I would also suggest:

#[cfg(all(feature = "std", feature = "backtrace"))]
inner: Option<Box<dyn std::error::Error>>,

I.e. the Error member should be protected with the backtrace feature in addition to std as its collection is non-trivial and needs allocations.

Additionally:

• The fmt::Display implementation of Error should be extended - when std and backtrace are enabled - to display the error message from the dyn std::error::Error so that you don't have to reach out to res.unwrap_err().details() every time
• The fmt::Debug implementation of Error should also be extended to capture the backtrace - if it exists - from the dyn std::error::Error - if possible. Not sure if this is possible with Rust stable as of yet and if the std::error::Error captures a backtrace

[ivmarkov]

Closing, because this is implemented since quite some time...