You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by xu...@apache.org on 2025/05/16 08:21:47 UTC
(opendal) branch make-error-smaller created (now c6ad91060)
This is an automated email from the ASF dual-hosted git repository.
xuanwo pushed a change to branch make-error-smaller
in repository https://gitbox.apache.org/repos/asf/opendal.git
at c6ad91060 refactor(core): Put Backtrace in a box to reduce error size
This branch includes the following new commits:
new c6ad91060 refactor(core): Put Backtrace in a box to reduce error size
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(opendal) 01/01: refactor(core): Put Backtrace in a box to reduce error size
Posted by xu...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
xuanwo pushed a commit to branch make-error-smaller
in repository https://gitbox.apache.org/repos/asf/opendal.git
commit c6ad9106043618b1e704a2009d7475176b8c025f
Author: Xuanwo <gi...@xuanwo.io>
AuthorDate: Fri May 16 16:21:15 2025 +0800
refactor(core): Put Backtrace in a box to reduce error size
Signed-off-by: Xuanwo <gi...@xuanwo.io>
---
core/src/services/azblob/backend.rs | 5 +---
core/src/services/azdls/backend.rs | 5 +---
core/src/types/blocking_write/std_writer.rs | 25 ++++------------
core/src/types/error.rs | 45 +++++++++++++++++++----------
4 files changed, 38 insertions(+), 42 deletions(-)
diff --git a/core/src/services/azblob/backend.rs b/core/src/services/azblob/backend.rs
index 331d9b954..98067218f 100644
--- a/core/src/services/azblob/backend.rs
+++ b/core/src/services/azblob/backend.rs
@@ -529,10 +529,7 @@ fn infer_storage_name_from_endpoint(endpoint: &str) -> Option<String> {
.trim_end_matches('/')
.to_lowercase();
- if KNOWN_AZBLOB_ENDPOINT_SUFFIX
- .iter()
- .any(|s| *s == endpoint_suffix.as_str())
- {
+ if KNOWN_AZBLOB_ENDPOINT_SUFFIX.contains(&endpoint_suffix.as_str()) {
storage_name.map(|s| s.to_string())
} else {
None
diff --git a/core/src/services/azdls/backend.rs b/core/src/services/azdls/backend.rs
index ed3b8ef38..991ffa945 100644
--- a/core/src/services/azdls/backend.rs
+++ b/core/src/services/azdls/backend.rs
@@ -366,10 +366,7 @@ fn infer_storage_name_from_endpoint(endpoint: &str) -> Option<String> {
.trim_end_matches('/')
.to_lowercase();
- if KNOWN_AZDLS_ENDPOINT_SUFFIX
- .iter()
- .any(|s| *s == endpoint_suffix.as_str())
- {
+ if KNOWN_AZDLS_ENDPOINT_SUFFIX.contains(&endpoint_suffix.as_str()) {
storage_name.map(|s| s.to_string())
} else {
None
diff --git a/core/src/types/blocking_write/std_writer.rs b/core/src/types/blocking_write/std_writer.rs
index 91accbde5..2a53b97bb 100644
--- a/core/src/types/blocking_write/std_writer.rs
+++ b/core/src/types/blocking_write/std_writer.rs
@@ -49,14 +49,10 @@ impl StdWriter {
self.flush()?;
let Some(w) = &mut self.w else {
- return Err(std::io::Error::new(
- std::io::ErrorKind::Other,
- "writer has been closed",
- ));
+ return Err(std::io::Error::other("writer has been closed"));
};
- w.close()
- .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
+ w.close().map_err(std::io::Error::other)?;
// Drop writer after close succeed;
self.w = None;
@@ -68,10 +64,7 @@ impl StdWriter {
impl Write for StdWriter {
fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
let Some(w) = &mut self.w else {
- return Err(std::io::Error::new(
- std::io::ErrorKind::Other,
- "writer has been closed",
- ));
+ return Err(std::io::Error::other("writer has been closed"));
};
loop {
@@ -81,19 +74,14 @@ impl Write for StdWriter {
}
let bs = self.buf.get().expect("frozen buffer must be valid");
- let n = w
- .write(Buffer::from(bs))
- .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
+ let n = w.write(Buffer::from(bs)).map_err(std::io::Error::other)?;
self.buf.advance(n);
}
}
fn flush(&mut self) -> std::io::Result<()> {
let Some(w) = &mut self.w else {
- return Err(std::io::Error::new(
- std::io::ErrorKind::Other,
- "writer has been closed",
- ));
+ return Err(std::io::Error::other("writer has been closed"));
};
loop {
@@ -103,8 +91,7 @@ impl Write for StdWriter {
return Ok(());
};
- w.write(Buffer::from(bs))
- .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err))?;
+ w.write(Buffer::from(bs)).map_err(std::io::Error::other)?;
self.buf.clean();
}
}
diff --git a/core/src/types/error.rs b/core/src/types/error.rs
index 1e027b912..6b1bc7a9e 100644
--- a/core/src/types/error.rs
+++ b/core/src/types/error.rs
@@ -98,8 +98,8 @@ impl ErrorKind {
/// For some kinds of errors, backtrace is not useful and we can skip it (e.g., check if a file exists).
///
/// See <https://github.com/apache/opendal/discussions/5569>
- fn disable_backtrace(&self) -> bool {
- matches!(self, ErrorKind::NotFound)
+ fn enable_backtrace(&self) -> bool {
+ matches!(self, ErrorKind::Unexpected)
}
}
@@ -227,8 +227,9 @@ pub struct Error {
status: ErrorStatus,
operation: &'static str,
context: Vec<(&'static str, String)>,
+
source: Option<anyhow::Error>,
- backtrace: Backtrace,
+ backtrace: Option<Box<Backtrace>>,
}
impl Display for Error {
@@ -293,10 +294,11 @@ impl Debug for Error {
writeln!(f, "Source:")?;
writeln!(f, " {source:#}")?;
}
- if self.backtrace.status() == BacktraceStatus::Captured {
+
+ if let Some(backtrace) = &self.backtrace {
writeln!(f)?;
writeln!(f, "Backtrace:")?;
- writeln!(f, "{}", self.backtrace)?;
+ writeln!(f, "{}", backtrace)?;
}
Ok(())
@@ -320,13 +322,16 @@ 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: if kind.disable_backtrace() {
- Backtrace::disabled()
- } else {
- Backtrace::capture()
- },
+
+ backtrace: kind
+ .enable_backtrace()
+ // `Backtrace::capture()` will check if backtrace has
+ // been enabled internally. It's zero cost if backtrace
+ // is disabled.
+ .then(Backtrace::capture)
+ // We only keep captured backtrace to avoid an extra box.
+ .filter(|bt| bt.status() == BacktraceStatus::Captured)
+ .map(Box::new),
}
}
@@ -428,12 +433,12 @@ impl From<Error> for io::Error {
#[cfg(test)]
mod tests {
+ use super::*;
use anyhow::anyhow;
use pretty_assertions::assert_eq;
+ use std::mem::size_of;
use std::sync::LazyLock;
- use super::*;
-
static TEST_ERROR: LazyLock<Error> = LazyLock::new(|| Error {
kind: ErrorKind::Unexpected,
message: "something wrong happened".to_string(),
@@ -444,9 +449,19 @@ mod tests {
("called", "send_async".to_string()),
],
source: Some(anyhow!("networking error")),
- backtrace: Backtrace::disabled(),
+ backtrace: None,
});
+ /// This is not a real test case.
+ ///
+ /// We assert our public structs here to make sure we don't introduce
+ /// unexpected struct/enum size change.
+ #[cfg(target_pointer_width = "64")]
+ #[test]
+ fn assert_size() {
+ assert_eq!(88, size_of::<Error>());
+ }
+
#[test]
fn test_error_display() {
let s = format!("{}", LazyLock::force(&TEST_ERROR));