constantoine/totp-rs

TotpUrlError does not implement std::error::Error

Closed this issue · 2 comments

tmpfs commented

Which means it can't be used with libraries like anyhow.

I would recommend using thiserror for your error types, I also noticed that you return Box<dyn Error> in get_qr() which means it can't be used in multi-threaded environments as it is not Send.

This means I need to unwrap() where ordinarily I would just use ?.

Happy to submit a PR if you are interested in fixing the ergonomics for error handling.

Hi!

Since TotpUrlError and Rfc6238Error already implements Display, and have derive Debug, just adding impl std::error::Error for Rfc6238Error {} and impl std::error::Error for TotpUrlError {} does the trick.

As for the Box, I will simply return a string using the to_string() of the two possible error types

Does PR #42 seem satisfactory?

tmpfs commented

Sure, thats fine to do it like that 👍