Enhance Rust errors with file and line details
crates.io
crates.io
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&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.It seems to be a bit faster: running 2 tests test tests::benchmark_location_caller ... bench: 15 ns/iter (+/- 0) test tests::benchmark_wherr ... bench: 18 ns/iter (+/- 1)
I also like that it's more explicit; the actual error type is then spelled out, instead of being magically wrapped by the macro, that's nice!
Sounds like a better approach overall, but I'm curious to understand more about the disadvantage you mention, the problem with using `#[track_caller]`, that then masquerades the actually error line number. For my own code, I can of course control that and just ensure I don't make use of `#[track_caller]` anywhere in my code. But if other crates would be making use of `#[track_caller]`, that would prevent getting the correct error line.
Can anything be said about how common it is to use `#[track_caller]` in crates? If very uncommon, then I can live with it. But it would be annoying if it works sometimes and other times not.
Your approach is indeed elegant, and leveraging `#[track_caller]` is quite idiomatic. I also see the value in opting out of macros when possible.
However, I'm pondering the error matching aspect. Using your `MyErrorInner` design, one might want to downcast and match on specific error types:
if let Some(specific_err) = err.downcast_ref::<SpecificErrorType>() {
match specific_err {
SpecificErrorType::Variant => { /* ... */ },
// ... other matches
}
}
But this raises a question: Given that `MyErrorInner` holds a concrete `E` (which is, at runtime, a specific error type) and the public API exposes a boxed trait object, how would we effectively match against inner error types without resorting to static lifetimes?In your design, while `location` is static, the concrete error type `E` is hidden away. Exposing it, especially without requiring static lifetimes, could be a challenge. Your thoughts?
Always a pleasure to dive into these Rust intricacies. Thanks for taking the time to contribute to this discussion!
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.
> 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...
> 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(),
}
}
}The code I posted is minimal: I would also add `From` impls for not `Send + Sync`, and maybe other features of popular error crates: the two most important that come to mind is downcasting and more importantly, adding a context to an error before propagating it. Downcasting requires unsafe code though, I think.
I actually have another nice new idea about contexts, so I may actually publish my own crate... If you don't do that before me. If you do, maybe I'll contribute :)
`src/lib.rs` is your code copy/pasted, with the addition of `impl<'a, E: StdError + 'a> From<E> for Box<dyn MyError + 'a>` like suggested (and also just `Send`).
I've also added a `fn inner_error<'a>(&'a self) -> &'a (dyn StdError + 'a);`, but not sure this is the proper way.
I can't wrap my head around how to match on the inner error though, see `examples/matching_inner.rs`:
match my_error_wrapped().unwrap_err() {
_ => todo!(),
}
Can you figure it out?Exactly. Although I think because `T: Impl Into<T>` it may not be necessary.
fn add(s1: &str, s2: &str) -> Result<i64, MyError> {
...
let i1 = i64::from_str_radix(s1, radix)?;
...
}
Thanks!The usage of the error type the OP wrote is to get the line number of any error without using a macro on top of every single function in our crate.
The person who wrote `MyError` accomplished it by defining their error type as "something that can be constructed from any other error" (`impl From<E: StdErr> for MyError`) and additionally heap-allocated the original error with `Box::new(error)`.
So the usage would be to write one single error type in a crate which can "capture" or be constructed from any error type `E` and also prints out the line number of that error via `impl Display for MyError`.
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-...
¹: 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.
More generally, the interpreter doesn't exist. For a script with a shebang that's whatever is under the shebang. For executables it's whatever is in the PT_INTERP header in the binary (in ELF, or whatever the equivalent is for PE/Mach-O).
For example you might have an ELF binary with PT_INTERP set to /usr/lib/ld-linux.so when it's actually /lib/x86_64-unknown-linux-gnu/ld-linux.so. You'll get a "exec: File not found" error, even if all the files you think you need exist. The loader itself is what's missing.
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.
This has implications that are obviously undesirable, like prohibiting returning errors borrowing file paths allocated on the stack.
In your particular example you could memcopy the path in the Error, without allocating at all (but you may also want to box the path instead if the path made the enum size too big).
Right making the standard library's error type far less useful if you have to start writing your own error types lol. That's such an obnoxious design. The way it is now you can write programs that return and handle io errors just fine when they don't care about the path or other arguments. Your suggestion would require those programs to make a dedicated error type to unabstract away what is essentially a thin wrapper around errno. The value of a `std::io::Error` is still obviously appealing.
That's a really heavy constraint that doesn't work in practice - errors usually bubble up past the scope of the arguments of whatever failed.
- discard the file path
- allocate
This way, you control the performance/UX trade-off. And it doesn't require much work: it's just a From implementation to add to your error enum.
[1] only higher than the actual owned path actually.
Maybe some day Rust will support multiple versions of the stdlib (in a compatible way, like what's done with editions) but until then we'll have to work with this half-baked corner of the language.
I've learned to immediately suspect disk exhaustion when I'm experiencing unexplained failures, but in the past it's taken me forever to figure it out.
It also increases the surface area of the API which makes everything around it less portable and may mean it’s awkward to mock in tests.
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
It's basically libc's behavior so any ecosystem that just offers a thin layer on top of libc for file access without thinking about it too much is going to do it this way.
Through also note that async code has ... not so nice backtraces, for now, even if you only care about the calling location.
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.
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-...
2: https://github.com/svix/svix-webhooks/blob/main/server/svix-...
[0]: https://doc.rust-lang.org/std/panic/struct.Location.html#met...
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.
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.
Very, very few end up in front of an end user. Typically only in a CLI.
Yes, that's awesome. I can either dig into the code myself or at least forward it to the project and get a great pinpointed report (at least some of the time). Much better than searching for strings and trying to guess what was parametrized.
This is definitely something that you would want in production/in customer deployments. The only reason not to want this is extremely tight optimization scenarios, where paying the cost of the larger error type is too much (or if you're returning errors on some hot path).
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.
#[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"); }
}
} // 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"),
}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.
If you can see a solution that doesn't require boxing, that could make an interesting blog post, or PR to this crate!
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.
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), ...
I just had the case already over the past decades where everyone wants to turn every error into Java backtraces (even when not in Java ;) ) and want to emphasize that this is almost never what one should do in regular operation.
As you said, unwrap() and expect() and panic!() do that for the "definitely a bug" case already. And, there, it's correct.
>probably most times want to let the "user" (the developer using the library) decide what course of action to take, based on the error.
I agree.
>But maybe the user wants to file a bug report, and then it would be helpful to know the file and line information.
In my opinion this belongs in the debug information (i.e. dwarf or similar) then. You can also store dwarf debug info outside of the object file.
>> 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), ...
I see. But what I meant is that if the Err case of a regular Result value is enriched like this, why not the Ok case? After all, the programmer (user of your crate) could have erroneously returned Ok where they should have returned Err. Would only be consistent.
I know that that isn't easily possible there. That brings me back to "all result::Result construction sites should show up in dwarf debug info".
I use debuggers less than I should. But thinking about it that's silly. We should be able to use debuggers to debug problems, including problems like this.
This attitude that the happy-case and the error-case paths are equivalent and should be treated the same is bizarre and seems to be contradicted by virtually every language choice, even in communities that profess it. Go is probably the most notorious example of professing "errors are just regular values" and refusing to add any error-handling constructs to the language; and yet, even in Go, errors are called, well, `error`s and there are explicit patterns that everyone follows that would never be considered acceptable for regular values (such as naming all error values "err" and reusing the "err" variable to hold any error returned by any function in the same context - try that with a "ret" to hold any non-err return value and see how many people accept your code).
Truly exceptional situations are those we don't expect, and for that we panic. I expect errors, because it's a computer and that's what they do.
As such, virtually all languages separate the business logic from the error logic. They offer exceptions, or Rust's ? macro, or enshrine certain error handling patterns (negative return codes in C, if err != nil in Go, use of the Either monad in Haskell).
If you don't need that control though and don't have a problem using `tracing` (which is excellent and very powerful btw), is there any reason to reach for wherr instead?
Not sure what you mean when you say you want to "turn off the stack traces"?
There are no implicit stack traces / backtraces. The detailed example in the docs makes use of `.expect()` and `RUST_BACKTRACE=1`, maybe this made you think it always does a backtrace?
Here is an example without backtraces, where the debug string is created and printed for the error instead:
use wherr::wherr;
#[wherr]
fn concat_files(path1: &str, path2: &str) -> Result<String, Box<dyn std::error::Error>> {
let mut content1 = std::fs::read_to_string(path1)?;
let content2 = std::fs::read_to_string(path2)?;
content1.push_str(&content2);
Ok(content1)
}
fn main() {
let content = concat_files("file1.txt", "file2.txt");
println!("{:?}", content);
}
% cargo run --example example
Compiling wherr v0.1.5 (/Users/joel/src/wherr/wherr)
Finished dev [unoptimized + debuginfo] target(s) in 0.15s
Running `/Users/joel/src/wherr/target/debug/examples/example`
Err(Os { code: 2, kind: NotFound, message: "No such file or directory" }
error at wherr/examples/example.rs:5)Now, maybe if the functions return something like Result<_, wherr::Error>, you could collapse those traces, but if you're returning Result<_, Box<dyn Error>>, that isn't possible (barring macro shenanigans).
Google it.
I’ll have to check if this macro is an easier solution!
`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.
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.
There is [0].
However, I'm not personally a fan of backtraces (or the lightweight location information, really). I'd much rather provide what I call a "semantic backtrace", which is where almost every Result-returning function is annotated by the programmer with a unique `.context(...)` call. This often results in a better error to the user ("couldn't connect to the server <because> couldn't get the URL <because> couldn't open the config file <because> couldn't open file ..."). The programmer can identify the location of the error due to the uniqueness of each context.
Backtraces are an easier way of getting similar information, but much less useful to the end user. I'd rather people prioritize the end user over the developer, so that's what I try to encourage in SNAFU.
That being said, plenty of people want location / backtrace information, so SNAFU tries to support those as well.
[0]: https://docs.rs/snafu/latest/snafu/struct.Backtrace.html
Also, most users are non-technical. So semantic backtraces that are visible only end up helping a small, more technical subset of your customer base (even if your customer base is developers in my experience). You do want to be able to have a way to link a semantic backtrace to a specific error instance that a customer observed though.
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.
That said, this crate is an answer to my prayers if it works as advertised.
#[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(),
}
}
}