From b2179c1f73f6e0e6bcd92c82344cd85a9c9170d7 Mon Sep 17 00:00:00 2001 From: Schneems Date: Thu, 17 Oct 2024 13:34:03 -0500 Subject: [PATCH 1/4] Show source by default, add feature to hide it close #59 Changes the default behavior so that the output in the readme now matches the output a user would see when they use the library. Specifically, it now includes the original `std::io::Error` in the output. It introduces a feature `anyhow`. By default: - By default: fs-err will include `std::io::Error` in the Display output, and return `None` from `Error::source()` - With `anyhow` fs-err will not include std::io::Error in the Display output, and return `Some(std::io::Error)` from `Error::source()` This is based on the guidance from #51. That discussion links to this 2020 discussion https://github.com/rust-lang/project-error-handling/issues/23 that suggests that you should either print the source and return `None` from `Error::source()` (https://doc.rust-lang.org/std/error/trait.Error.html#method.source) or not print anything and return `Some(E)`. This allows users of anyhow (or similar) libraries to configure fs-err for the behavior they desire, while not hiding information by default from unsuspecting adopters that are not using those libraries. It optimizes for the case of accidentally showing extra information that a user can then investigate and disable, rather than hiding information that the user might not realize is missing. --- .github/workflows/ci.yml | 6 ++++++ CHANGELOG.md | 2 ++ Cargo.toml | 3 +++ src/errors.rs | 26 ++++++++++++++++++++++++-- src/lib.rs | 4 ++++ 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 798422a..64aae50 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -67,3 +67,9 @@ jobs: command: check args: --features tokio if: ${{ matrix.rust_version == 'stable' || matrix.rust_version == 'beta' }} + + - name: cargo check --features anyhow + uses: actions-rs/cargo@v1 + with: + command: check + args: --features anyhow diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ad5ccc..7db14a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ # fs-err Changelog +* Change errors to output original `std::io::Error` information Display by default. This functionality can be disabled for [anyhow](https://docs.rs/anyhow/latest/anyhow/) users by using the new feature `anyhow` ([#60](https://github.com/andrewhickman/fs-err/pull/60)). + ## 2.11.0 * Added the first line of the standard library documentation to each function's rustdocs, to make them more useful in IDEs ([#50](https://github.com/andrewhickman/fs-err/issues/45)) diff --git a/Cargo.toml b/Cargo.toml index 6c999e3..4d8cf44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,9 @@ serde_json = "1.0.64" [features] # Adds I/O safety traits, introduced in Rust 1.63 io_safety = [] +# Removes the original error text from Display and relies on a wrapper library +# (such as anyhow) to emit them via `Error::source()`. +anyhow = [] [package.metadata.release] tag-name = "{{version}}" diff --git a/src/errors.rs b/src/errors.rs index 22a0eaa..a8318d0 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -99,7 +99,12 @@ impl fmt::Display for Error { E::ReadAt => write!(formatter, "failed to read with offset from `{}`", path), #[cfg(unix)] E::WriteAt => write!(formatter, "failed to write with offset to `{}`", path), - } + }?; + + #[cfg(not(feature = "anyhow"))] + write!(formatter, " caused by: {}", self.source)?; + + Ok(()) } } @@ -108,6 +113,12 @@ impl StdError for Error { self.source() } + #[cfg(not(feature = "anyhow"))] + fn source(&self) -> Option<&(dyn StdError + 'static)> { + None + } + + #[cfg(feature = "anyhow")] fn source(&self) -> Option<&(dyn StdError + 'static)> { Some(&self.source) } @@ -188,7 +199,12 @@ impl fmt::Display for SourceDestError { SourceDestErrorKind::SymlinkDir => { write!(formatter, "failed to symlink dir from {} to {}", from, to) } - } + }?; + + #[cfg(not(feature = "anyhow"))] + write!(formatter, " caused by: {}", self.source)?; + + Ok(()) } } @@ -197,6 +213,12 @@ impl StdError for SourceDestError { self.source() } + #[cfg(not(feature = "anyhow"))] + fn source(&self) -> Option<&(dyn StdError + 'static)> { + None + } + + #[cfg(feature = "anyhow")] fn source(&self) -> Option<&(dyn StdError + 'static)> { Some(&self.source) } diff --git a/src/lib.rs b/src/lib.rs index 19b8c8d..991dd52 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,10 @@ failed to open file `does not exist.txt` caused by: The system cannot find the file specified. (os error 2) ``` +> Note: To bypass displaying the original error message you can enable the `anyhow` feature. +> When the `anyhow` feature is enabled `Error::source()` will return `Some` and the original +> error will not be `Display`-ed via fs-err. + # Usage fs-err's API is the same as [`std::fs`][std::fs], so migrating code to use it is easy. From 7c1df0a754e045efce354430812fed0689bad4ca Mon Sep 17 00:00:00 2001 From: Schneems Date: Fri, 18 Oct 2024 12:41:12 -0500 Subject: [PATCH 2/4] Update feature name I discussed the logic here https://github.com/andrewhickman/fs-err/issues/59#issuecomment-2422721867. The prior suggested name "anyhow" was a little too specific/esoteric and in the event that the feature flag outlives the popularity of `anyhow` it wouldn't make sense. This suggested name tries to focus on the behavior outcome of the feature. It empowers callers to format the "called by" information however they want by: - Providing a source of that data (Error::source) - Not formatting emitting that data in `Display` --- .github/workflows/ci.yml | 4 ++-- CHANGELOG.md | 2 +- Cargo.toml | 10 +++++++--- src/errors.rs | 12 ++++++------ src/lib.rs | 5 ++--- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 64aae50..6b6d64a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,8 +68,8 @@ jobs: args: --features tokio if: ${{ matrix.rust_version == 'stable' || matrix.rust_version == 'beta' }} - - name: cargo check --features anyhow + - name: cargo check --features custom_caused_by uses: actions-rs/cargo@v1 with: command: check - args: --features anyhow + args: --features custom_caused_by diff --git a/CHANGELOG.md b/CHANGELOG.md index 7db14a5..33258e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # fs-err Changelog -* Change errors to output original `std::io::Error` information Display by default. This functionality can be disabled for [anyhow](https://docs.rs/anyhow/latest/anyhow/) users by using the new feature `anyhow` ([#60](https://github.com/andrewhickman/fs-err/pull/60)). +* Change errors to output original `std::io::Error` information Display by default. This functionality can be disabled for [anyhow](https://docs.rs/anyhow/latest/anyhow/) users by using the new feature `custom_caused_by` ([#60](https://github.com/andrewhickman/fs-err/pull/60)). ## 2.11.0 diff --git a/Cargo.toml b/Cargo.toml index 4d8cf44..25ee99c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,9 +23,13 @@ serde_json = "1.0.64" [features] # Adds I/O safety traits, introduced in Rust 1.63 io_safety = [] -# Removes the original error text from Display and relies on a wrapper library -# (such as anyhow) to emit them via `Error::source()`. -anyhow = [] + +# Allow custom formatting of the error source +# +# When enabled errors emit `std::error::Error::source()` as Some (default is `None`) and +# no longer include the original `std::io::Error` source in the `Display` implementation. +# This is useful if errors are wrapped in another library such as Anyhow. +custom_caused_by = [] [package.metadata.release] tag-name = "{{version}}" diff --git a/src/errors.rs b/src/errors.rs index a8318d0..eaf05f2 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -101,7 +101,7 @@ impl fmt::Display for Error { E::WriteAt => write!(formatter, "failed to write with offset to `{}`", path), }?; - #[cfg(not(feature = "anyhow"))] + #[cfg(not(feature = "custom_caused_by"))] write!(formatter, " caused by: {}", self.source)?; Ok(()) @@ -113,12 +113,12 @@ impl StdError for Error { self.source() } - #[cfg(not(feature = "anyhow"))] + #[cfg(not(feature = "custom_caused_by"))] fn source(&self) -> Option<&(dyn StdError + 'static)> { None } - #[cfg(feature = "anyhow")] + #[cfg(feature = "custom_caused_by")] fn source(&self) -> Option<&(dyn StdError + 'static)> { Some(&self.source) } @@ -201,7 +201,7 @@ impl fmt::Display for SourceDestError { } }?; - #[cfg(not(feature = "anyhow"))] + #[cfg(not(feature = "custom_caused_by"))] write!(formatter, " caused by: {}", self.source)?; Ok(()) @@ -213,12 +213,12 @@ impl StdError for SourceDestError { self.source() } - #[cfg(not(feature = "anyhow"))] + #[cfg(not(feature = "custom_caused_by"))] fn source(&self) -> Option<&(dyn StdError + 'static)> { None } - #[cfg(feature = "anyhow")] + #[cfg(feature = "custom_caused_by")] fn source(&self) -> Option<&(dyn StdError + 'static)> { Some(&self.source) } diff --git a/src/lib.rs b/src/lib.rs index 991dd52..ee83100 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,9 +26,8 @@ failed to open file `does not exist.txt` caused by: The system cannot find the file specified. (os error 2) ``` -> Note: To bypass displaying the original error message you can enable the `anyhow` feature. -> When the `anyhow` feature is enabled `Error::source()` will return `Some` and the original -> error will not be `Display`-ed via fs-err. +> Note: Users of `anyhow` or other libraries that format an Error's sources can enable the `custom_caused_by` feature to control the formatting of the orginal error message. +> When enabled, the `std::fmt::Display` implementation will emit the failed operation and paths but not the original `std::io::Error`. It will instead provide access via [Error::source](https://doc.rust-lang.org/std/error/trait.Error.html#method.source), which will be used by `anyhow` (or similar) libraries. # Usage From 5fe5650c792d1e22816b1b63705f862910f7e077 Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 21 Oct 2024 10:21:04 -0500 Subject: [PATCH 3/4] Update feature name to `expose_original_error` --- .github/workflows/ci.yml | 4 ++-- CHANGELOG.md | 2 +- Cargo.toml | 2 +- src/errors.rs | 12 ++++++------ src/lib.rs | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b6d64a..e467637 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -68,8 +68,8 @@ jobs: args: --features tokio if: ${{ matrix.rust_version == 'stable' || matrix.rust_version == 'beta' }} - - name: cargo check --features custom_caused_by + - name: cargo check --features expose_original_error uses: actions-rs/cargo@v1 with: command: check - args: --features custom_caused_by + args: --features expose_original_error diff --git a/CHANGELOG.md b/CHANGELOG.md index 33258e1..6baeb14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # fs-err Changelog -* Change errors to output original `std::io::Error` information Display by default. This functionality can be disabled for [anyhow](https://docs.rs/anyhow/latest/anyhow/) users by using the new feature `custom_caused_by` ([#60](https://github.com/andrewhickman/fs-err/pull/60)). +* Change errors to output original `std::io::Error` information Display by default. This functionality can be disabled for [anyhow](https://docs.rs/anyhow/latest/anyhow/) users by using the new feature `expose_original_error` ([#60](https://github.com/andrewhickman/fs-err/pull/60)). ## 2.11.0 diff --git a/Cargo.toml b/Cargo.toml index 25ee99c..dd8e02b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,7 @@ io_safety = [] # When enabled errors emit `std::error::Error::source()` as Some (default is `None`) and # no longer include the original `std::io::Error` source in the `Display` implementation. # This is useful if errors are wrapped in another library such as Anyhow. -custom_caused_by = [] +expose_original_error = [] [package.metadata.release] tag-name = "{{version}}" diff --git a/src/errors.rs b/src/errors.rs index eaf05f2..8188621 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -101,7 +101,7 @@ impl fmt::Display for Error { E::WriteAt => write!(formatter, "failed to write with offset to `{}`", path), }?; - #[cfg(not(feature = "custom_caused_by"))] + #[cfg(not(feature = "expose_original_error"))] write!(formatter, " caused by: {}", self.source)?; Ok(()) @@ -113,12 +113,12 @@ impl StdError for Error { self.source() } - #[cfg(not(feature = "custom_caused_by"))] + #[cfg(not(feature = "expose_original_error"))] fn source(&self) -> Option<&(dyn StdError + 'static)> { None } - #[cfg(feature = "custom_caused_by")] + #[cfg(feature = "expose_original_error")] fn source(&self) -> Option<&(dyn StdError + 'static)> { Some(&self.source) } @@ -201,7 +201,7 @@ impl fmt::Display for SourceDestError { } }?; - #[cfg(not(feature = "custom_caused_by"))] + #[cfg(not(feature = "expose_original_error"))] write!(formatter, " caused by: {}", self.source)?; Ok(()) @@ -213,12 +213,12 @@ impl StdError for SourceDestError { self.source() } - #[cfg(not(feature = "custom_caused_by"))] + #[cfg(not(feature = "expose_original_error"))] fn source(&self) -> Option<&(dyn StdError + 'static)> { None } - #[cfg(feature = "custom_caused_by")] + #[cfg(feature = "expose_original_error")] fn source(&self) -> Option<&(dyn StdError + 'static)> { Some(&self.source) } diff --git a/src/lib.rs b/src/lib.rs index ee83100..52eb2b1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,7 +26,7 @@ failed to open file `does not exist.txt` caused by: The system cannot find the file specified. (os error 2) ``` -> Note: Users of `anyhow` or other libraries that format an Error's sources can enable the `custom_caused_by` feature to control the formatting of the orginal error message. +> Note: Users of `anyhow` or other libraries that format an Error's sources can enable the `expose_original_error` feature to control the formatting of the orginal error message. > When enabled, the `std::fmt::Display` implementation will emit the failed operation and paths but not the original `std::io::Error`. It will instead provide access via [Error::source](https://doc.rust-lang.org/std/error/trait.Error.html#method.source), which will be used by `anyhow` (or similar) libraries. # Usage From b860ec938725347ad8a0828e2f53a148b7e06ef8 Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 21 Oct 2024 11:06:42 -0500 Subject: [PATCH 4/4] Document why that feature disables displaying error At a glance it's not clear why that logic exists based solely on the feature name. Adding a doc here clarifies the intent. --- src/errors.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/errors.rs b/src/errors.rs index 8188621..56fd4f8 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -101,6 +101,7 @@ impl fmt::Display for Error { E::WriteAt => write!(formatter, "failed to write with offset to `{}`", path), }?; + // The `expose_original_error` feature indicates the caller should display the original error #[cfg(not(feature = "expose_original_error"))] write!(formatter, " caused by: {}", self.source)?; @@ -201,6 +202,7 @@ impl fmt::Display for SourceDestError { } }?; + // The `expose_original_error` feature indicates the caller should display the original error #[cfg(not(feature = "expose_original_error"))] write!(formatter, " caused by: {}", self.source)?;