From 8ce3e78952bd8f1fcbf6098951fa4e3937018c8b Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Sun, 17 Nov 2019 21:54:44 +0100 Subject: [PATCH 1/3] verbose errors feature This adds a new "verbose-errors" feature flag to async-std that enables wrapping certain errors in structures with more context. As an example, we use it in `fs::File::{open,create}` to add the given path to the error message (something that is lacking in std to annoyance of many). --- .github/workflows/ci.yml | 6 +++++ Cargo.toml | 1 + src/fs/file.rs | 13 ++++++++-- src/io/mod.rs | 1 + src/io/utils.rs | 51 ++++++++++++++++++++++++++++++++++++++++ src/utils.rs | 8 +++++++ tests/verbose_errors.rs | 20 ++++++++++++++++ 7 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 src/io/utils.rs create mode 100644 tests/verbose_errors.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99436b7..5d5639c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,6 +64,12 @@ jobs: command: test args: --all --features unstable + - name: tests with verbose errors + uses: actions-rs/cargo@v1 + with: + command: test + args: --all --features 'unstable verbose-errors' + check_fmt_and_docs: name: Checking fmt and docs runs-on: ubuntu-latest diff --git a/Cargo.toml b/Cargo.toml index 7ffaae3..7886bcf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ std = [ "pin-utils", "slab", ] +verbose-errors = [] [dependencies] async-attributes = { version = "1.1.1", optional = true } diff --git a/src/fs/file.rs b/src/fs/file.rs index 8bc6c2c..5186e96 100644 --- a/src/fs/file.rs +++ b/src/fs/file.rs @@ -9,6 +9,7 @@ use std::sync::{Arc, Mutex}; use crate::fs::{Metadata, Permissions}; use crate::future; +use crate::utils::VerboseErrorExt; use crate::io::{self, Read, Seek, SeekFrom, Write}; use crate::path::Path; use crate::prelude::*; @@ -112,7 +113,11 @@ impl File { /// ``` pub async fn open>(path: P) -> io::Result { let path = path.as_ref().to_owned(); - let file = spawn_blocking(move || std::fs::File::open(&path)).await?; + let file = spawn_blocking(move || { + std::fs::File::open(&path) + .verbose_context(|| format!("Could not open {}", path.display())) + }) + .await?; Ok(File::new(file, true)) } @@ -147,7 +152,11 @@ impl File { /// ``` pub async fn create>(path: P) -> io::Result { let path = path.as_ref().to_owned(); - let file = spawn_blocking(move || std::fs::File::create(&path)).await?; + let file = spawn_blocking(move || { + std::fs::File::create(&path) + .verbose_context(|| format!("Could not create {}", path.display())) + }) + .await?; Ok(File::new(file, true)) } diff --git a/src/io/mod.rs b/src/io/mod.rs index 4e83230..d9660a7 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -291,6 +291,7 @@ cfg_std! { pub(crate) mod read; pub(crate) mod seek; pub(crate) mod write; + pub(crate) mod utils; mod buf_reader; mod buf_writer; diff --git a/src/io/utils.rs b/src/io/utils.rs new file mode 100644 index 0000000..ba6285f --- /dev/null +++ b/src/io/utils.rs @@ -0,0 +1,51 @@ +use std::{error::Error, fmt, io}; +use crate::utils::VerboseErrorExt; + +/// Wrap `std::io::Error` with additional message +/// +/// *Note* Only active when `verbose-errors` feature is enabled for this crate! +/// +/// Keeps the original error kind and stores the original I/O error as `source`. +impl VerboseErrorExt for Result { + fn verbose_context(self, message: impl Fn() -> String) -> Self { + if cfg!(feature = "verbose-errors") { + self.map_err(|e| VerboseError::wrap(e, message())) + } else { + self + } + } +} + +#[derive(Debug)] +struct VerboseError { + source: io::Error, + message: String, +} + +impl VerboseError { + fn wrap(source: io::Error, message: impl Into) -> io::Error { + io::Error::new( + source.kind(), + VerboseError { + source, + message: message.into(), + }, + ) + } +} + +impl fmt::Display for VerboseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.message) + } +} + +impl Error for VerboseError { + fn description(&self) -> &str { + self.source.description() + } + + fn source(&self) -> Option<&(dyn Error + 'static)> { + Some(&self.source) + } +} diff --git a/src/utils.rs b/src/utils.rs index 13dbe37..00dc793 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -52,6 +52,14 @@ pub fn random(n: u32) -> u32 { }) } +/// Add additional context to errors +/// +/// *Note for implementors:* The given closure must only be executed when +/// `verbose-errors` feature is enabled for this crate! +pub(crate) trait VerboseErrorExt { + fn verbose_context(self, message: impl Fn() -> String) -> Self; +} + /// Defers evaluation of a block of code until the end of the scope. #[cfg(feature = "default")] #[doc(hidden)] diff --git a/tests/verbose_errors.rs b/tests/verbose_errors.rs new file mode 100644 index 0000000..3d99ede --- /dev/null +++ b/tests/verbose_errors.rs @@ -0,0 +1,20 @@ +#[cfg(feature = "verbose-errors")] +mod verbose_tests { + use async_std::{fs, task}; + + #[test] + fn open_file() { + task::block_on(async { + let non_existing_file = + "/ashjudlkahasdasdsikdhajik/asdasdasdasdasdasd/fjuiklashdbflasas"; + let res = fs::File::open(non_existing_file).await; + match res { + Ok(_) => panic!("Found file with random name: We live in a simulation"), + Err(e) => assert_eq!( + "Could not open /ashjudlkahasdasdsikdhajik/asdasdasdasdasdasd/fjuiklashdbflasas", + &format!("{}", e) + ), + } + }) + } +} From 99ddfb3f9393273e401b94afee84004ae3e284cb Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Sun, 17 Nov 2019 23:11:11 +0100 Subject: [PATCH 2/3] Wrap code more clearly in cfg blocks --- src/io/utils.rs | 63 +++++++++++++++++++++++++------------------------ src/utils.rs | 8 ++++++- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/src/io/utils.rs b/src/io/utils.rs index ba6285f..ebd2214 100644 --- a/src/io/utils.rs +++ b/src/io/utils.rs @@ -1,4 +1,3 @@ -use std::{error::Error, fmt, io}; use crate::utils::VerboseErrorExt; /// Wrap `std::io::Error` with additional message @@ -6,46 +5,48 @@ use crate::utils::VerboseErrorExt; /// *Note* Only active when `verbose-errors` feature is enabled for this crate! /// /// Keeps the original error kind and stores the original I/O error as `source`. -impl VerboseErrorExt for Result { +impl VerboseErrorExt for Result { + #[cfg(feature = "verbose-errors")] fn verbose_context(self, message: impl Fn() -> String) -> Self { - if cfg!(feature = "verbose-errors") { - self.map_err(|e| VerboseError::wrap(e, message())) - } else { - self - } + self.map_err(|e| verbose::Error::wrap(e, message())) } } -#[derive(Debug)] -struct VerboseError { - source: io::Error, - message: String, -} +#[cfg(feature = "verbose-errors")] +mod verbose { + use std::{error::Error as StdError, fmt, io}; -impl VerboseError { - fn wrap(source: io::Error, message: impl Into) -> io::Error { - io::Error::new( - source.kind(), - VerboseError { - source, - message: message.into(), - }, - ) + #[derive(Debug)] + pub(crate) struct Error { + source: io::Error, + message: String, } -} -impl fmt::Display for VerboseError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.message) + impl Error { + pub(crate) fn wrap(source: io::Error, message: impl Into) -> io::Error { + io::Error::new( + source.kind(), + Error { + source, + message: message.into(), + }, + ) + } } -} -impl Error for VerboseError { - fn description(&self) -> &str { - self.source.description() + impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.message) + } } - fn source(&self) -> Option<&(dyn Error + 'static)> { - Some(&self.source) + impl StdError for Error { + fn description(&self) -> &str { + self.source.description() + } + + fn source(&self) -> Option<&(dyn StdError + 'static)> { + Some(&self.source) + } } } diff --git a/src/utils.rs b/src/utils.rs index 00dc793..ce4a85c 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -56,8 +56,14 @@ pub fn random(n: u32) -> u32 { /// /// *Note for implementors:* The given closure must only be executed when /// `verbose-errors` feature is enabled for this crate! -pub(crate) trait VerboseErrorExt { +pub(crate) trait VerboseErrorExt: Sized { + #[cfg(feature = "verbose-errors")] fn verbose_context(self, message: impl Fn() -> String) -> Self; + + #[cfg(not(feature = "verbose-errors"))] + fn verbose_context(self, _: impl Fn() -> String) -> Self { + self + } } /// Defers evaluation of a block of code until the end of the scope. From c7046432965b374ed7bc03dc35b6e465a706b215 Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Mon, 18 Nov 2019 23:55:48 +0100 Subject: [PATCH 3/3] Remove verbose-errors cargo feature --- .github/workflows/ci.yml | 6 ---- Cargo.toml | 1 - src/fs/file.rs | 6 ++-- src/io/utils.rs | 66 ++++++++++++++++++---------------------- src/utils.rs | 13 ++------ tests/verbose_errors.rs | 32 +++++++++---------- 6 files changed, 49 insertions(+), 75 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5d5639c..99436b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -64,12 +64,6 @@ jobs: command: test args: --all --features unstable - - name: tests with verbose errors - uses: actions-rs/cargo@v1 - with: - command: test - args: --all --features 'unstable verbose-errors' - check_fmt_and_docs: name: Checking fmt and docs runs-on: ubuntu-latest diff --git a/Cargo.toml b/Cargo.toml index 7886bcf..7ffaae3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,6 @@ std = [ "pin-utils", "slab", ] -verbose-errors = [] [dependencies] async-attributes = { version = "1.1.1", optional = true } diff --git a/src/fs/file.rs b/src/fs/file.rs index 5186e96..f824281 100644 --- a/src/fs/file.rs +++ b/src/fs/file.rs @@ -9,7 +9,7 @@ use std::sync::{Arc, Mutex}; use crate::fs::{Metadata, Permissions}; use crate::future; -use crate::utils::VerboseErrorExt; +use crate::utils::Context as _; use crate::io::{self, Read, Seek, SeekFrom, Write}; use crate::path::Path; use crate::prelude::*; @@ -115,7 +115,7 @@ impl File { let path = path.as_ref().to_owned(); let file = spawn_blocking(move || { std::fs::File::open(&path) - .verbose_context(|| format!("Could not open {}", path.display())) + .context(|| format!("Could not open {}", path.display())) }) .await?; Ok(File::new(file, true)) @@ -154,7 +154,7 @@ impl File { let path = path.as_ref().to_owned(); let file = spawn_blocking(move || { std::fs::File::create(&path) - .verbose_context(|| format!("Could not create {}", path.display())) + .context(|| format!("Could not create {}", path.display())) }) .await?; Ok(File::new(file, true)) diff --git a/src/io/utils.rs b/src/io/utils.rs index ebd2214..1b73064 100644 --- a/src/io/utils.rs +++ b/src/io/utils.rs @@ -1,52 +1,46 @@ -use crate::utils::VerboseErrorExt; +use crate::utils::Context; /// Wrap `std::io::Error` with additional message /// -/// *Note* Only active when `verbose-errors` feature is enabled for this crate! -/// /// Keeps the original error kind and stores the original I/O error as `source`. -impl VerboseErrorExt for Result { - #[cfg(feature = "verbose-errors")] - fn verbose_context(self, message: impl Fn() -> String) -> Self { - self.map_err(|e| verbose::Error::wrap(e, message())) +impl Context for Result { + fn context(self, message: impl Fn() -> String) -> Self { + self.map_err(|e| VerboseError::wrap(e, message())) } } -#[cfg(feature = "verbose-errors")] -mod verbose { - use std::{error::Error as StdError, fmt, io}; +use std::{error::Error as StdError, fmt, io}; - #[derive(Debug)] - pub(crate) struct Error { - source: io::Error, - message: String, - } +#[derive(Debug)] +pub(crate) struct VerboseError { + source: io::Error, + message: String, +} - impl Error { - pub(crate) fn wrap(source: io::Error, message: impl Into) -> io::Error { - io::Error::new( - source.kind(), - Error { - source, - message: message.into(), - }, - ) - } +impl VerboseError { + pub(crate) fn wrap(source: io::Error, message: impl Into) -> io::Error { + io::Error::new( + source.kind(), + VerboseError { + source, + message: message.into(), + }, + ) } +} - impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.message) - } +impl fmt::Display for VerboseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.message) } +} - impl StdError for Error { - fn description(&self) -> &str { - self.source.description() - } +impl StdError for VerboseError { + fn description(&self) -> &str { + self.source.description() + } - fn source(&self) -> Option<&(dyn StdError + 'static)> { - Some(&self.source) - } + fn source(&self) -> Option<&(dyn StdError + 'static)> { + Some(&self.source) } } diff --git a/src/utils.rs b/src/utils.rs index ce4a85c..645dc84 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -53,17 +53,8 @@ pub fn random(n: u32) -> u32 { } /// Add additional context to errors -/// -/// *Note for implementors:* The given closure must only be executed when -/// `verbose-errors` feature is enabled for this crate! -pub(crate) trait VerboseErrorExt: Sized { - #[cfg(feature = "verbose-errors")] - fn verbose_context(self, message: impl Fn() -> String) -> Self; - - #[cfg(not(feature = "verbose-errors"))] - fn verbose_context(self, _: impl Fn() -> String) -> Self { - self - } +pub(crate) trait Context { + fn context(self, message: impl Fn() -> String) -> Self; } /// Defers evaluation of a block of code until the end of the scope. diff --git a/tests/verbose_errors.rs b/tests/verbose_errors.rs index 3d99ede..54d04f8 100644 --- a/tests/verbose_errors.rs +++ b/tests/verbose_errors.rs @@ -1,20 +1,16 @@ -#[cfg(feature = "verbose-errors")] -mod verbose_tests { - use async_std::{fs, task}; +use async_std::{fs, task}; - #[test] - fn open_file() { - task::block_on(async { - let non_existing_file = - "/ashjudlkahasdasdsikdhajik/asdasdasdasdasdasd/fjuiklashdbflasas"; - let res = fs::File::open(non_existing_file).await; - match res { - Ok(_) => panic!("Found file with random name: We live in a simulation"), - Err(e) => assert_eq!( - "Could not open /ashjudlkahasdasdsikdhajik/asdasdasdasdasdasd/fjuiklashdbflasas", - &format!("{}", e) - ), - } - }) - } +#[test] +fn open_file() { + task::block_on(async { + let non_existing_file = "/ashjudlkahasdasdsikdhajik/asdasdasdasdasdasd/fjuiklashdbflasas"; + let res = fs::File::open(non_existing_file).await; + match res { + Ok(_) => panic!("Found file with random name: We live in a simulation"), + Err(e) => assert_eq!( + "Could not open /ashjudlkahasdasdsikdhajik/asdasdasdasdasdasd/fjuiklashdbflasas", + &format!("{}", e) + ), + } + }) }