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.
// 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 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.
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.
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.
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.
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.
> In addition, with this crate, you will also get the information that this error was returned in washing_machine.rs:3562. Thanks, I guess?
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).
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.
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.
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.
> (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.
No, if you need your error to outlive the path, then you can create a dedicated error that hold the path itself. This way it could lead to an allocation, but only when you actually need it.
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).
> can create a dedicated error that hold the path itself.
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.
What you're talking about is very practical imo and likely the right solution, but unfortunately would be a breaking change (you can't just add a lifetime annotation to the error).
It's not even my biggest complaint about std::fs, the entire module feels very much not like the rest of Rust and way too close to the underlying libc (for legitimate reasons: they couldn't release Rust 1.0 without file access and likely didn't have much bandwidth to work on an alternative API).
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.
> The one time out of ten is things like a script with a #! line that refers to a file that doesn't exist.
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.
No, because that's not desired behavior in all cases. There can be security/privacy issues stemming from that. Making it the default means you have to do string manipulation to cut out information you don't otherwise want, which is way less robust and more fragile than simply not including it in the first place.
For the maybe 2 or 3 times I've thought "we probably shouldn't include that in an error visible to end-users" there's probably 100 times I've wished it had been included though, and would've been able to save said end users considerable time if we/they had known which file/resource was being accessed at the point a critical error occurred.
At any rate at most I'd expect the file name to be written to a log file, to which only relatively privileged users would have access.
Yes sure but to make it the default asserts that it's the correct thing to do in all cases. Those aren't assertions rust folks tend to make, and for things like logging that's really up to the application how things are logged anyway. Plus if you're issuing a filesystem call you're almost definitely in possession of the filename anyway so it's just a matter of format!("failed to write: {:?}: {:?}", err, filename).
My related pet peeve is a failure to give a good error message when the disk runs out of space. This often manifests in multiple applications failing silently or with cryptic error numbers. Sometimes you can't log in to the computer, the password is accepted but you're returned to the login prompt with no error message.
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.
I remember spending an afternoon figuring out why Docker images stopped building with legitimate errors because it ran out of disk space. It never even occurred to me to check and I found the virtual disk was full by accident.
Including the filename is a highly opinionated decision. First of all it means your error value needs to have a string in it at minimum, which means extra overhead compared (eg) an enum that could compile down to an int. Maybe you don’t normally care, but the standard library has to. The caller always has the context needed to know what file wasn’t found and propagate it as appropriate. This also introduces a whole angle around ownership of that memory in a language like Rust, and would be a huge headache in C.
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.
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.
For the file not found errors, I recommend the fs-err crate. If you ask for async support, you should use spawn_blocking anyway as that's what tokio::fs does too
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.
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),
...
This crate is nice to have for debugging. Thank you for writing it.
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.
It seems that you are trying to take the position that Ok/Err are similar to if/else in that neither is more or less expected than the other. The very names chosen in Rust show that this is not how most think about these two cases. Returning Err is for cases that the developer considered errors, abnormal situations.
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).
But errors that are returned by functions are expected behavior. Except for where you assert its bizarre you seem to support this position. The only semantic thing rust offers here is '?', and that's just syntax sugar for errors are values.
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.
No, the error path is a distinct concept from the success path, in almost all situations. The success path implements some kind of business logic, and will be highly specific to the task at hand. The error path almost always does nothing more than returning early with an indication that the operation failed (with some context about the failure) to an entity above, ultimately to the user. A minority of cases may involve an automatic retry of the operation. Anything else is vanishingly rare.
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).
Excellent work! Note, this is also possible in tokio-rs/tracing, by using `.with_file()` and `.with_line_number()` to the subscriber creation.. I'm sure you can set up filters/layers for more granular control over this if you don't always want file/line info included, but it's nice to have another option that makes it as easy as using the `#[wherr]` macro, and honestly, tokio-rs/tracing layers and filters are a bit unwieldy.
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?
This crate is an answer to my debugging prayers, thank you for making it. Is there a feature I can toggle to turn off the stack traces? This is a feature that I can see being useful only in development builds for some of my use cases.
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)
Ah, I meant "stack traces" in the system sense: Imagine a really deep chain of #[wherr]-marked functions. If I understand the crate's behavior right, when an error occurs in that case, you're effectively constructing a stack trace as that error bubbles up through `?` operators.
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).
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.
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.
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.
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.
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!
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.
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.
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.
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.
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.
Yes, this is how I prefer to do it as well. Takes some work to consistently remember to put in context information, but you can add in things like the filename for those NotFound errors, and so on.
Semantic backtraces should be visible to the system administrator. In a DevOps shop, that's your developer, not your end user. This mentality works for application software the end user is running on their own hardware. Showing backtraces to users external to your system administrator is generally not desirable - you don't want to leak implementation details because they may reveal proprietary intellectual property or they may reveal internal details that can be used to compromise your security. User-facing errors where you're running software in a managed environment are typically carefully managed as explicit carveouts in terms of what information they will reveal.
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.
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.
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.
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.
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.
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>>
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:
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.
Author of `wherr` here. Thanks for sharing a really nice alternative solution to the problem, I like it!
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.
Not the poster. What exactly are you asking about?
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`.
Not the person who posted the snippet. Yea, that should work. It tracks the position of the line that calls from() on the other error to convert it into MyError, which is what the ? operator will expand into.
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.
> 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.
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?
> 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?
Exactly. Although I think because `T: Impl Into<T>` it may not be necessary.
You're free to use it and/or create a crate for it. If you do, I would go with the second version (the one I linked in the playground), as it is more optimized and allows for not `Send + Sync` and lifetimes other than `'static`. It can be optimized further using unsafe code to use one word (like `anyhow`) does, but I don't think it is necessary.
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!(),
}
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!
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`.
93 comments
[ 2.4 ms ] story [ 124 ms ] threadI 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!
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.
Very, very few end up in front of an end user. Typically only in a CLI.
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.
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.
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-...
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
¹: 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.
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.
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.
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.
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.
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:
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.
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’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.
https://github.com/rust-lang/rust/issues/53487
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.
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.
That said, this crate is an answer to my prayers if it works as advertised.
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://docs.rs/snafu/latest/snafu/struct.Location.html
[0]: https://doc.rust-lang.org/std/panic/struct.Location.html#met...
https://news.ycombinator.com/item?id=37235165
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.
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`.
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?
Exactly. Although I think because `T: Impl Into<T>` it may not be necessary.
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`:
Can you figure it out?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:
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!