8 ms·

Enhance Rust errors with file and line details

120 points by JoelJacobson 3y ago

JoelJacobson 3y ago
Just released `wherr` v0.1.7! Now with optional `anyhow` support via feature flag. This lets you combine `anyhow`'s error handling with `wherr`'s file and line details seamlessly. Check out the example in `wherr/examples/anyhow.rs`.
Animats 3y ago
Ah, like automatically adding .context("msg") to Return values.
imetatroll 3y ago
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.
    - 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.
diarrhea 3y ago
Reminds me of `diags` from [0]. Side note: can we make 'file not found' errors that fail to mention what wasn't found illegal? What's the point of 'file not found', then keeping silent about the exact file? Seen it in various ecosystems now. [0]: https://www.lurklurk.org/effective-rust/macros.html#when-to-use-macros https://www.lurklurk.org/effective-rust/macros.html#when-to-...
- dathinab 3y ago
  I wonder if not having `what` comes from two things: 1. the underlying OS error not containing what 2. some edge cases in which you don't have a what, or at least not a clear one 3. paths, especially absolute paths, always being prone on leaking personal user information
- thristian 3y ago
  The Unix kernel API only returns a numeric error code, no additional information. Nine times out of ten¹ the file that wasn't found is the file that you just tried to open or rename or whatever, so a language runtime can make a copy of that and attach it to their error representation, but (a) that's more work so it gets shunted out of the MVP, and (b) it requires allocating memory to store the copy, and some languages (like Rust) do not want silent memory allocation as a default behaviour. ¹: The one time out of ten is things like a script with a #! line that refers to a file that doesn't exist. The file you asked for (the script) definitely exists, but you'll get a file-not-found error if you ask the kernel to run it. Modern versions of bash seem to detect this situation and display a more helpful error message, but this is specific to bash, not other shells or languages.
  - deleted 3y ago
    [deleted]
  - littlestymaar 3y ago
    > (b) it requires allocating memory to store the copy, and some languages (like Rust) do not want silent memory allocation as a default behaviour. There's no need for an allocation: you already have the path allocated somewhere since you're making the call. So you could just have a pointer to that path in the error. That would obviously be very dangerous in C, because then you'd have to be careful not to have your error outlive your allocated path, but in Rust you'd have a lifetime annotation on your Error guaranteeing that your Error does not outlive your allocated path.
dannymi 3y ago
Errors are for the regular case when the program detected that something wasn't allowed or set up right and the program did everything right (by returning an error). Why then do you need the rs source code file&line of the place where everything went right? What about all the other places where everything went right? Wanna log those, too? :) What will the end user do with rs source code file&line? I read the example on the page. What they want to do is return which argument of `add` the error was with. Well, then introduce an `AddError` (or I guess it could be a more generic `BinaryOperationError`) like this: `struct AddError { argument_1: Option<...>, argument_2: Option<...> }` and return that error from `add`. Then you can even have it return errors due to both arguments at the same time. One use case of the crate would be if you made a mistake and something actually shouldn't be an error but should be a panic, and you wanna find where it is in order to change it to a panic. So, debugging.
- IshKebab 3y ago
  > What will the end user do with rs source code file&line? Google it.
- JoelJacobson 3y ago
  Author of `wherr` here. The idea is to minimise the effort needed to debug programs that already make use of the `?` operator, when the underlying error doesn't already contain file and line information, where the error is possibly coming from code of of your own control. Sure, you could catch such errors and return a refined error type, but that's more code, which might be what you want in the end, but this is for the code that currently makes use of the simple `?` operator. Once the file and line where the error is returned has been identified (using `wherr`), it might be a good idea, like you suggest, to introduce a new error type or refine the existing one. But maybe the error is fine as it is, and you just needed to know where it came from, to fix some bug, that when fixed, will cause the error to not happen any longer. Basically, when feeling confident an error cannot happen, some people use `.unwrap()` or `.expect()`, which *do* reveal the file and line, but then the program also panics. This might be what you want, but if you're developing a library, you probably most times want to let the "user" (the developer using the library) decide what course of action to take, based on the error. However, that "user" might not be at all interested in the file and line information of where the error originates from in your library source code, since the user might not be capable of fixing the possible bug anyway. But maybe the user wants to file a bug report, and then it would be helpful to know the file and line information. > What about all the other places where everything went right? Wanna log those, too? :) If everything went right, the Ok(val) is simply returned as Ok(val) unchanged, see `wherr/src/lib.rs`: match result { Ok(val) => Ok(val), ...
dathinab 3y ago
Note that some things, like anyhow do create backtraces (if the feature is enabled) when they are created, including by `?` auto-converting into them and including chaining the backtrace from the error they where created from. Through also note that async code has ... not so nice backtraces, for now, even if you only care about the calling location.
- ahahahahah 3y ago
  anyhow's backtraces are from where the error is created. This isn't necessarily the same as the path through which it is propagated with "?". honestly, i think the former is probably more valuable, but i really don't know.
  - JoelJacobson 3y ago
    I like `anyhow`, it's a great! But it doesn't help if the existing error, wherever it is created, doesn't already use `anyhow`. For errors that you can't find, then adding the `#[wherr]` macro to the functions, could possibly be faster, than changing all lines of code where errors are created, to use `anyhow`, to find the error. `#[wherr]` is one line per function, whereas `anyhow`'s `.context()?` is one line per error. Also, a backtrace could be more expensive than just wrapping the error once with file and line information as `wherr` does, and the backtrace might be overkill, if only interested in finding the line of code where the error is created.
    - IshKebab 3y ago
      You don't need to use `.context()` to get the backtrace.
  - dathinab 3y ago
    The point I was trying to make is that when (and only when) `?` creates an anyhow error through automatic conversion from a different error it will add a backtrace at that point, additional to any backtrace the original error might have.
lewantmontreal 3y ago
I’ve always wondered why the backtrace just kind of stops midway to when you finally handle an error, instead of showing up to the `?` where the error happened. I’ve been using anyhow’s `with_context` to add helpful notes to each usage of `?` to get around it. I’ll have to check if this macro is an easier solution!
- JoelJacobson 3y ago
  Author of `wherr` here. I can totally relate to the situation you describe. That's exactly what happened to me, and why I created wherr. At first, I couldn't understand why I couldn't find the `?` line in the backtrace. `anyhow`'s `with_context` is great, but seems to require you to modify each line of code where errors are created, to find the error. With `wherr`, you only need to add one line of code, `#[wherr]`, per function.
  - vlovich123 3y ago
    That doesn’t seem like a meaningful DX improvement and seems more stylistic, no? Really we’d need rustc to support some kind of AOP technique that lets you inject code at every call site somehow. That way you write the extra context info in a single macro that gets applied transparently for you. That being said, SNAFU’s approach seems better in the interim. Just add a Location (and presumably there’s a backtrace option too) to your error type and it’s captured automatically without any macros.
    - vlovich123 3y ago
      Correction: seems like the Error crate is working to natively acquire support for reporting backtrace automatically [1]. Extending that to add line number support seems reasonable. That way you get an upgrade across all codebases without anyone needing to make any changes. https://github.com/rust-lang/rust/issues/53487 https://github.com/rust-lang/rust/issues/53487
    - JoelJacobson 3y ago
      I like the idea of not even having to write a line of code per function. Maybe `cargo fmt` could be extended to add such annotations automatically? That way, it wouldn't be "magic", since the macros would be spelled out in the code, but the manual work of adding them would be eliminated.
olih_ 3y ago
This looks great! Is there a way to enable this automatically for a whole crate, without needing to set it per function?
sidit77 3y ago
Any reason why you implemented your own proc macro instead of just using std::panic::Location::caller()? Right now I don't see the advantage of this crate.
- ryukoposting 3y ago
  Panic location will only tell you where your program choked on an error, not where that error actually came from- there can be a very big difference when you have a long chain of functions returning Result<_, Box<dyn Error>>
  - sidit77 3y ago
    You can just pass the location up the stack alongside the type erased error. #[derive(Debug)] pub struct LocatedError { inner: Box<dyn Error + 'static>, location: &'static Location<'static> } impl<E: Error + 'static> From<E> for LocatedError { #[track_caller] fn from(value: E) -> Self { Self { inner: Box::new(value), location: Location::caller(), } } }
    - ryukoposting 3y ago
      Sure, but I think the point of this crate is that it removes this boilerplate.
tasn 3y ago
We opted for a more manual approach, we have a ctx!() macro[1] we use for wrapping errors we want to enrich that we then use like this[2]: ctx!(some_fallible_fund(foo))? I wodner if anyone is doing anything better? The nice thing about our approach is that we store the data on each error, so we get a full backtrace, even when using async. 1: https://github.com/svix/svix-webhooks/blob/main/server/svix-server/src/error.rs#L168 https://github.com/svix/svix-webhooks/blob/main/server/svix-... 2: https://github.com/svix/svix-webhooks/blob/main/server/svix-server/src/v1/endpoints/application.rs#L258 https://github.com/svix/svix-webhooks/blob/main/server/svix-...
- shepmaster 3y ago
  In SNAFU, you can add a Location [0] type to your error which will automatically capture that information when you add context to your error [0] https://docs.rs/snafu/latest/snafu/struct.Location.html https://docs.rs/snafu/latest/snafu/struct.Location.html
  - tasn 3y ago
    We have a similar structure in our code. How does snafu get filename and line number when adding context? Context usually added like this: `some_error.context("bloop")`, no? Which can't get the line info, that's why we used a macro.
    - shepmaster 3y ago
      You will want to use `Location::caller` [0] instead of the `line` macro (and friends). You will need to annotate enough places with `#[track_caller]` so the location is correct. [0]: https://doc.rust-lang.org/std/panic/struct.Location.html#method.caller https://doc.rust-lang.org/std/panic/struct.Location.html#met...
      - tasn 3y ago
        Ah, gotcha! We are actually going to try what the sibling comment recommended. https://news.ycombinator.com/item?id=37235165 https://news.ycombinator.com/item?id=37235165
afdbcreid 3y ago
I don't like having to wrap my function with a macro, with all disadvantages (e.g. worse IDE support), just to get error location. Especially given we can do this without macros at all: use std::error::Error as StdError; use std::panic::Location; use std::fmt; #[derive(Debug)] pub struct MyError { pub error: Box<dyn StdError + Send + Sync>, pub location: &'static Location<'static>, } impl<E: StdError + Send + Sync + 'static> From<E> for MyError { #[track_caller] #[inline] fn from(error: E) -> Self { Self { error: Box::new(error), location: Location::caller(), } } } impl fmt::Display for MyError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}, {}", self.error, self.location) } } We can also optimize this more (e.g. a simple optimization to be two words instead of three: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2a46e8a71103b4a2f432defc320dc06a https://play.rust-lang.org/?version=stable&mode=debug&editio.... Also allows opting `Send` and `Sync` out or using non-`'static` lifetimes), but this works as a baseline. The only disadvantage is that if called in a function with `#[track_caller]` this will report the location of the caller... But this is something I can live with.
- insanitybit 3y ago
  I think the disadvantage you mention is actually an advantage. I would probably want this to propagate alongside other `track_caller`'s since anyone using `track_caller` is "in the know" about where that error should be appearing to come from. TBH I had assumed this crate was using the `#[track_caller]` attribute. The main downside I see is that if I manually construct the error type I'll miss that place, unless the constructor of that error also tracks the caller.
  - JoelJacobson 3y ago
    Author of `wherr` here. > I think the disadvantage you mention is actually an advantage. I would probably want this to propagate alongside other `track_caller`'s since anyone using `track_caller` is "in the know" about where that error should be appearing to come from. I've now read about `#[track_caller]`, and that's my conclusion too. For those who haven't heard about `#[track_caller]` like myself, I can recommend this page: https://blog.rust-lang.org/2020/08/27/Rust-1.46.0.html#track_caller https://blog.rust-lang.org/2020/08/27/Rust-1.46.0.html#track... > TBH I had assumed this crate was using the `#[track_caller]` attribute. TBH, having read about the `#[track_caller]` attribute, I'm starting to think the crate should use it. If you and/or @afdbcreid would be interested in working with me on a new implementation of the `wherr` crate that uses the `#[track_caller]` approach instead, please contact me at joel at compiler dot org. Hope to hear from you! > The main downside I see is that if I manually construct the error type I'll miss that place, unless the constructor of that error also tracks the caller. You mean, we would need to make sure the error type is always created via a new() constructor method, and that the new() method also has the `#[track_caller]` attribute? Something like this? impl MyError { #[track_caller] pub fn new<E: StdError + Send + Sync + 'static>(error: E) -> Self { Self { error: Box::new(error), location: Location::caller(), } } }

120 points by JoelJacobson 3y ago

Just released `wherr` v0.1.7! Now with optional `anyhow` support via feature flag. This lets you combine `anyhow`'s error handling with `wherr`'s file and line details seamlessly. Check out the example in `wherr/examples/anyhow.rs`.

Animats 3y ago

Ah, like automatically adding .context("msg") to Return values.

imetatroll 3y ago

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.
  - 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.

diarrhea 3y ago

Reminds me of `diags` from [0]. Side note: can we make 'file not found' errors that fail to mention what wasn't found illegal? What's the point of 'file not found', then keeping silent about the exact file? Seen it in various ecosystems now. [0]: https://www.lurklurk.org/effective-rust/macros.html#when-to-use-macros https://www.lurklurk.org/effective-rust/macros.html#when-to-...

dathinab 3y ago
I wonder if not having `what` comes from two things: 1. the underlying OS error not containing what 2. some edge cases in which you don't have a what, or at least not a clear one 3. paths, especially absolute paths, always being prone on leaking personal user information
thristian 3y ago
The Unix kernel API only returns a numeric error code, no additional information. Nine times out of ten¹ the file that wasn't found is the file that you just tried to open or rename or whatever, so a language runtime can make a copy of that and attach it to their error representation, but (a) that's more work so it gets shunted out of the MVP, and (b) it requires allocating memory to store the copy, and some languages (like Rust) do not want silent memory allocation as a default behaviour. ¹: The one time out of ten is things like a script with a #! line that refers to a file that doesn't exist. The file you asked for (the script) definitely exists, but you'll get a file-not-found error if you ask the kernel to run it. Modern versions of bash seem to detect this situation and display a more helpful error message, but this is specific to bash, not other shells or languages.
- deleted 3y ago
  [deleted]
- littlestymaar 3y ago
  > (b) it requires allocating memory to store the copy, and some languages (like Rust) do not want silent memory allocation as a default behaviour. There's no need for an allocation: you already have the path allocated somewhere since you're making the call. So you could just have a pointer to that path in the error. That would obviously be very dangerous in C, because then you'd have to be careful not to have your error outlive your allocated path, but in Rust you'd have a lifetime annotation on your Error guaranteeing that your Error does not outlive your allocated path.

dannymi 3y ago

Errors are for the regular case when the program detected that something wasn't allowed or set up right and the program did everything right (by returning an error). Why then do you need the rs source code file&line of the place where everything went right? What about all the other places where everything went right? Wanna log those, too? :) What will the end user do with rs source code file&line? I read the example on the page. What they want to do is return which argument of `add` the error was with. Well, then introduce an `AddError` (or I guess it could be a more generic `BinaryOperationError`) like this: `struct AddError { argument_1: Option<...>, argument_2: Option<...> }` and return that error from `add`. Then you can even have it return errors due to both arguments at the same time. One use case of the crate would be if you made a mistake and something actually shouldn't be an error but should be a panic, and you wanna find where it is in order to change it to a panic. So, debugging.

IshKebab 3y ago
> What will the end user do with rs source code file&line? Google it.
JoelJacobson 3y ago
Author of `wherr` here. The idea is to minimise the effort needed to debug programs that already make use of the `?` operator, when the underlying error doesn't already contain file and line information, where the error is possibly coming from code of of your own control. Sure, you could catch such errors and return a refined error type, but that's more code, which might be what you want in the end, but this is for the code that currently makes use of the simple `?` operator. Once the file and line where the error is returned has been identified (using `wherr`), it might be a good idea, like you suggest, to introduce a new error type or refine the existing one. But maybe the error is fine as it is, and you just needed to know where it came from, to fix some bug, that when fixed, will cause the error to not happen any longer. Basically, when feeling confident an error cannot happen, some people use `.unwrap()` or `.expect()`, which *do* reveal the file and line, but then the program also panics. This might be what you want, but if you're developing a library, you probably most times want to let the "user" (the developer using the library) decide what course of action to take, based on the error. However, that "user" might not be at all interested in the file and line information of where the error originates from in your library source code, since the user might not be capable of fixing the possible bug anyway. But maybe the user wants to file a bug report, and then it would be helpful to know the file and line information. > What about all the other places where everything went right? Wanna log those, too? :) If everything went right, the Ok(val) is simply returned as Ok(val) unchanged, see `wherr/src/lib.rs`: match result { Ok(val) => Ok(val), ...

dathinab 3y ago

Note that some things, like anyhow do create backtraces (if the feature is enabled) when they are created, including by `?` auto-converting into them and including chaining the backtrace from the error they where created from. Through also note that async code has ... not so nice backtraces, for now, even if you only care about the calling location.

ahahahahah 3y ago
anyhow's backtraces are from where the error is created. This isn't necessarily the same as the path through which it is propagated with "?". honestly, i think the former is probably more valuable, but i really don't know.
- JoelJacobson 3y ago
  I like `anyhow`, it's a great! But it doesn't help if the existing error, wherever it is created, doesn't already use `anyhow`. For errors that you can't find, then adding the `#[wherr]` macro to the functions, could possibly be faster, than changing all lines of code where errors are created, to use `anyhow`, to find the error. `#[wherr]` is one line per function, whereas `anyhow`'s `.context()?` is one line per error. Also, a backtrace could be more expensive than just wrapping the error once with file and line information as `wherr` does, and the backtrace might be overkill, if only interested in finding the line of code where the error is created.
  - IshKebab 3y ago
    You don't need to use `.context()` to get the backtrace.
- dathinab 3y ago
  The point I was trying to make is that when (and only when) `?` creates an anyhow error through automatic conversion from a different error it will add a backtrace at that point, additional to any backtrace the original error might have.

lewantmontreal 3y ago

I’ve always wondered why the backtrace just kind of stops midway to when you finally handle an error, instead of showing up to the `?` where the error happened. I’ve been using anyhow’s `with_context` to add helpful notes to each usage of `?` to get around it. I’ll have to check if this macro is an easier solution!

JoelJacobson 3y ago
Author of `wherr` here. I can totally relate to the situation you describe. That's exactly what happened to me, and why I created wherr. At first, I couldn't understand why I couldn't find the `?` line in the backtrace. `anyhow`'s `with_context` is great, but seems to require you to modify each line of code where errors are created, to find the error. With `wherr`, you only need to add one line of code, `#[wherr]`, per function.
- vlovich123 3y ago
  That doesn’t seem like a meaningful DX improvement and seems more stylistic, no? Really we’d need rustc to support some kind of AOP technique that lets you inject code at every call site somehow. That way you write the extra context info in a single macro that gets applied transparently for you. That being said, SNAFU’s approach seems better in the interim. Just add a Location (and presumably there’s a backtrace option too) to your error type and it’s captured automatically without any macros.
  - vlovich123 3y ago
    Correction: seems like the Error crate is working to natively acquire support for reporting backtrace automatically [1]. Extending that to add line number support seems reasonable. That way you get an upgrade across all codebases without anyone needing to make any changes. https://github.com/rust-lang/rust/issues/53487 https://github.com/rust-lang/rust/issues/53487
  - JoelJacobson 3y ago
    I like the idea of not even having to write a line of code per function. Maybe `cargo fmt` could be extended to add such annotations automatically? That way, it wouldn't be "magic", since the macros would be spelled out in the code, but the manual work of adding them would be eliminated.

olih_ 3y ago

This looks great! Is there a way to enable this automatically for a whole crate, without needing to set it per function?

sidit77 3y ago

Any reason why you implemented your own proc macro instead of just using std::panic::Location::caller()? Right now I don't see the advantage of this crate.

ryukoposting 3y ago
Panic location will only tell you where your program choked on an error, not where that error actually came from- there can be a very big difference when you have a long chain of functions returning Result<_, Box<dyn Error>>
- sidit77 3y ago
  You can just pass the location up the stack alongside the type erased error. #[derive(Debug)] pub struct LocatedError { inner: Box<dyn Error + 'static>, location: &'static Location<'static> } impl<E: Error + 'static> From<E> for LocatedError { #[track_caller] fn from(value: E) -> Self { Self { inner: Box::new(value), location: Location::caller(), } } }
  - ryukoposting 3y ago
    Sure, but I think the point of this crate is that it removes this boilerplate.

tasn 3y ago

We opted for a more manual approach, we have a ctx!() macro[1] we use for wrapping errors we want to enrich that we then use like this[2]: ctx!(some_fallible_fund(foo))? I wodner if anyone is doing anything better? The nice thing about our approach is that we store the data on each error, so we get a full backtrace, even when using async. 1: https://github.com/svix/svix-webhooks/blob/main/server/svix-server/src/error.rs#L168 https://github.com/svix/svix-webhooks/blob/main/server/svix-... 2: https://github.com/svix/svix-webhooks/blob/main/server/svix-server/src/v1/endpoints/application.rs#L258 https://github.com/svix/svix-webhooks/blob/main/server/svix-...

shepmaster 3y ago
In SNAFU, you can add a Location [0] type to your error which will automatically capture that information when you add context to your error [0] https://docs.rs/snafu/latest/snafu/struct.Location.html https://docs.rs/snafu/latest/snafu/struct.Location.html
- tasn 3y ago
  We have a similar structure in our code. How does snafu get filename and line number when adding context? Context usually added like this: `some_error.context("bloop")`, no? Which can't get the line info, that's why we used a macro.
  - shepmaster 3y ago
    You will want to use `Location::caller` [0] instead of the `line` macro (and friends). You will need to annotate enough places with `#[track_caller]` so the location is correct. [0]: https://doc.rust-lang.org/std/panic/struct.Location.html#method.caller https://doc.rust-lang.org/std/panic/struct.Location.html#met...
    - tasn 3y ago
      Ah, gotcha! We are actually going to try what the sibling comment recommended. https://news.ycombinator.com/item?id=37235165 https://news.ycombinator.com/item?id=37235165

afdbcreid 3y ago

I don't like having to wrap my function with a macro, with all disadvantages (e.g. worse IDE support), just to get error location. Especially given we can do this without macros at all: use std::error::Error as StdError; use std::panic::Location; use std::fmt; #[derive(Debug)] pub struct MyError { pub error: Box<dyn StdError + Send + Sync>, pub location: &'static Location<'static>, } impl<E: StdError + Send + Sync + 'static> From<E> for MyError { #[track_caller] #[inline] fn from(error: E) -> Self { Self { error: Box::new(error), location: Location::caller(), } } } impl fmt::Display for MyError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}, {}", self.error, self.location) } } We can also optimize this more (e.g. a simple optimization to be two words instead of three: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=2a46e8a71103b4a2f432defc320dc06a https://play.rust-lang.org/?version=stable&mode=debug&editio.... Also allows opting `Send` and `Sync` out or using non-`'static` lifetimes), but this works as a baseline. The only disadvantage is that if called in a function with `#[track_caller]` this will report the location of the caller... But this is something I can live with.

insanitybit 3y ago
I think the disadvantage you mention is actually an advantage. I would probably want this to propagate alongside other `track_caller`'s since anyone using `track_caller` is "in the know" about where that error should be appearing to come from. TBH I had assumed this crate was using the `#[track_caller]` attribute. The main downside I see is that if I manually construct the error type I'll miss that place, unless the constructor of that error also tracks the caller.
- JoelJacobson 3y ago
  Author of `wherr` here. > I think the disadvantage you mention is actually an advantage. I would probably want this to propagate alongside other `track_caller`'s since anyone using `track_caller` is "in the know" about where that error should be appearing to come from. I've now read about `#[track_caller]`, and that's my conclusion too. For those who haven't heard about `#[track_caller]` like myself, I can recommend this page: https://blog.rust-lang.org/2020/08/27/Rust-1.46.0.html#track_caller https://blog.rust-lang.org/2020/08/27/Rust-1.46.0.html#track... > TBH I had assumed this crate was using the `#[track_caller]` attribute. TBH, having read about the `#[track_caller]` attribute, I'm starting to think the crate should use it. If you and/or @afdbcreid would be interested in working with me on a new implementation of the `wherr` crate that uses the `#[track_caller]` approach instead, please contact me at joel at compiler dot org. Hope to hear from you! > The main downside I see is that if I manually construct the error type I'll miss that place, unless the constructor of that error also tracks the caller. You mean, we would need to make sure the error type is always created via a new() constructor method, and that the new() method also has the `#[track_caller]` attribute? Something like this? impl MyError { #[track_caller] pub fn new<E: StdError + Send + Sync + 'static>(error: E) -> Self { Self { error: Box::new(error), location: Location::caller(), } } }