thewh1teagle / rookie

Load cookies from your web browsers
https://crates.io/crates/rookie
MIT License
204 stars 18 forks source link

[Feature Request]: Error size cannot be known at compilation time. #13

Closed ShayBox closed 1 year ago

ShayBox commented 1 year ago

Describe the feature

Currently the Error type used is Box<dyn Error> which cannot be known at compilation time, which prevents using the ? operator within a method that returns Result.

Using a crate such as thiserror or even anyhow if you'd rather not have strict Error types would solve this and make using this library easier.

error[E0277]: the size for values of type `dyn std::error::Error` cannot be known at compilation time
  --> src\lib.rs:29:44
   |
29 |         let cookies = rookie::load(domains)?;
   |                                            ^ doesn't have a size known at compile-time
   |
   = help: the trait `Sized` is not implemented for `dyn std::error::Error`
   = help: the following other types implement trait `FromResidual<R>`:
             <Result<T, F> as FromResidual<Yeet<E>>>
             <Result<T, F> as FromResidual<Result<Infallible, E>>>
   = note: required for `Box<dyn std::error::Error>` to implement `std::error::Error`
   = note: required for `anyhow::Error` to implement `From<Box<dyn std::error::Error>>`
   = note: required for `Result<SpotifyLyrics, anyhow::Error>` to implement `FromResidual<Result<Infallible, Box<dyn std::error::Error>>>`

error[E0277]: `dyn std::error::Error` cannot be sent between threads safely
   --> src\lib.rs:29:44
    |
29  |         let cookies = rookie::load(domains)?;
    |                                            ^ `dyn std::error::Error` cannot be sent between threads safely
    |
    = help: the trait `Send` is not implemented for `dyn std::error::Error`
    = help: the following other types implement trait `FromResidual<R>`:
              <Result<T, F> as FromResidual<Yeet<E>>>
              <Result<T, F> as FromResidual<Result<Infallible, E>>>
    = note: required for `Unique<dyn std::error::Error>` to implement `Send`
note: required because it appears within the type `Box<dyn Error>`
   --> C:\Users\Admin\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib/rustlib/src/rust\library\alloc\src\boxed.rs:195:12
    |
195 | pub struct Box<
    |            ^^^
    = note: required for `anyhow::Error` to implement `From<Box<dyn std::error::Error>>`
    = note: required for `Result<SpotifyLyrics, anyhow::Error>` to implement `FromResidual<Result<Infallible, Box<dyn std::error::Error>>>`

error[E0277]: `dyn std::error::Error` cannot be shared between threads safely
   --> src\lib.rs:29:44
    |
29  |         let cookies = rookie::load(domains)?;
    |                                            ^ `dyn std::error::Error` cannot be shared between threads safely
    |
    = help: the trait `Sync` is not implemented for `dyn std::error::Error`
    = help: the following other types implement trait `FromResidual<R>`:
              <Result<T, F> as FromResidual<Yeet<E>>>
              <Result<T, F> as FromResidual<Result<Infallible, E>>>
    = note: required for `Unique<dyn std::error::Error>` to implement `Sync`
note: required because it appears within the type `Box<dyn Error>`
   --> C:\Users\Admin\.rustup\toolchains\stable-x86_64-pc-windows-msvc\lib/rustlib/src/rust\library\alloc\src\boxed.rs:195:12
    |
195 | pub struct Box<
    |            ^^^
    = note: required for `anyhow::Error` to implement `From<Box<dyn std::error::Error>>`
    = note: required for `Result<SpotifyLyrics, anyhow::Error>` to implement `FromResidual<Result<Infallible, Box<dyn std::error::Error>>>`

For more information about this error, try `rustc --explain E0277`.
ShayBox commented 1 year ago

Unrelated, but you should use the log or tracing crates to print info/warn/error messages instead of prepending [WARN] so the final binary programs can subscribe/filter/modify the logs of libraries.

thewh1teagle commented 1 year ago

Thanks for letting me know! I agree that we need better logging. I will add the pretty_env_logger or just the log crate and more log entries because it's a good way to catch errors.

Regarding error types, I'm not sure if we should have strict error types or not, but we definitely need to change the type I used

ShayBox commented 1 year ago

I'm not too familiar with the log/env_logger/pretty_env_logger family of crates, as I use tracing/tracing_subscriber crates.

I assume env_logger/pretty_env_logger (tracing_subscriber) is meant to be used in the binary to configure logging, while log (tracing) is meant to be used in both to do the logging.

I usually use anyhow for everything, unless I think there's a need for multiple error types.

If there's a method that can error for multiple reasons and if there's a good chance the end user might need to know the difference for behavior or if they just need to know Ok/Err

thewh1teagle commented 1 year ago

Hi I changed all the errors to throw Result of anyhow, and also added logging using log. Can you try it before I publish?

You can add it to Cargo.toml and add the latest code this way:

[dependencies]
rookie = { git = "https://github.com/thewh1teagle/rookie" }
ShayBox commented 1 year ago

It works!

thewh1teagle commented 1 year ago

Great! added to latest version.