frank2 / exe-rs

The PE Executable Library, but for Rust!
GNU General Public License v3.0
71 stars 13 forks source link

exe::Error doesnt implement std::error::Error #1

Closed polloloco closed 3 years ago

polloloco commented 3 years ago

Is there any particular reason why exe::Error doesnt implement std::error::Error (and thus std::fmt::Display)? If not, could you please add that?

frank2 commented 3 years ago

The reason is that I'm still technically new to Rust! I'll add it in, thanks for the report!

frank2 commented 3 years ago

Looking through what I'd need to do to implement this properly, this will probably result in a useful refactor of error codes (e.g., Error::InvalidRVA should also return the offending RVA)! Not sure how long the refactor will take, but I doubt it would be too long. For now to deal with the lack of Display formatting I usually just use debug printing for the error codes to figure out what's going on (i.e., println!("{:?}", error)).

polloloco commented 3 years ago

Right now I'm using lots of .map_err() to map to my own errors. Not having the std::error::Error impl is very annoying because it makes the code more explicit, especially when it comes to interop with other error types/crates. I've been using the thiserror crate for my errors and wanted to generate a From impl for your errors but thiserror refuses to do that because your errors don't implement std::error::Error yet I've been using the Debug impl as a workaround for printing already :smile:

frank2 commented 3 years ago

Thanks for the clarification, that's helpful to know!

frank2 commented 3 years ago

Okay, the trait is implemented for errors and errors are a bit more helpful! Thanks again for reporting!

polloloco commented 3 years ago

Did you somehow overwrite the 0.4.4 version on crates.io with your new code? I pulled my project on a different pc and it has exe = "0.4.4" in dependencies and now everything broke. I assume if I do cargo clean and then try compiling again on my pc it would break too :(

Edit: Nevermind, exe = "=0.4.4" works, I never had any crate that had "breaking" changes in a minor version update before 😄

frank2 commented 3 years ago

Yeah this will break things because of how much I modified the errors! I got like 100s of errors in exe-rs when I did that, haha! Glad you fixed it!