fix(storage): classify backend-init failures too
Ed proposed the obvious end-to-end test — point an S3 entry at 127.0.0.1 with nothing listening, get a refused connection, expect a transient error — and it would have failed, because `initialize()` was the one SDK call still wrapped as a plain `internal_error`. That is the FIRST call both jobs make, so it is what a wrong-endpoint test actually hits: `backend_consistency` and `backend_migration` each return `Failed` on init, and every classification added in the previous commits sits downstream of a path the test never reaches. Now `head_bucket` goes through `s3_domain_error` like the rest, and both call sites route through `RunOutcome::from_domain_error`. A refused connection or a 5xx pauses and can be resumed once the endpoint returns; a wrong bucket or bad credentials is 4xx and stays terminal, which is the distinction that makes pausing safe to offer at all. No cursor at init — nothing has been scanned — so the pause resumes from the start, which is correct rather than lossy. Worth noting for `backend_migration`: target init runs BEFORE `migration_readonly` is engaged, so pausing there holds no write freeze. An operator can leave it paused indefinitely and resume when the target comes back, with no read-only window. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -378,9 +378,11 @@ impl RecoverableJobHandler for BackendConsistencyCheck {
|
||||
},
|
||||
};
|
||||
if let Err(e) = backend.initialize().await {
|
||||
return RunOutcome::Failed {
|
||||
message: format!("probed backend init: {e}"),
|
||||
};
|
||||
// Nothing has been scanned yet, so there is no cursor to keep
|
||||
// — but the distinction still matters: an unreachable endpoint
|
||||
// pauses and can be resumed once it is back, while a wrong
|
||||
// bucket or bad credentials stays terminal.
|
||||
return RunOutcome::from_domain_error(None, "probed backend init", &e);
|
||||
}
|
||||
if let Some(name) = &probed_storage {
|
||||
tracing::info!(
|
||||
|
||||
@@ -452,9 +452,11 @@ impl RecoverableJobHandler for BackendMigrationService {
|
||||
// for the swap-hot-swap call in `finish_completed`.
|
||||
let target = build_entry_backend_typed(target_entry, &self.storage_path_fallback);
|
||||
if let Err(e) = target.initialize().await {
|
||||
return RunOutcome::Failed {
|
||||
message: format!("target backend init: {e}"),
|
||||
};
|
||||
// Runs BEFORE `migration_readonly` is engaged, so pausing
|
||||
// here holds no write freeze — an operator can leave it
|
||||
// paused indefinitely and resume when the target comes back.
|
||||
// A wrong bucket or bad credentials still fails terminally.
|
||||
return RunOutcome::from_domain_error(None, "target backend init", &e);
|
||||
}
|
||||
|
||||
// All guards passed. Engage server-wide read-only mode for
|
||||
|
||||
@@ -105,10 +105,14 @@ impl BlobStorageBackend for S3BlobBackend {
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error(
|
||||
"S3",
|
||||
format!("Cannot access bucket '{}': {}", self.bucket, e),
|
||||
)
|
||||
// Classified like every other SDK call. A refused
|
||||
// connection or a 5xx here is the endpoint being
|
||||
// down, not the configuration being wrong, and the
|
||||
// jobs that call `initialize()` should pause rather
|
||||
// than fail on it. A genuine misconfiguration —
|
||||
// wrong bucket, bad credentials — still lands as 4xx
|
||||
// and stays terminal.
|
||||
s3_domain_error("S3", format!("Cannot access bucket '{}'", self.bucket), &e)
|
||||
})?;
|
||||
|
||||
tracing::info!("S3 blob backend initialized: bucket={}", self.bucket);
|
||||
|
||||
Reference in New Issue
Block a user