This is an automated email from the ASF dual-hosted git repository.
xuanwo pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git
The following commit(s) were added to refs/heads/main by this push:
new 64ddcb6ec refactor: Implement backtrace for Error correctly (#2871)
64ddcb6ec is described below
commit 64ddcb6ecc37edcbb69297f5d82460e6b3aa1f9d
Author: Xuanwo <[email protected]>
AuthorDate: Wed Aug 16 15:25:38 2023 +0800
refactor: Implement backtrace for Error correctly (#2871)
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;
