diff --git a/objectstore-service/src/backend/bigtable.rs b/objectstore-service/src/backend/bigtable.rs index 7ab842e4..015e8ff9 100644 --- a/objectstore-service/src/backend/bigtable.rs +++ b/objectstore-service/src/backend/bigtable.rs @@ -1133,27 +1133,30 @@ impl HighVolumeBackend for BigTableBackend { /// required by BigTable, the resulting timestamp has millisecond precision, with the last digits at /// 0. fn ttl_to_micros(ttl: Duration, from: SystemTime) -> Result { - let deadline = from.checked_add(ttl).ok_or_else(|| Error::Generic { - context: format!( + let deadline = from.checked_add(ttl).ok_or_else(|| { + Error::generic(format!( "TTL duration overflow: {} plus {}s cannot be represented as SystemTime", humantime::format_rfc3339_seconds(from), ttl.as_secs() - ), - cause: None, + )) })?; let millis = deadline .duration_since(SystemTime::UNIX_EPOCH) - .map_err(|e| Error::Generic { - context: format!( - "unable to get duration since UNIX_EPOCH for SystemTime {}", - humantime::format_rfc3339_seconds(deadline) - ), - cause: Some(Box::new(e)), + .map_err(|e| { + Error::generic_cause( + format!( + "unable to get duration since UNIX_EPOCH for SystemTime {}", + humantime::format_rfc3339_seconds(deadline) + ), + e, + ) })? .as_millis(); - (millis * 1000).try_into().map_err(|e| Error::Generic { - context: format!("failed to convert {millis}ms to i64 microseconds"), - cause: Some(Box::new(e)), + (millis * 1000).try_into().map_err(|e| { + Error::generic_cause( + format!("failed to convert {millis}ms to i64 microseconds"), + e, + ) }) } @@ -1186,10 +1189,10 @@ where Ok(res) => return Ok(res), Err(e) if retry_count >= REQUEST_RETRY_COUNT || !is_retryable(&e) => { objectstore_metrics::count!("bigtable.failures", action = context); - return Err(Error::Generic { - context: format!("Bigtable: `{context}` failed"), - cause: Some(Box::new(e)), - }); + return Err(Error::generic_cause( + format!("Bigtable: `{context}` failed"), + e, + )); } Err(e) => { retry_count += 1; diff --git a/objectstore-service/src/backend/gcs.rs b/objectstore-service/src/backend/gcs.rs index d43f3eb8..927f2347 100644 --- a/objectstore-service/src/backend/gcs.rs +++ b/objectstore-service/src/backend/gcs.rs @@ -203,9 +203,8 @@ impl GcsObject { .size .map(|size| size.parse()) .transpose() - .map_err(|e| Error::Generic { - context: "GCS: failed to parse size from object metadata".to_string(), - cause: Some(Box::new(e)), + .map_err(|e| { + Error::generic_cause("GCS: failed to parse size from object metadata", e) })?; let time_created = self.time_created; @@ -215,12 +214,9 @@ impl GcsObject { if let GcsMetaKey::Custom(custom_key) = key { custom.insert(custom_key, value); } else { - return Err(Error::Generic { - context: format!( - "GCS: unexpected built-in metadata key in object metadata: {key}" - ), - cause: None, - }); + return Err(Error::generic(format!( + "GCS: unexpected built-in metadata key in object metadata: {key}" + ))); } } @@ -314,23 +310,19 @@ fn metadata_to_gcs_headers(metadata: &Metadata) -> Result { let formatted = humantime::format_rfc3339_seconds(custom_time); headers.insert( HeaderName::from_static("x-goog-custom-time"), - formatted.to_string().parse().map_err(|e| Error::Generic { - context: "GCS: invalid custom-time header value".into(), - cause: Some(Box::new(e)), - })?, + formatted + .to_string() + .parse() + .map_err(|e| Error::generic_cause("GCS: invalid custom-time header value", e))?, ); } if let Some(compression) = metadata.compression { headers.insert( header::CONTENT_ENCODING, - compression - .to_string() - .parse() - .map_err(|e| Error::Generic { - context: "GCS: invalid content-encoding header value".into(), - cause: Some(Box::new(e)), - })?, + compression.to_string().parse().map_err(|e| { + Error::generic_cause("GCS: invalid content-encoding header value", e) + })?, ); } @@ -364,13 +356,11 @@ fn insert_gcs_meta_header( ) -> Result<()> { let header_name = format!("x-goog-meta-{key}"); headers.insert( - HeaderName::try_from(&header_name).map_err(|e| Error::Generic { - context: format!("GCS: invalid header name: {header_name}"), - cause: Some(Box::new(e)), + HeaderName::try_from(&header_name).map_err(|e| { + Error::generic_cause(format!("GCS: invalid header name: {header_name}"), e) })?, - value.parse().map_err(|e| Error::Generic { - context: format!("GCS: invalid header value for {header_name}"), - cause: Some(Box::new(e)), + value.parse().map_err(|e| { + Error::generic_cause(format!("GCS: invalid header value for {header_name}"), e) })?, ); Ok(()) @@ -438,12 +428,11 @@ impl GcsBackend { let path = id.as_storage_path().to_string(); url.path_segments_mut() - .map_err(|()| Error::Generic { - context: format!( + .map_err(|()| { + Error::generic(format!( "GCS: invalid endpoint URL, {} cannot be a base", self.endpoint - ), - cause: None, + )) })? .extend(&["storage", "v1", "b", &self.bucket, "o", &path]); @@ -455,12 +444,11 @@ impl GcsBackend { let mut url = self.endpoint.clone(); url.path_segments_mut() - .map_err(|()| Error::Generic { - context: format!( + .map_err(|()| { + Error::generic(format!( "GCS: invalid endpoint URL, {} cannot be a base", self.endpoint - ), - cause: None, + )) })? .extend(&["upload", "storage", "v1", "b", &self.bucket, "o"]); @@ -480,12 +468,11 @@ impl GcsBackend { fn xml_object_url(&self, id: &ObjectId) -> Result { let mut url = self.endpoint.clone(); { - let mut segments = url.path_segments_mut().map_err(|()| Error::Generic { - context: format!( + let mut segments = url.path_segments_mut().map_err(|()| { + Error::generic(format!( "GCS: invalid endpoint URL, {} cannot be a base", self.endpoint - ), - cause: None, + )) })?; segments.push(&self.bucket); for part in id.as_storage_path().to_string().split('/') { @@ -636,10 +623,8 @@ impl Backend for GcsBackend { // NB: Ensure the order of these fields and that a content-type is attached to them. Both // are required by the GCS API. - let metadata_json = serde_json::to_string(&gcs_metadata).map_err(|cause| Error::Serde { - context: "failed to serialize metadata for GCS upload".to_string(), - cause, - })?; + let metadata_json = serde_json::to_string(&gcs_metadata) + .map_err(|cause| Error::serde("failed to serialize metadata for GCS upload", cause))?; let multipart = multipart::Form::new() .part( @@ -652,9 +637,11 @@ impl Backend for GcsBackend { "media", multipart::Part::stream(Body::wrap_stream(stream)) .mime_str(&metadata.content_type) - .map_err(|e| Error::Generic { - context: format!("invalid mime type: {}", &metadata.content_type), - cause: Some(Box::new(e)), + .map_err(|e| { + Error::generic_cause( + format!("invalid mime type: {}", &metadata.content_type), + e, + ) })?, ); @@ -707,12 +694,9 @@ impl Backend for GcsBackend { match total { Some(total) => return Err(Error::RangeNotSatisfiable { total }), None => { - return Err(Error::Generic { - context: format!( - "GCS: 416 response with invalid Content-Range: {raw:?}" - ), - cause: None, - }); + return Err(Error::generic(format!( + "GCS: 416 response with invalid Content-Range: {raw:?}" + ))); } } } @@ -728,9 +712,8 @@ impl Backend for GcsBackend { .get(header::CONTENT_RANGE) .and_then(|v| v.to_str().ok()) .and_then(|s| s.parse::().ok()) - .ok_or_else(|| Error::Generic { - context: "GCS: 206 response missing valid Content-Range header".to_owned(), - cause: None, + .ok_or_else(|| { + Error::generic("GCS: 206 response missing valid Content-Range header") })?, ) } else { @@ -920,9 +903,8 @@ impl MultipartUploadBackend for GcsBackend { .map_err(|e| Error::reqwest("GCS: read initiate multipart body", e))?; let xml: XmlInitiateMultipartUploadResponse = quick_xml::de::from_reader(body.as_ref()) - .map_err(|e| Error::Generic { - context: "GCS: failed to parse initiate multipart response".to_owned(), - cause: Some(Box::new(e)), + .map_err(|e| { + Error::generic_cause("GCS: failed to parse initiate multipart response", e) })?; Ok(xml.try_into()?) @@ -1000,11 +982,8 @@ impl MultipartUploadBackend for GcsBackend { .await .map_err(|e| Error::reqwest("GCS: read list parts body", e))?; - let xml: XmlListPartsResponse = - quick_xml::de::from_reader(body.as_ref()).map_err(|e| Error::Generic { - context: "GCS: failed to parse list parts response".to_owned(), - cause: Some(Box::new(e)), - })?; + let xml: XmlListPartsResponse = quick_xml::de::from_reader(body.as_ref()) + .map_err(|e| Error::generic_cause("GCS: failed to parse list parts response", e))?; Ok(xml.into()) } @@ -1041,9 +1020,8 @@ impl MultipartUploadBackend for GcsBackend { url.query_pairs_mut().append_pair("uploadId", upload_id); let body = XmlCompleteMultipartUpload::from(parts); - let xml = quick_xml::se::to_string(&body).map_err(|e| Error::Generic { - context: "GCS: failed to serialize complete multipart request".into(), - cause: Some(Box::new(e)), + let xml = quick_xml::se::to_string(&body).map_err(|e| { + Error::generic_cause("GCS: failed to serialize complete multipart request", e) })?; let resp = self diff --git a/objectstore-service/src/backend/local_fs.rs b/objectstore-service/src/backend/local_fs.rs index ae3f8c86..3c13f570 100644 --- a/objectstore-service/src/backend/local_fs.rs +++ b/objectstore-service/src/backend/local_fs.rs @@ -96,10 +96,8 @@ impl Backend for LocalFsBackend { let mut reader = pin!(StreamReader::new(stream)); let mut writer = BufWriter::new(file); - let metadata_json = serde_json::to_string(metadata).map_err(|cause| Error::Serde { - context: "failed to serialize metadata".to_string(), - cause, - })?; + let metadata_json = serde_json::to_string(metadata) + .map_err(|cause| Error::serde("failed to serialize metadata", cause))?; writer.write_all(metadata_json.as_bytes()).await?; writer.write_all(b"\n").await?; @@ -136,11 +134,8 @@ impl Backend for LocalFsBackend { let mut metadata_line = String::new(); reader.read_line(&mut metadata_line).await?; let file_len = reader.get_ref().metadata().await?.len(); - let mut metadata: Metadata = - serde_json::from_str(metadata_line.trim_end()).map_err(|cause| Error::Serde { - context: "failed to deserialize metadata".to_string(), - cause, - })?; + let mut metadata: Metadata = serde_json::from_str(metadata_line.trim_end()) + .map_err(|cause| Error::serde("failed to deserialize metadata", cause))?; let payload_size = file_len .checked_sub(metadata_line.len() as u64) .ok_or_else(|| Error::generic("local-fs file corrupted: shorter than header"))?; @@ -199,10 +194,8 @@ impl MultipartUploadBackend for LocalFsBackend { tokio::fs::create_dir_all(&dir).await?; let meta_path = dir.join("metadata.json"); - let metadata_json = serde_json::to_string(metadata).map_err(|cause| Error::Serde { - context: "failed to serialize multipart metadata".to_string(), - cause, - })?; + let metadata_json = serde_json::to_string(metadata) + .map_err(|cause| Error::serde("failed to serialize multipart metadata", cause))?; tokio::fs::write(meta_path, metadata_json).await?; Ok(upload_id) @@ -229,10 +222,8 @@ impl MultipartUploadBackend for LocalFsBackend { "uploaded_at": SystemTime::now(), "size": content_length, }); - let header_line = serde_json::to_string(&header).map_err(|cause| Error::Serde { - context: "failed to serialize part header".to_string(), - cause, - })?; + let header_line = serde_json::to_string(&header) + .map_err(|cause| Error::serde("failed to serialize part header", cause))?; let part_path = dir.join(format!("{part_number}.part")); let file = OpenOptions::new() @@ -299,11 +290,8 @@ impl MultipartUploadBackend for LocalFsBackend { let mut reader = BufReader::new(file); let mut header_line = String::new(); reader.read_line(&mut header_line).await?; - let header: serde_json::Value = - serde_json::from_str(header_line.trim_end()).map_err(|cause| Error::Serde { - context: "failed to deserialize part header".to_string(), - cause, - })?; + let header: serde_json::Value = serde_json::from_str(header_line.trim_end()) + .map_err(|cause| Error::serde("failed to deserialize part header", cause))?; parts.push(Part { part_number: pn, @@ -359,11 +347,8 @@ impl MultipartUploadBackend for LocalFsBackend { // Read metadata let meta_path = dir.join("metadata.json"); let meta_bytes = tokio::fs::read(&meta_path).await?; - let metadata: Metadata = - serde_json::from_slice(&meta_bytes).map_err(|cause| Error::Serde { - context: "failed to deserialize multipart metadata".to_string(), - cause, - })?; + let metadata: Metadata = serde_json::from_slice(&meta_bytes) + .map_err(|cause| Error::serde("failed to deserialize multipart metadata", cause))?; // TODO: validate that parts are in ascending part_number order and reject with // InvalidPartOrder if not (matches S3/GCS behavior). Needs a proper client error variant. @@ -382,11 +367,8 @@ impl MultipartUploadBackend for LocalFsBackend { let mut reader = BufReader::new(file); let mut header_line = String::new(); reader.read_line(&mut header_line).await?; - let header: serde_json::Value = - serde_json::from_str(header_line.trim_end()).map_err(|cause| Error::Serde { - context: "failed to deserialize part header".to_string(), - cause, - })?; + let header: serde_json::Value = serde_json::from_str(header_line.trim_end()) + .map_err(|cause| Error::serde("failed to deserialize part header", cause))?; let stored_etag = header["etag"].as_str().unwrap_or(""); if stored_etag != completed.etag { @@ -411,10 +393,8 @@ impl MultipartUploadBackend for LocalFsBackend { .await?; let mut writer = BufWriter::new(file); - let metadata_json = serde_json::to_string(&metadata).map_err(|cause| Error::Serde { - context: "failed to serialize metadata".to_string(), - cause, - })?; + let metadata_json = serde_json::to_string(&metadata) + .map_err(|cause| Error::serde("failed to serialize metadata", cause))?; writer.write_all(metadata_json.as_bytes()).await?; writer.write_all(b"\n").await?; diff --git a/objectstore-service/src/backend/s3_compatible.rs b/objectstore-service/src/backend/s3_compatible.rs index 6987cd3a..77eca3d9 100644 --- a/objectstore-service/src/backend/s3_compatible.rs +++ b/objectstore-service/src/backend/s3_compatible.rs @@ -148,7 +148,7 @@ where .get_token() .await .map_err(|err| Error::Generic { - context: "S3: failed to get authentication token".to_owned(), + context: "S3: failed to get authentication token".into(), cause: Some(err.into()), })? .as_str(), @@ -172,10 +172,10 @@ where if let Some(r) = range { builder = builder.header(reqwest::header::RANGE, r.to_header_value()); } - let response = builder.send().await.map_err(|cause| Error::Reqwest { - context: "S3: failed to send request".to_string(), - cause, - })?; + let response = builder + .send() + .await + .map_err(|cause| Error::reqwest("S3: failed to send request", cause))?; if response.status() == StatusCode::NOT_FOUND { objectstore_log::debug!("Object not found"); @@ -191,10 +191,9 @@ where match total { Some(total) => return Err(Error::RangeNotSatisfiable { total }), None => { - return Err(Error::Generic { - context: format!("S3: 416 response with invalid Content-Range: {raw:?}"), - cause: None, - }); + return Err(Error::generic(format!( + "S3: 416 response with invalid Content-Range: {raw:?}" + ))); } } } @@ -209,9 +208,8 @@ where .get(reqwest::header::CONTENT_RANGE) .and_then(|v| v.to_str().ok()) .and_then(|s| s.parse::().ok()) - .ok_or_else(|| Error::Generic { - context: "S3: 206 response missing valid Content-Range header".to_owned(), - cause: None, + .ok_or_else(|| { + Error::generic("S3: 206 response missing valid Content-Range header") })?; metadata.size = Some(range.total as usize); Some(range) @@ -341,10 +339,7 @@ impl Backend for S3CompatibleBackend { .await? .send() .await - .map_err(|cause| Error::Reqwest { - context: "S3: failed to send delete request".to_string(), - cause, - })?; + .map_err(|cause| Error::reqwest("S3: failed to send delete request", cause))?; // Do not error for objects that do not exist. if response.status() != StatusCode::NOT_FOUND { diff --git a/objectstore-service/src/backend/tiered.rs b/objectstore-service/src/backend/tiered.rs index 204613bb..878f1f2e 100644 --- a/objectstore-service/src/backend/tiered.rs +++ b/objectstore-service/src/backend/tiered.rs @@ -593,7 +593,7 @@ impl TryFrom<&UploadId> for TieredUploadId { fn try_from(value: &UploadId) -> Result { let json = base64::engine::general_purpose::URL_SAFE_NO_PAD .decode(value.as_bytes()) - .map_err(|e| Error::generic(format!("invalid multipart upload ID: {e}")))?; + .map_err(|e| Error::generic_cause("invalid multipart upload ID", e))?; serde_json::from_slice(&json).map_err(|e| Error::serde("decoding multipart token", e)) } } diff --git a/objectstore-service/src/error.rs b/objectstore-service/src/error.rs index 9643dd43..12b7c701 100644 --- a/objectstore-service/src/error.rs +++ b/objectstore-service/src/error.rs @@ -4,6 +4,7 @@ //! and backend-specific failures. [`Result`] is the corresponding alias. use std::any::Any; +use std::borrow::Cow; use std::fmt; use objectstore_log::Level; @@ -62,7 +63,7 @@ pub enum Error { #[error("serde error: {context}")] Serde { /// Context describing what was being serialized/deserialized. - context: String, + context: &'static str, /// The underlying serde error. #[source] cause: serde_json::Error, @@ -75,7 +76,7 @@ pub enum Error { #[error("reqwest error: {context}")] Reqwest { /// Context describing the request that failed. - context: String, + context: &'static str, /// The underlying reqwest error. #[source] cause: reqwest::Error, @@ -138,7 +139,7 @@ pub enum Error { #[error("storage backend error: {context}")] Generic { /// Context describing the operation that failed. - context: String, + context: Cow<'static, str>, /// The underlying error, if available. #[source] cause: Option>, @@ -167,29 +168,34 @@ impl Error { } /// Creates an [`Error::Reqwest`] from a reqwest error with context. - pub fn reqwest(context: impl Into, cause: reqwest::Error) -> Self { - Self::Reqwest { - context: context.into(), - cause, - } + pub fn reqwest(context: &'static str, cause: reqwest::Error) -> Self { + Self::Reqwest { context, cause } } /// Creates an [`Error::Serde`] from a serde error with context. - pub fn serde(context: impl Into, cause: serde_json::Error) -> Self { - Self::Serde { - context: context.into(), - cause, - } + pub fn serde(context: &'static str, cause: serde_json::Error) -> Self { + Self::Serde { context, cause } } /// Creates an [`Error::Generic`] with a context string and no cause. - pub fn generic(context: impl Into) -> Self { + pub fn generic(context: impl Into>) -> Self { Self::Generic { context: context.into(), cause: None, } } + /// Creates an [`Error::Generic`] with a context string and a cause. + pub fn generic_cause( + context: impl Into>, + cause: impl std::error::Error + Send + Sync + 'static, + ) -> Self { + Self::Generic { + context: context.into(), + cause: Some(Box::new(cause)), + } + } + /// Returns the appropriate log level for this error. pub fn level(&self) -> Level { match self {