From eba22f4c2cf012953f60f0af93a50399633e8803 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 7 Sep 2026 22:27:10 +0200 Subject: [PATCH] =?UTF-8?q?fix(storage):=20classify=20blob=5Fexists=20too?= =?UTF-8?q?=20=E2=80=94=20it=20is=20the=20migration's=20first=20probe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../services/azure_blob_backend.rs | 10 +++++++--- src/infrastructure/services/s3_blob_backend.rs | 16 +++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/infrastructure/services/azure_blob_backend.rs b/src/infrastructure/services/azure_blob_backend.rs index 03feb291..f5e29587 100644 --- a/src/infrastructure/services/azure_blob_backend.rs +++ b/src/infrastructure/services/azure_blob_backend.rs @@ -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, )) } } diff --git a/src/infrastructure/services/s3_blob_backend.rs b/src/infrastructure/services/s3_blob_backend.rs index d61d90e8..ed7df48c 100644 --- a/src/infrastructure/services/s3_blob_backend.rs +++ b/src/infrastructure/services/s3_blob_backend.rs @@ -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) } } }