Add line break after original error and improve examples in README.

joelonsql · Aug 23, 2023 · 8662fd1 · 8662fd1
1 parent 2757236
commit 8662fd1
Show file tree

Hide file tree

Showing 9 changed files with 237 additions and 30 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,3 @@
 /target
 Cargo.lock
+.vscode
diff --git a/README.md b/README.md
@@ -15,6 +15,7 @@ prohibits mixing normal functions with procedural macros in a single crate:
 ## Table of Contents
 
 - [Quick Start](#quick-start)
+- [What's the problem?](#whats-the-problem)
 - [How it works](#how-it-works)
 - [Usage](#usage)
   - [Without `#[wherr]`](#without-wherr)
@@ -38,17 +39,196 @@ wherr = "0.1"
 Now, by simply annotating your functions with `#[wherr]`, any error propagated
 using `?` will also include the file and line number.
 
+Note, that the Error type needs to be `Box<dyn std::error::Error>`.
+
 ```rust
-use wherr::wherr;
+ 1 | use wherr::wherr;
+ 2 |
+ 3 | #[wherr]
+ 4 | fn concat_files(path1: &str, path2: &str) -> Result<String, Box<dyn std::error::Error>> {
+ 5 |     let mut content1 = std::fs::read_to_string(path1)?;
+ 6 |     let content2 = std::fs::read_to_string(path2)?;
+ 7 |
+ 8 |     content1.push_str(&content2);
+ 9 |     Ok(content1)
+10 | }
+11 |
+12 | fn main() {
+13 |     let content = concat_files("file1.txt", "file2.txt").expect("Failed to concatenate the files");
+14 |     println!("Concatenated content:\n{}", content);
+15 | }
+```
 
-#[wherr]
-fn some_function() -> Result<(), Box<dyn std::error::Error>> {
-    /* ... */
-    some_operation()?;
-    /* ... */
-}
+```sh
+% cargo run --example with_wherr
+```
+```
+% cargo run --example with_wherr
+    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
+     Running `target/debug/examples/with_wherr`
+thread 'main' panicked at 'Failed to concatenate the files: Os { code: 2, kind: NotFound, message: "No such file or directory" }
+error at wherr/examples/with_wherr.rs:5', wherr/examples/with_wherr.rs:13:58
+note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ```
 
+Notice the text added by `wherr` in the output: **error at wherr/examples/with_wherr.rs:5**
+
+This tells us where the error originated from:
+
+```rust
+ 5 |     let mut content1 = std::fs::read_to_string(path1)?;
+```
+
+## What's the problem?
+
+The problem is that when simply using `?` to propagate errors, you don't get file and line information on **where** the error occurs.
+
+To demonstrate the problem, let's run the same example as above, but without `wherr`.
+
+```rust
+ 1 | fn concat_files(path1: &str, path2: &str) -> Result<String, std::io::Error> {
+ 2 |     let mut content1 = std::fs::read_to_string(path1)?;
+ 3 |     let content2 = std::fs::read_to_string(path2)?;
+ 4 |
+ 5 |     content1.push_str(&content2);
+ 6 |     Ok(content1)
+ 7 | }
+ 8 |
+ 9 | fn main() {
+10 |     let content = concat_files("file1.txt", "file2.txt").expect("Failed to concatenate the files");
+11 |     println!("Concatenated content:\n{}", content);
+12 | }
+```
+
+```sh
+% cargo run --example without_wherr
+```
+```
+    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
+     Running `target/debug/examples/without_wherr`
+thread 'main' panicked at 'Failed to concatenate the files: Os { code: 2, kind: NotFound, message: "No such file or directory" }', wherr/examples/without_wherr.rs:10:58
+note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
+```
+
+As you can see, the only file and line information we get is `wherr/examples/without_wherr.rs:10:58`, which is this line:
+
+```rust
+10 |     let content = concat_files("file1.txt", "file2.txt").expect("Failed to concatenate the files");
+```
+
+Unfortunately, if we suspect we have a bug somewhere in `concat_files()`,
+we might need to know what specific line that causes a certain error.
+
+This might be tricky, especially when you have nested layers of functions,
+where the same error kind might occurr at multiple different places.
+
+In our example, there are two calls to `read_to_string()`. Any one of them
+could have caused the `NotFound` kind of error.
+
+Running with `RUST_BACKTRACE=1` doesn't help:
+
+```sh
+% RUST_BACKTRACE=1 cargo run --example without_wherr
+```
+```
+    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
+     Running `target/debug/examples/without_wherr`
+thread 'main' panicked at 'Failed to concatenate the files: Os { code: 2, kind: NotFound, message: "No such file or directory" }', wherr/examples/without_wherr.rs:10:58
+stack backtrace:
+   0: rust_begin_unwind
+             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:579:5
+   1: core::panicking::panic_fmt
+             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/panicking.rs:64:14
+   2: core::result::unwrap_failed
+             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/result.rs:1750:5
+   3: core::result::Result<T,E>::expect
+             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/result.rs:1047:23
+   4: without_wherr::main
+             at ./wherr/examples/without_wherr.rs:10:19
+   5: core::ops::function::FnOnce::call_once
+             at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/ops/function.rs:250:5
+note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
+```
+
+Still only `without_wherr.rs:10`.
+
+Not even `RUST_BACKTRACE=full` helps:
+
+```sh
+% RUST_BACKTRACE=full cargo run --example without_wherr
+```
+```
+    Finished dev [unoptimized + debuginfo] target(s) in 0.01s
+     Running `target/debug/examples/without_wherr`
+thread 'main' panicked at 'Failed to concatenate the files: Os { code: 2, kind: NotFound, message: "No such file or directory" }', wherr/examples/without_wherr.rs:10:58
+stack backtrace:
+   0:        0x10266beec - std::backtrace_rs::backtrace::libunwind::trace::h2000fb4d08dbcc59
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/../../backtrace/src/backtrace/libunwind.rs:93:5
+   1:        0x10266beec - std::backtrace_rs::backtrace::trace_unsynchronized::h2b5e61495350674d
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/../../backtrace/src/backtrace/mod.rs:66:5
+   2:        0x10266beec - std::sys_common::backtrace::_print_fmt::h05f5bfbdb3415936
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/sys_common/backtrace.rs:65:5
+   3:        0x10266beec - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h105074e3d85f800b
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/sys_common/backtrace.rs:44:22
+   4:        0x10267d820 - core::fmt::write::h34766cf8fff7af1e
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/fmt/mod.rs:1232:17
+   5:        0x102669f60 - std::io::Write::write_fmt::hd64c4cf6e7adea59
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/io/mod.rs:1684:15
+   6:        0x10266bd00 - std::sys_common::backtrace::_print::hd92783a665d3ebfb
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/sys_common/backtrace.rs:47:5
+   7:        0x10266bd00 - std::sys_common::backtrace::print::h2a6828a537036cf9
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/sys_common/backtrace.rs:34:9
+   8:        0x10266d2ac - std::panicking::default_hook::{{closure}}::h4e82ce6ccef941b2
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:271:22
+   9:        0x10266d004 - std::panicking::default_hook::h29f62f8795c5cb00
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:290:9
+  10:        0x10266d7bc - std::panicking::rust_panic_with_hook::h19862cbd0fbda7ba
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:692:13
+  11:        0x10266d6f0 - std::panicking::begin_panic_handler::{{closure}}::h3f3626935e1669fe
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:583:13
+  12:        0x10266c30c - std::sys_common::backtrace::__rust_end_short_backtrace::h5054ef52bd507d0a
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/sys_common/backtrace.rs:150:18
+  13:        0x10266d44c - rust_begin_unwind
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:579:5
+  14:        0x102684590 - core::panicking::panic_fmt::h7e47e10600a90221
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/panicking.rs:64:14
+  15:        0x102684838 - core::result::unwrap_failed::h6a1757e313e2d291
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/result.rs:1750:5
+  16:        0x1026519b0 - core::result::Result<T,E>::expect::h3bb438a6543db6fe
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/result.rs:1047:23
+  17:        0x102653950 - without_wherr::main::hea9e71193dce0291
+                               at /Users/joel/src/wherr/wherr/examples/without_wherr.rs:10:19
+  18:        0x102653218 - core::ops::function::FnOnce::call_once::ha7443c5406a2bc4c
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/ops/function.rs:250:5
+  19:        0x1026518b4 - std::sys_common::backtrace::__rust_begin_short_backtrace::ha104f92716531d05
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/sys_common/backtrace.rs:134:18
+  20:        0x102650b40 - std::rt::lang_start::{{closure}}::h4d7ab56f8229bc15
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/rt.rs:166:18
+  21:        0x102667604 - core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &F>::call_once::hf2f6b444963da11f
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/core/src/ops/function.rs:287:13
+  22:        0x102667604 - std::panicking::try::do_call::h9152231fddd58858
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:487:40
+  23:        0x102667604 - std::panicking::try::hcc27eab3b8ee3cb1
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:451:19
+  24:        0x102667604 - std::panic::catch_unwind::hca546a4311ab9871
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panic.rs:140:14
+  25:        0x102667604 - std::rt::lang_start_internal::{{closure}}::h4e65aa71fe685c85
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/rt.rs:148:48
+  26:        0x102667604 - std::panicking::try::do_call::h61aea55fbdf97fc2
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:487:40
+  27:        0x102667604 - std::panicking::try::hcfc3b62fb8f6215e
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panicking.rs:451:19
+  28:        0x102667604 - std::panic::catch_unwind::h61a201e98b56a743
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/panic.rs:140:14
+  29:        0x102667604 - std::rt::lang_start_internal::h91996717d3eb1d2a
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/rt.rs:148:20
+  30:        0x102650b0c - std::rt::lang_start::ha5cfaf50836838f2
+                               at /rustc/84c898d65adf2f39a5a98507f1fe0ce10a2b8dbc/library/std/src/rt.rs:165:17
+  31:        0x102653a08 - _main
+```
+
+Still only `without_wherr.rs:10`.
+
 ## How it works
 
 The #[wherr] notation is a [proc_macro_attribute](https://doc.rust-lang.org/reference/procedural-macros.html#attribute-macros),
@@ -264,7 +444,8 @@ fn main() {
 cargo run --example basic_with_wherr
 ```
 ```
-x = Err(Error at wherr/examples/basic_with_wherr.rs:10. Original error: ParseIntError { kind: InvalidDigit })
+x = Err(ParseIntError { kind: InvalidDigit }
+error at wherr/examples/basic_with_wherr.rs:10)
 ```
 
 The line **wherr/examples/basic_with_wherr.rs:10** corresponds to:
@@ -297,9 +478,8 @@ fn main() {
 cargo run --example unwrap_with_wherr
 ```
 ```
-thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value:
-Error at wherr/examples/unwrap_with_wherr.rs:10.
-Original error: ParseIntError { kind: InvalidDigit }', wherr/examples/unwrap_with_wherr.rs:15:40
+thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: InvalidDigit }
+error at wherr/examples/unwrap_with_wherr.rs:10', wherr/examples/unwrap_with_wherr.rs:15:40
 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ```
 
@@ -313,13 +493,13 @@ And the line **wherr/examples/unwrap_with_wherr.rs:15** corresponds to:
     let x = add("123", "not a number").unwrap();
 ```
 
-Here is the diff (additional newlines added for clarity):
+Here is the diff:
 
 ```diff
-thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value:
+-thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value:
 -ParseIntError { kind: InvalidDigit }', wherr/examples/unwrap_without_wherr.rs:12:40
-+Error at wherr/examples/unwrap_with_wherr.rs:10.
-+Original error: ParseIntError { kind: InvalidDigit }', wherr/examples/unwrap_with_wherr.rs:15:40
++thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: InvalidDigit }
++error at wherr/examples/unwrap_with_wherr.rs:10', wherr/examples/unwrap_with_wherr.rs:15:40
 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
 ```
 
@@ -395,5 +575,4 @@ Rust community guidelines and submit a PR on our repository.
 
 ## License
 
-Please refer to the `LICENSE` file in the root directory of the crate for
-license details.
+Dual license. See the `LICENSE-MIT` and `LICENSE-APACHE` files.
diff --git a/wherr-macro/Cargo.toml b/wherr-macro/Cargo.toml
@@ -1,10 +1,10 @@
 [package]
 name = "wherr-macro"
-version = "0.1.3"
+version = "0.1.4"
 edition = "2021"
 authors = ["Joel Jakobsson <[email protected]>"]
 categories = ["rust-patterns"]
-description = "Enhance Rust error messages with file and line details using the `#[wherr]` macro for clearer debugging."
+description = "Enhance Rust errors with file and line details using the `#[wherr]` macro for clearer debugging."
 keywords = ["error", "error-handling"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/joelonsql/wherr"

diff --git a/wherr-macro/src/lib.rs b/wherr-macro/src/lib.rs
@@ -1,14 +1,14 @@
 //! `wherr-macro` crate provides a procedural macro to enhance Rust errors with file and line number information.
 //!
-//! When using the provided `wherr` macro attribute on a function, any error returned by the `?` operator within that function 
+//! When using the provided `wherr` macro attribute on a function, any error returned by the `?` operator within that function
 //! is automatically wrapped to include the file and line number where the error occurred.
 
 use proc_macro::TokenStream;
 use quote::{quote, quote_spanned};
 use syn::spanned::Spanned;
 use syn::{parse_macro_input, visit_mut::VisitMut, Expr};
 
-/// Procedural macro attribute that processes a function to automatically wrap errors using the `?` operator 
+/// Procedural macro attribute that processes a function to automatically wrap errors using the `?` operator
 /// with file and line number details.
 #[proc_macro_attribute]
 pub fn wherr(_attrs: TokenStream, input: TokenStream) -> TokenStream {
@@ -22,7 +22,7 @@ pub fn wherr(_attrs: TokenStream, input: TokenStream) -> TokenStream {
 
 /// Visitor used by the `wherr` procedural macro to traverse and mutate the Abstract Syntax Tree (AST) of the function.
 ///
-/// This visitor specifically looks for expressions using the `?` operator and wraps them with additional 
+/// This visitor specifically looks for expressions using the `?` operator and wraps them with additional
 /// file and line information.
 struct WherrVisitor;
 

diff --git a/wherr/Cargo.toml b/wherr/Cargo.toml
@@ -1,10 +1,10 @@
 [package]
 name = "wherr"
-version = "0.1.3"
+version = "0.1.4"
 edition = "2021"
 authors = ["Joel Jakobsson <[email protected]>"]
 categories = ["rust-patterns"]
-description = "Enhance Rust error messages with file and line details using the `#[wherr]` macro for clearer debugging."
+description = "Enhance Rust errors with file and line details using the `#[wherr]` macro for clearer debugging."
 keywords = ["error", "error-handling"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/joelonsql/wherr"

diff --git a/wherr/examples/with_wherr.rs b/wherr/examples/with_wherr.rs
@@ -0,0 +1,15 @@
+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").expect("Failed to concatenate the files");
+    println!("Concatenated content:\n{}", content);
+}
diff --git a/wherr/examples/without_wherr.rs b/wherr/examples/without_wherr.rs
@@ -0,0 +1,12 @@
+fn concat_files(path1: &str, path2: &str) -> Result<String, std::io::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").expect("Failed to concatenate the files");
+    println!("Concatenated content:\n{}", content);
+}
diff --git a/wherr/src/lib.rs b/wherr/src/lib.rs
@@ -39,8 +39,8 @@ impl fmt::Display for Wherr {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
-            "Error at {}:{}. Original error: {}",
-            self.file, self.line, self.inner
+            "{}\nerror at {}:{}",
+            self.inner, self.file, self.line
         )
     }
 }
@@ -49,8 +49,8 @@ impl fmt::Debug for Wherr {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(
             f,
-            "Error at {}:{}. Original error: {:?}",
-            self.file, self.line, self.inner
+            "{:?}\nerror at {}:{}",
+            self.inner, self.file, self.line
         )
     }
 }

diff --git a/wherr/tests/integration_tests.rs b/wherr/tests/integration_tests.rs
@@ -21,7 +21,7 @@ fn test_wherr_display() {
 
     assert_eq!(
         format!("{}", wherr),
-        format!("Error at test.rs:42. Original error: {}", error_message)
+        format!("{}\nerror at test.rs:42", error_message)
     );
 }
 
@@ -74,4 +74,4 @@ fn test_wherr_macro() {
             assert_eq!(wherr.inner.to_string(), "invalid digit found in string");
         }
     }
-}
+}