[wasi-common] Use thiserror proc macros for auto From impls #758
Conversation
This commit refactors `wasi_common::error::Error` by using `#[from]` proc macro to autoderive `From` for wrapped errors.
crates/wasi-common/src/error.rs
Outdated
| @@ -97,38 +97,23 @@ impl WasiError { | |||
| impl fmt::Display for WasiError { | |||
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | |||
| match *self { | |||
| _ => write!(f, "{:?}", self), | |||
| _ => write!(f, "{}", self), | |||
There was a problem hiding this comment.
Can this whole fmt::Display impl be removed entirely now?
There was a problem hiding this comment.
We can get rid of it, but we still need WasiError to derive fmt::Display which however can be done more efficiently using #[error(..)] attribute. See 3b392db.
| @@ -97,38 +97,23 @@ impl WasiError { | |||
| impl fmt::Display for WasiError { | |||
| fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { | |||
| match *self { | |||
| _ => write!(f, "{:?}", self), | |||
| _ => write!(f, "{}", self), | |||
Agreed! Currently, whenever some unexpected error happens, all we get is the error code which can be quite tricky to "parse" mentally ;-) |
crates/wasi-common/src/error.rs
Outdated
| use thiserror::Error; | ||
|
|
||
| #[derive(Clone, Copy, Debug, Error, Eq, PartialEq)] | ||
| #[repr(u16)] | ||
| #[error("{}", self)] |
There was a problem hiding this comment.
This seems unlikely to work. You are implementing Display for WasiError by calling Display for WasiError.
#[test]
fn test_display_wasi_error() {
WasiError::ESUCCESS.to_string();
}thread 'error::test_display_wasi_error' has overflowed its stack
fatal runtime error: stack overflowDid you maybe mean #[error("{}", *self as __wasi_errno_t)] or #[error("{:?}", self)]?
There was a problem hiding this comment.
Ah, I missed that one, thanks for the pointer! Yeah, I definitely meant #[error("{}", *self as __wasi_errno_t)]. I'll provide a fix ASAP.
There was a problem hiding this comment.
Hmm, on second thought, I think I prefer the second option of #[error("{:?}", self)] since it displays error value that's closer to its actual string representation rather than displaying the raw error code; that is, println!("err={}", err)
- with
#[error("{}", *self as wasi::__wasi_errno_t)]produceserr=28, - whereas with
#[error("{:?}", self)], a nicer looking and easier to reason abouterr=EINVAL
|
👍 |
…alliance#758) * Use thiserror proc macros for auto From impls This commit refactors `wasi_common::error::Error` by using `#[from]` proc macro to autoderive `From` for wrapped errors. * Back port changes to snapshot0 * Auto impl Display for WasiError * Fix stack overflow when auto generating Display for WasiError
This commit refactors
wasi_common::error::Errorby using#[from]proc macro to autoderive
Fromfor wrapped errors.