This is an automated email from the ASF dual-hosted git repository. xuanwo pushed a commit to branch refactor-error in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git
commit 97cc4f75cccc09b40a1da87589f279df149e5f69 Author: Xuanwo <[email protected]> AuthorDate: Wed Aug 16 15:13:26 2023 +0800 refactor: Implement backtrace for Error correctly Signed-off-by: Xuanwo <[email protected]> --- bin/oay/src/services/webdav/webdav_dir_entry.rs | 6 +- bin/oay/src/services/webdav/webdav_metadata.rs | 3 +- bin/oay/src/services/webdav/webdavfs.rs | 9 +-- bindings/haskell/src/logger.rs | 3 +- bindings/ocaml/src/lib.rs | 6 +- bindings/php/src/lib.rs | 5 +- core/fuzz/fuzz_reader.rs | 7 +- core/fuzz/fuzz_writer.rs | 8 +-- core/src/layers/blocking.rs | 3 +- core/src/services/postgresql/backend.rs | 17 +++-- core/src/types/entry.rs | 6 +- core/src/types/error.rs | 87 +++++++++++++++++++++++-- core/src/types/operator/blocking_operator.rs | 14 ++-- core/tests/behavior/blocking_write.rs | 3 +- 14 files changed, 130 insertions(+), 47 deletions(-) diff --git a/bin/oay/src/services/webdav/webdav_dir_entry.rs b/bin/oay/src/services/webdav/webdav_dir_entry.rs index 80bd5ed43..b3e4f6f11 100644 --- a/bin/oay/src/services/webdav/webdav_dir_entry.rs +++ b/bin/oay/src/services/webdav/webdav_dir_entry.rs @@ -15,9 +15,11 @@ // specific language governing permissions and limitations // under the License. -use dav_server::fs::{DavDirEntry, DavMetaData}; +use dav_server::fs::DavDirEntry; +use dav_server::fs::DavMetaData; use futures::FutureExt; -use opendal::{Entry, Operator}; +use opendal::Entry; +use opendal::Operator; use super::webdav_metadata::WebdavMetaData; diff --git a/bin/oay/src/services/webdav/webdav_metadata.rs b/bin/oay/src/services/webdav/webdav_metadata.rs index a473b3d76..03295125d 100644 --- a/bin/oay/src/services/webdav/webdav_metadata.rs +++ b/bin/oay/src/services/webdav/webdav_metadata.rs @@ -15,7 +15,8 @@ // specific language governing permissions and limitations // under the License. -use dav_server::fs::{DavMetaData, FsError}; +use dav_server::fs::DavMetaData; +use dav_server::fs::FsError; use opendal::Metadata; #[derive(Debug, Clone)] diff --git a/bin/oay/src/services/webdav/webdavfs.rs b/bin/oay/src/services/webdav/webdavfs.rs index 0a974dfd9..3ec86343a 100644 --- a/bin/oay/src/services/webdav/webdavfs.rs +++ b/bin/oay/src/services/webdav/webdavfs.rs @@ -17,7 +17,8 @@ use std::path::Path; use std::pin::Pin; -use std::task::Poll::{Pending, Ready}; +use std::task::Poll::Pending; +use std::task::Poll::Ready; use dav_server::davpath::DavPath; use dav_server::fs::DavDirEntry; @@ -30,10 +31,10 @@ use futures_util::StreamExt; use opendal::Lister; use opendal::Operator; -use crate::services::webdav::webdav_dir_entry::WebDAVDirEntry; - -use super::webdav_file::{convert_error, WebdavFile}; +use super::webdav_file::convert_error; +use super::webdav_file::WebdavFile; use super::webdav_metadata::WebdavMetaData; +use crate::services::webdav::webdav_dir_entry::WebDAVDirEntry; #[derive(Clone)] pub struct WebdavFs { diff --git a/bindings/haskell/src/logger.rs b/bindings/haskell/src/logger.rs index 1ffa84c6c..ebb34f155 100644 --- a/bindings/haskell/src/logger.rs +++ b/bindings/haskell/src/logger.rs @@ -15,7 +15,8 @@ // specific language governing permissions and limitations // under the License. -use std::ffi::{c_char, CString}; +use std::ffi::c_char; +use std::ffi::CString; pub struct HsLogger { pub callback: extern "C" fn(u32, *const c_char), diff --git a/bindings/ocaml/src/lib.rs b/bindings/ocaml/src/lib.rs index 4a6d1533c..f6ec3f2b8 100644 --- a/bindings/ocaml/src/lib.rs +++ b/bindings/ocaml/src/lib.rs @@ -15,10 +15,12 @@ // specific language governing permissions and limitations // under the License. -use ::opendal as od; -use std::collections::{BTreeMap, HashMap}; +use std::collections::BTreeMap; +use std::collections::HashMap; use std::str::FromStr; +use ::opendal as od; + mod operator; pub fn new_operator( diff --git a/bindings/php/src/lib.rs b/bindings/php/src/lib.rs index dbb85a8e6..2e5166935 100644 --- a/bindings/php/src/lib.rs +++ b/bindings/php/src/lib.rs @@ -15,6 +15,9 @@ // specific language governing permissions and limitations // under the License. +use std::collections::HashMap; +use std::str::FromStr; + use ::opendal as od; use ext_php_rs::binary::Binary; use ext_php_rs::convert::FromZval; @@ -22,8 +25,6 @@ use ext_php_rs::exception::PhpException; use ext_php_rs::flags::DataType; use ext_php_rs::prelude::*; use ext_php_rs::types::Zval; -use std::collections::HashMap; -use std::str::FromStr; #[php_class(name = "OpenDAL\\Operator")] pub struct Operator(od::BlockingOperator); diff --git a/core/fuzz/fuzz_reader.rs b/core/fuzz/fuzz_reader.rs index 4cd8a68ff..25b9db37e 100644 --- a/core/fuzz/fuzz_reader.rs +++ b/core/fuzz/fuzz_reader.rs @@ -23,14 +23,13 @@ use bytes::Bytes; use libfuzzer_sys::arbitrary::Arbitrary; use libfuzzer_sys::arbitrary::Unstructured; use libfuzzer_sys::fuzz_target; -use rand::prelude::*; -use sha2::Digest; -use sha2::Sha256; - use opendal::raw::oio::ReadExt; use opendal::raw::BytesRange; use opendal::Operator; use opendal::Result; +use rand::prelude::*; +use sha2::Digest; +use sha2::Sha256; mod utils; diff --git a/core/fuzz/fuzz_writer.rs b/core/fuzz/fuzz_writer.rs index 63c3ebb3a..4c2151035 100644 --- a/core/fuzz/fuzz_writer.rs +++ b/core/fuzz/fuzz_writer.rs @@ -17,17 +17,17 @@ #![no_main] -use bytes::{Bytes, BytesMut}; +use bytes::Bytes; +use bytes::BytesMut; use libfuzzer_sys::arbitrary::Arbitrary; use libfuzzer_sys::arbitrary::Unstructured; use libfuzzer_sys::fuzz_target; +use opendal::Operator; +use opendal::Result; use rand::prelude::*; use sha2::Digest; use sha2::Sha256; -use opendal::Operator; -use opendal::Result; - mod utils; const MAX_DATA_SIZE: usize = 16 * 1024 * 1024; diff --git a/core/src/layers/blocking.rs b/core/src/layers/blocking.rs index a9f8e0df4..c6fe23bb9 100644 --- a/core/src/layers/blocking.rs +++ b/core/src/layers/blocking.rs @@ -221,9 +221,8 @@ impl<I: oio::Page> oio::BlockingPage for BlockingWrapper<I> { mod tests { use once_cell::sync::Lazy; - use crate::types::Result; - use super::*; + use crate::types::Result; static RUNTIME: Lazy<tokio::runtime::Runtime> = Lazy::new(|| { tokio::runtime::Builder::new_multi_thread() diff --git a/core/src/services/postgresql/backend.rs b/core/src/services/postgresql/backend.rs index 7600a5bde..23eb127dd 100644 --- a/core/src/services/postgresql/backend.rs +++ b/core/src/services/postgresql/backend.rs @@ -15,16 +15,21 @@ // specific language governing permissions and limitations // under the License. -use crate::raw::adapters::kv; -use crate::raw::*; -use crate::*; -use async_trait::async_trait; use std::collections::HashMap; -use std::fmt::{Debug, Formatter}; +use std::fmt::Debug; +use std::fmt::Formatter; use std::str::FromStr; use std::sync::Arc; + +use async_trait::async_trait; use tokio::sync::OnceCell; -use tokio_postgres::{Client, Config, Statement}; +use tokio_postgres::Client; +use tokio_postgres::Config; +use tokio_postgres::Statement; + +use crate::raw::adapters::kv; +use crate::raw::*; +use crate::*; /// [Postgresql](https://www.postgresql.org/) services support. #[doc = include_str!("docs.md")] diff --git a/core/src/types/entry.rs b/core/src/types/entry.rs index d6a062828..86121293c 100644 --- a/core/src/types/entry.rs +++ b/core/src/types/entry.rs @@ -175,11 +175,7 @@ impl Entry { /// let (path, meta) = entry.into_parts(); /// match meta.mode() { /// EntryMode::FILE => { - /// println!( - /// "Handling file {} with size {}", - /// path, - /// meta.content_length() - /// ) + /// println!("Handling file {} with size {}", path, meta.content_length()) /// } /// EntryMode::DIR => { /// println!("Handling dir {}", path) diff --git a/core/src/types/error.rs b/core/src/types/error.rs index e4266a96f..cd1def506 100644 --- a/core/src/types/error.rs +++ b/core/src/types/error.rs @@ -35,6 +35,8 @@ //! # } //! ``` +use std::backtrace::Backtrace; +use std::backtrace::BacktraceStatus; use std::fmt; use std::fmt::Debug; use std::fmt::Display; @@ -172,6 +174,65 @@ impl Display for ErrorStatus { } /// Error is the error struct returned by all opendal functions. +/// +/// ## Display +/// +/// Error can be displayed in two ways: +/// +/// - Via `Display`: like `err.to_string()` or `format!("{err}")` +/// +/// Error will be printed in a single line: +/// +/// ```shell +/// Unexpected, context: { path: /path/to/file, called: send_async } => something wrong happened, source: networking error" +/// ``` +/// +/// - Via `Debug`: like `format!("{err:?}")` +/// +/// Error will be printed in multi lines with more details and backtraces (if captured): +/// +/// ```shell +/// Unexpected => something wrong happened +/// +/// Context: +/// path: /path/to/file +/// called: send_async +/// +/// Source: networking error +/// +/// Backtrace: +/// 0: opendal::error::Error::new +/// at ./src/error.rs:197:24 +/// 1: opendal::error::tests::generate_error +/// at ./src/error.rs:241:9 +/// 2: opendal::error::tests::test_error_debug_with_backtrace::{{closure}} +/// at ./src/error.rs:305:41 +/// ... +/// ``` +/// +/// - For conventional struct-style Debug representation, like `format!("{err:#?}")`: +/// +/// ```shell +/// Error { +/// kind: Unexpected, +/// message: "something wrong happened", +/// status: Permanent, +/// operation: "Read", +/// context: [ +/// ( +/// "path", +/// "/path/to/file", +/// ), +/// ( +/// "called", +/// "send_async", +/// ), +/// ], +/// source: Some( +/// "networking error", +/// ), +/// } +/// ``` pub struct Error { kind: ErrorKind, message: String, @@ -180,6 +241,7 @@ pub struct Error { operation: &'static str, context: Vec<(&'static str, String)>, source: Option<anyhow::Error>, + backtrace: Backtrace, } impl Display for Error { @@ -236,12 +298,18 @@ impl Debug for Error { writeln!(f)?; writeln!(f, "Context:")?; for (k, v) in self.context.iter() { - writeln!(f, " {k}: {v}")?; + writeln!(f, " {k}: {v}")?; } } if let Some(source) = &self.source { writeln!(f)?; - writeln!(f, "Source: {source:?}")?; + writeln!(f, "Source:")?; + writeln!(f, " {source:#}")?; + } + if self.backtrace.status() == BacktraceStatus::Captured { + writeln!(f)?; + writeln!(f, "Backtrace:")?; + writeln!(f, "{}", self.backtrace)?; } Ok(()) @@ -265,6 +333,9 @@ impl Error { operation: "", context: Vec::default(), source: None, + // `Backtrace::capture()` will check if backtrace has been enabled + // internally. It's zero cost if backtrace is disabled. + backtrace: Backtrace::capture(), } } @@ -359,6 +430,7 @@ impl From<Error> for io::Error { mod tests { use anyhow::anyhow; use once_cell::sync::Lazy; + use pretty_assertions::assert_eq; use super::*; @@ -372,6 +444,7 @@ mod tests { ("called", "send_async".to_string()), ], source: Some(anyhow!("networking error")), + backtrace: Backtrace::disabled(), }); #[test] @@ -380,7 +453,8 @@ mod tests { assert_eq!( s, r#"Unexpected (permanent) at Read, context: { path: /path/to/file, called: send_async } => something wrong happened, source: networking error"# - ) + ); + println!("{:#?}", Lazy::force(&TEST_ERROR)); } #[test] @@ -391,10 +465,11 @@ mod tests { r#"Unexpected (permanent) at Read => something wrong happened Context: - path: /path/to/file - called: send_async + path: /path/to/file + called: send_async -Source: networking error +Source: + networking error "# ) } diff --git a/core/src/types/operator/blocking_operator.rs b/core/src/types/operator/blocking_operator.rs index 8a9027df5..ade1086da 100644 --- a/core/src/types/operator/blocking_operator.rs +++ b/core/src/types/operator/blocking_operator.rs @@ -737,9 +737,9 @@ impl BlockingOperator { /// /// ```no_run /// # use anyhow::Result; + /// use opendal::BlockingOperator; /// use opendal::EntryMode; /// use opendal::Metakey; - /// use opendal::BlockingOperator; /// # fn test(op: BlockingOperator) -> Result<()> { /// let mut entries = op.list("path/to/dir/")?; /// for entry in entries { @@ -786,9 +786,9 @@ impl BlockingOperator { /// /// ```no_run /// # use anyhow::Result; + /// use opendal::BlockingOperator; /// use opendal::EntryMode; /// use opendal::Metakey; - /// use opendal::BlockingOperator; /// # fn test(op: BlockingOperator) -> Result<()> { /// let mut entries = op.list_with("prefix/").delimiter("").call()?; /// for entry in entries { @@ -810,9 +810,9 @@ impl BlockingOperator { /// /// ```no_run /// # use anyhow::Result; + /// use opendal::BlockingOperator; /// use opendal::EntryMode; /// use opendal::Metakey; - /// use opendal::BlockingOperator; /// # fn test(op: BlockingOperator) -> Result<()> { /// let mut entries = op /// .list_with("dir/") @@ -888,9 +888,9 @@ impl BlockingOperator { /// # use anyhow::Result; /// # use futures::io; /// use futures::TryStreamExt; + /// use opendal::BlockingOperator; /// use opendal::EntryMode; /// use opendal::Metakey; - /// use opendal::BlockingOperator; /// # fn test(op: BlockingOperator) -> Result<()> { /// let mut ds = op.lister("path/to/dir/")?; /// for de in ds { @@ -926,9 +926,9 @@ impl BlockingOperator { /// # use anyhow::Result; /// # use futures::io; /// use futures::TryStreamExt; + /// use opendal::BlockingOperator; /// use opendal::EntryMode; /// use opendal::Metakey; - /// use opendal::BlockingOperator; /// # fn test(op: BlockingOperator) -> Result<()> { /// let mut ds = op /// .lister_with("path/to/dir/") @@ -957,9 +957,9 @@ impl BlockingOperator { /// # use anyhow::Result; /// # use futures::io; /// use futures::TryStreamExt; + /// use opendal::BlockingOperator; /// use opendal::EntryMode; /// use opendal::Metakey; - /// use opendal::BlockingOperator; /// # fn test(op: BlockingOperator) -> Result<()> { /// let mut ds = op.lister_with("path/to/dir/").delimiter("").call()?; /// for entry in ds { @@ -984,9 +984,9 @@ impl BlockingOperator { /// # use anyhow::Result; /// # use futures::io; /// use futures::TryStreamExt; + /// use opendal::BlockingOperator; /// use opendal::EntryMode; /// use opendal::Metakey; - /// use opendal::BlockingOperator; /// # fn test(op: BlockingOperator) -> Result<()> { /// let mut ds = op /// .lister_with("path/to/dir/") diff --git a/core/tests/behavior/blocking_write.rs b/core/tests/behavior/blocking_write.rs index 62838d8e0..2b4f65a91 100644 --- a/core/tests/behavior/blocking_write.rs +++ b/core/tests/behavior/blocking_write.rs @@ -19,7 +19,8 @@ use std::io::Read; use std::io::Seek; use anyhow::Result; -use log::{debug, warn}; +use log::debug; +use log::warn; use sha2::Digest; use sha2::Sha256;
