fix(storage): classify blob_exists too — it is the migration's first probe
The previous commit fixed get / get-range / stat but left `blob_exists`
returning `internal_error` on S3 and Azure, which undoes the point of
the exercise: `blob_exists` is the FIRST call `backend_migration` makes
against the source for every blob.
match self.source.blob_exists(hash).await { // migration, per blob
An unclassified error there is permanent, so a refused connection during
a migration takes the permanent branch — record a finding and move on —
which is the skip-and-advance behaviour the pause was added to prevent.
The classification has to hold at the probe, not only at the read that
follows it.
Both now classify before deciding: only a genuine 404 / `is_not_found`
answers "absent", everything else keeps its transient class. On S3 that
means classifying the `SdkError` by reference first, since
`into_service_error()` consumes it.
Local was already routed through `local_io_error` at its stat site.
Audited the rest of the S3 surface: initialize, put ×3, get, get-range,
delete, stat, list and exists all classify. The two remaining
`internal_error`s in `put_blob` read a *local* source file, so there is
no network class to preserve.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -393,9 +393,13 @@ impl BlobStorageBackend for AzureBlobBackend {
|
||||
if status == Some(azure_core::StatusCode::NotFound) {
|
||||
Ok(false)
|
||||
} else {
|
||||
Err(DomainError::internal_error(
|
||||
"Azure",
|
||||
format!("Failed to check blob {hash}: {e}"),
|
||||
// Only the 404 means "absent"; everything else keeps
|
||||
// its transient/permanent class so the migration's
|
||||
// source probe can pause on an outage instead of
|
||||
// recording a permanent finding.
|
||||
Err(azure_domain_error(
|
||||
format!("Failed to check blob {hash}"),
|
||||
&e,
|
||||
))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -405,15 +405,17 @@ impl BlobStorageBackend for S3BlobBackend {
|
||||
{
|
||||
Ok(_) => Ok(true),
|
||||
Err(e) => {
|
||||
// Check if it's a 404 (not found) vs an actual error
|
||||
let service_err = e.into_service_error();
|
||||
if service_err.is_not_found() {
|
||||
// A 404 is the only answer that means "absent". Classify
|
||||
// before consuming the SdkError so everything else keeps
|
||||
// its transient/permanent class: this is the migration's
|
||||
// source probe, and a refused connection reported as a
|
||||
// plain failure would be treated as permanent.
|
||||
let classified =
|
||||
s3_domain_error("S3", format!("Failed to check blob {hash}"), &e);
|
||||
if e.into_service_error().is_not_found() {
|
||||
Ok(false)
|
||||
} else {
|
||||
Err(DomainError::internal_error(
|
||||
"S3",
|
||||
format!("Failed to check blob {}: {}", hash, service_err),
|
||||
))
|
||||
Err(classified)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user