5 ms·

There must be some downside to doing this? That is, why wouldn't the '?' operator already give us this kind of important detail?

vore 3y ago
This boxes all errors and erases the return type at compile time.
- ahahahahah 3y ago
  that's pretty much a non-factor. the cost here is that dumping a bunch of file names and line numbers to users is a shit experience.
  - vore 3y ago
    It is a huge factor: you can't pattern match the original error anymore because its type is erased. You can no longer distinguish between different variants of an error enum, so you have to handle any kind of error the same way, which is the real shit experience.
    - JoelJacobson 3y ago
      Not true, it's not "erased", it's wrapped. The original error is available via the `.inner` Wherr struct field.
      - vore 3y ago
        Consider this: #[derive(Debug)] enum MyError { A, B, C } impl std::fmt::Display for MyError { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!(f, "{:?}", self) } } impl std::error::Error for MyError {} // Equivalent to wherr::Wherr. #[derive(Debug)] struct ErrorWrapper { inner: Box<dyn std::error::Error> } impl std::fmt::Display for ErrorWrapper { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { self.inner.fmt(f) } } impl std::error::Error for ErrorWrapper {} fn my_error() -> Result<(), MyError> { Err(MyError::A)? } // Equivalent to applying #[wherr] to my_error. fn my_error_wrapped() -> Result<(), ErrorWrapper> { Err(ErrorWrapper { inner: Box::new(MyError::A) })? } fn main() { // Works. match my_error().unwrap_err() { MyError::A => { println!("got MyError::A"); } _ => { println!("got some other MyError variant"); } } // Doesn't work: how do I match out the inner MyError? match my_error_wrapped().unwrap_err().inner { MyError::A => { println!("got MyError::A"); } _ => { println!("got some other MyError variant"); } } }
        JoelJacobson 3y ago
        // This will work: downcast the inner error and then match on it. match my_error_wrapped().unwrap_err().inner.downcast_ref::<MyError>() { Some(MyError::A) => println!("got MyError::A"), Some(_) => println!("got some other MyError variant"), None => println!("got an unknown error type"), }
        vore 3y ago
        I think, at that point, you should rather make Wherr be `struct Wherr<E> { inner: E, ... }` to avoid having to do a dynamic cast - I don't personally like the idea of erasing guarantees of what types you're getting.
        JoelJacobson 3y ago
        That seems like a great idea. I've started a branch "attempt_to_avoid_dynamic_cast" where I've tried to implement your idea: https://github.com/joelonsql/wherr/commit/c5906a257fc9768ad0add455bbc89107d4e7d978 https://github.com/joelonsql/wherr/commit/c5906a257fc9768ad0... I couldn't figure out how to make it work though, since with these changes, now nested functions using the #[wherr] macro won't work due to "doesn't have a size known at compile-time". The other idea of just using #[track_caller] also seems promising, and will be considered for a future major version. But I still think it's interesting if the macro variant can be improved, to avoid having to do dynamic casts, hence this new branch.
    - deleted 3y ago
      [deleted]
    - JoelJacobson 3y ago
      It is a valid problem though, since if you have code that currently depend on the different error enum variants, then that code needs to be modified to match on the original error via the `.inner` field instead.
  - bacon_waffle 3y ago
    It's pretty nice that the `Result<>` and `?` pattern can be used in no-std Rust, without heap allocation.
    - richardwhiuk 3y ago
      This could be written without heap allocation.
      - bacon_waffle 3y ago
        Maybe so; I don't doubt you're correct, but offhand I can't think of how to do it. If you can see a solution that doesn't require boxing, that could make an interesting blog post, or PR to this crate!
dannymi 3y ago
Imagine that you try to open your washing machine while it is washing. Correctly, you will get an error that this is not possible because you would be drenched with water. In addition, with this crate, you will also get the information that this error was returned in washing_machine.rs:3562. Thanks, I guess? >That is, why wouldn't the '?' operator already give us this kind of important detail? It's just not very useful information to have, except for the original developer (when debugging)--and the latter is a minority use case. Rust doesn't make you pay for things you don't use. But for this feature Rust would need to add an integer (and possible alignment bytes) and a str reference every single time there is a "?" (there are a lot of those) and an error is constructed, also when not debugging. This crate requires you to add `#[wherr]` in order for it to change what `?` does--and you would only do that when debugging. That's nice.
- imetatroll 3y ago
  I don't have the opportunity (yet?) to develop professionally in Rust but in the other languages I use, I do tend to include filename + line number information in server logs so that I can immediately find where something went wrong. So I'm not questioning the why of adding these, but was wondering why '?' doesn't have a way to do this for us already.
- yazaddaruvala 3y ago
  Having the file and line number automatically added is quite useful for server logs - especially in production. Very, very few end up in front of an end user. Typically only in a CLI.
- JoelJacobson 3y ago
  Author of `wherr` here. I agree that you wouldn't always want to pay the extra cost for this information. However, I imagine real-world use-cases when it's preferable to pay that cost in production, allowing users to forward file and line information to the original developer via a bug report. Alternatively, indirectly, for instance via some company's customer support, that might ask the customer to include output from their program, which they in turn can let their developers analyse, who in turn can contact the developer of the library used, where the error originates from. Not having this information in production, means the original developer will need to try to figure out how to reproduce the error locally, which might be necessary anyway of course, but the effort will probably be less, given the file and line information. That said, I agree with you that the most likely use-case, is to enable this only when debugging, on a per-function basis, when needed.

vore 3y ago

This boxes all errors and erases the return type at compile time.

ahahahahah 3y ago
that's pretty much a non-factor. the cost here is that dumping a bunch of file names and line numbers to users is a shit experience.
- vore 3y ago
  It is a huge factor: you can't pattern match the original error anymore because its type is erased. You can no longer distinguish between different variants of an error enum, so you have to handle any kind of error the same way, which is the real shit experience.
  - JoelJacobson 3y ago
    Not true, it's not "erased", it's wrapped. The original error is available via the `.inner` Wherr struct field.
    - vore 3y ago
      Consider this: #[derive(Debug)] enum MyError { A, B, C } impl std::fmt::Display for MyError { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!(f, "{:?}", self) } } impl std::error::Error for MyError {} // Equivalent to wherr::Wherr. #[derive(Debug)] struct ErrorWrapper { inner: Box<dyn std::error::Error> } impl std::fmt::Display for ErrorWrapper { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { self.inner.fmt(f) } } impl std::error::Error for ErrorWrapper {} fn my_error() -> Result<(), MyError> { Err(MyError::A)? } // Equivalent to applying #[wherr] to my_error. fn my_error_wrapped() -> Result<(), ErrorWrapper> { Err(ErrorWrapper { inner: Box::new(MyError::A) })? } fn main() { // Works. match my_error().unwrap_err() { MyError::A => { println!("got MyError::A"); } _ => { println!("got some other MyError variant"); } } // Doesn't work: how do I match out the inner MyError? match my_error_wrapped().unwrap_err().inner { MyError::A => { println!("got MyError::A"); } _ => { println!("got some other MyError variant"); } } }
      - JoelJacobson 3y ago
        // This will work: downcast the inner error and then match on it. match my_error_wrapped().unwrap_err().inner.downcast_ref::<MyError>() { Some(MyError::A) => println!("got MyError::A"), Some(_) => println!("got some other MyError variant"), None => println!("got an unknown error type"), }
        vore 3y ago
        I think, at that point, you should rather make Wherr be `struct Wherr<E> { inner: E, ... }` to avoid having to do a dynamic cast - I don't personally like the idea of erasing guarantees of what types you're getting.
        JoelJacobson 3y ago
        That seems like a great idea. I've started a branch "attempt_to_avoid_dynamic_cast" where I've tried to implement your idea: https://github.com/joelonsql/wherr/commit/c5906a257fc9768ad0add455bbc89107d4e7d978 https://github.com/joelonsql/wherr/commit/c5906a257fc9768ad0... I couldn't figure out how to make it work though, since with these changes, now nested functions using the #[wherr] macro won't work due to "doesn't have a size known at compile-time". The other idea of just using #[track_caller] also seems promising, and will be considered for a future major version. But I still think it's interesting if the macro variant can be improved, to avoid having to do dynamic casts, hence this new branch.
  - deleted 3y ago
    [deleted]
  - JoelJacobson 3y ago
    It is a valid problem though, since if you have code that currently depend on the different error enum variants, then that code needs to be modified to match on the original error via the `.inner` field instead.
- bacon_waffle 3y ago
  It's pretty nice that the `Result<>` and `?` pattern can be used in no-std Rust, without heap allocation.
  - richardwhiuk 3y ago
    This could be written without heap allocation.
    - bacon_waffle 3y ago
      Maybe so; I don't doubt you're correct, but offhand I can't think of how to do it. If you can see a solution that doesn't require boxing, that could make an interesting blog post, or PR to this crate!

dannymi 3y ago

Imagine that you try to open your washing machine while it is washing. Correctly, you will get an error that this is not possible because you would be drenched with water. In addition, with this crate, you will also get the information that this error was returned in washing_machine.rs:3562. Thanks, I guess? >That is, why wouldn't the '?' operator already give us this kind of important detail? It's just not very useful information to have, except for the original developer (when debugging)--and the latter is a minority use case. Rust doesn't make you pay for things you don't use. But for this feature Rust would need to add an integer (and possible alignment bytes) and a str reference every single time there is a "?" (there are a lot of those) and an error is constructed, also when not debugging. This crate requires you to add `#[wherr]` in order for it to change what `?` does--and you would only do that when debugging. That's nice.

imetatroll 3y ago
I don't have the opportunity (yet?) to develop professionally in Rust but in the other languages I use, I do tend to include filename + line number information in server logs so that I can immediately find where something went wrong. So I'm not questioning the why of adding these, but was wondering why '?' doesn't have a way to do this for us already.
yazaddaruvala 3y ago
Having the file and line number automatically added is quite useful for server logs - especially in production. Very, very few end up in front of an end user. Typically only in a CLI.
JoelJacobson 3y ago
Author of `wherr` here. I agree that you wouldn't always want to pay the extra cost for this information. However, I imagine real-world use-cases when it's preferable to pay that cost in production, allowing users to forward file and line information to the original developer via a bug report. Alternatively, indirectly, for instance via some company's customer support, that might ask the customer to include output from their program, which they in turn can let their developers analyse, who in turn can contact the developer of the library used, where the error originates from. Not having this information in production, means the original developer will need to try to figure out how to reproduce the error locally, which might be necessary anyway of course, but the effort will probably be less, given the file and line information. That said, I agree with you that the most likely use-case, is to enable this only when debugging, on a per-function basis, when needed.