This is an automated email from the ASF dual-hosted git repository.
tustvold pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs-object-store.git
The following commit(s) were added to refs/heads/main by this push:
new 407184a Return Non-Generic Errors from HttpStore (#366)
407184a is described below
commit 407184a4b2652c6765c0130c69b27855f2d89e4f
Author: Lukas Söder <[email protected]>
AuthorDate: Sat May 17 18:15:17 2025 +0200
Return Non-Generic Errors from HttpStore (#366)
* Adjust how RetryError is handled for http ObjectStore, convert to the
relevant crate::Error using RetryError::source.
Allows for better error handling of HTTP statuses, and makes it consistent
with how the S3/Azure/GCP clients handles RetryError.
* Remove unnecessary match statement from http client
---
src/http/client.rs | 60 ++++++++++++++++++++++++++++++++++++++----------------
src/http/mod.rs | 4 +++-
2 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/src/http/client.rs b/src/http/client.rs
index 652d326..8a96e74 100644
--- a/src/http/client.rs
+++ b/src/http/client.rs
@@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.
+use super::STORE;
use crate::client::get::GetClient;
use crate::client::header::HeaderConfig;
use crate::client::retry::{self, RetryConfig, RetryExt};
@@ -37,7 +38,10 @@ use url::Url;
#[derive(Debug, thiserror::Error)]
enum Error {
#[error("Request error: {}", source)]
- Request { source: retry::RetryError },
+ Request {
+ source: retry::RetryError,
+ path: String,
+ },
#[error("Request error: {}", source)]
Reqwest { source: HttpError },
@@ -75,9 +79,12 @@ enum Error {
impl From<Error> for crate::Error {
fn from(err: Error) -> Self {
- Self::Generic {
- store: "HTTP",
- source: Box::new(err),
+ match err {
+ Error::Request { source, path } => source.error(STORE, path),
+ _ => Self::Generic {
+ store: STORE,
+ source: Box::new(err),
+ },
}
}
}
@@ -128,7 +135,10 @@ impl Client {
.request(method, String::from(url))
.send_retry(&self.retry_config)
.await
- .map_err(|source| Error::Request { source })?;
+ .map_err(|source| Error::Request {
+ source,
+ path: path.to_string(),
+ })?;
Ok(())
}
@@ -144,7 +154,7 @@ impl Client {
match self.make_directory(prefix).await {
Ok(_) => break,
- Err(Error::Request { source })
+ Err(Error::Request { source, path: _ })
if matches!(source.status(), Some(StatusCode::CONFLICT)) =>
{
// Need to create parent
@@ -213,7 +223,13 @@ impl Client {
retry = true;
self.create_parent_directories(location).await?
}
- _ => return Err(Error::Request { source }.into()),
+ _ => {
+ return Err(Error::Request {
+ source,
+ path: location.to_string(),
+ }
+ .into())
+ }
},
}
}
@@ -255,7 +271,13 @@ impl Client {
}
};
}
- Err(source) => return Err(Error::Request { source }.into()),
+ Err(source) => {
+ return Err(Error::Request {
+ source,
+ path: location.map(|x| x.to_string()).unwrap_or_default(),
+ }
+ .into())
+ }
};
let status = quick_xml::de::from_reader(response.reader())
@@ -270,13 +292,7 @@ impl Client {
.delete(url)
.send_retry(&self.retry_config)
.await
- .map_err(|source| match source.status() {
- Some(StatusCode::NOT_FOUND) => crate::Error::NotFound {
- source: Box::new(source),
- path: path.to_string(),
- },
- _ => Error::Request { source }.into(),
- })?;
+ .map_err(|source| source.error(STORE, path.to_string()))?;
Ok(())
}
@@ -313,7 +329,11 @@ impl Client {
self.create_parent_directories(to).await?;
continue;
}
- _ => Error::Request { source }.into(),
+ _ => Error::Request {
+ source,
+ path: from.to_string(),
+ }
+ .into(),
}),
};
}
@@ -322,7 +342,7 @@ impl Client {
#[async_trait]
impl GetClient for Client {
- const STORE: &'static str = "HTTP";
+ const STORE: &'static str = STORE;
/// Override the [`HeaderConfig`] to be less strict to support a
/// broader range of HTTP servers (#4831)
@@ -354,7 +374,11 @@ impl GetClient for Client {
path: path.to_string(),
}
}
- _ => Error::Request { source }.into(),
+ _ => Error::Request {
+ source,
+ path: path.to_string(),
+ }
+ .into(),
})?;
// We expect a 206 Partial Content response if a range was requested
diff --git a/src/http/mod.rs b/src/http/mod.rs
index 9786d83..8b1f505 100644
--- a/src/http/mod.rs
+++ b/src/http/mod.rs
@@ -51,6 +51,8 @@ use crate::{
mod client;
+const STORE: &str = "HTTP";
+
#[derive(Debug, thiserror::Error)]
enum Error {
#[error("Must specify a URL")]
@@ -71,7 +73,7 @@ enum Error {
impl From<Error> for crate::Error {
fn from(err: Error) -> Self {
Self::Generic {
- store: "HTTP",
+ store: STORE,
source: Box::new(err),
}
}
