diff --git a/src/server/services/s3-storage.service.ts b/src/server/services/s3-storage.service.ts index 600e5dd..9bee3c4 100644 --- a/src/server/services/s3-storage.service.ts +++ b/src/server/services/s3-storage.service.ts @@ -1,4 +1,5 @@ import { + DeleteObjectCommand, DeleteObjectsCommand, ListObjectsV2Command, S3Client, @@ -99,19 +100,81 @@ export async function deleteS3Objects( if (keys.length === 0) return; const client = createS3Client(destination); + await deleteS3ObjectsWithClient(client, destination.bucket, keys); +} + +type S3DeleteClient = Pick; + +export function isMissingContentMd5Error(error: unknown) { + if (!error || typeof error !== "object") return false; + + const details = error as { + Code?: unknown; + code?: unknown; + name?: unknown; + message?: unknown; + }; + const code = String(details.Code ?? details.code ?? details.name ?? ""); + const message = String(details.message ?? ""); + + return ( + (code === "InvalidRequest" || code === "MissingContentMD5") && + /content-?md5/i.test(message) + ); +} + +async function deleteS3ObjectsIndividually( + client: S3DeleteClient, + bucket: string, + keys: string[], +) { + const failedKeys: string[] = []; + + for (const key of keys) { + try { + await client.send(new DeleteObjectCommand({ Bucket: bucket, Key: key })); + } catch { + failedKeys.push(key); + } + } + + if (failedKeys.length > 0) { + throw new Error( + `Failed to delete ${failedKeys.length} S3 object(s): ${failedKeys.join(", ")}`, + ); + } +} + +export async function deleteS3ObjectsWithClient( + client: S3DeleteClient, + bucket: string, + keys: string[], +) { for (let offset = 0; offset < keys.length; offset += 1000) { const identifiers: ObjectIdentifier[] = keys .slice(offset, offset + 1000) .map((key) => ({ Key: key })); - const response = await client.send( - new DeleteObjectsCommand({ - Bucket: destination.bucket, - Delete: { Objects: identifiers, Quiet: true }, - }), - ); + let response; + try { + response = await client.send( + new DeleteObjectsCommand({ + Bucket: bucket, + Delete: { Objects: identifiers, Quiet: true }, + }), + ); + } catch (error) { + if (!isMissingContentMd5Error(error)) throw error; - if (response.Errors && response.Errors.length > 0) { + await deleteS3ObjectsIndividually( + client, + bucket, + identifiers.map(({ Key }) => Key).filter((key): key is string => Boolean(key)), + ); + continue; + } + + if ("Errors" in response && response.Errors && response.Errors.length > 0) { const failedKeys = response.Errors.map((error) => error.Key).filter( (key): key is string => Boolean(key), ); @@ -121,4 +184,3 @@ export async function deleteS3Objects( } } } - diff --git a/tests/backend/s3-storage-retention.test.ts b/tests/backend/s3-storage-retention.test.ts index 6e76f26..412a603 100644 --- a/tests/backend/s3-storage-retention.test.ts +++ b/tests/backend/s3-storage-retention.test.ts @@ -1,6 +1,11 @@ import assert from "node:assert/strict"; import test from "node:test"; +import { DeleteObjectCommand, DeleteObjectsCommand } from "@aws-sdk/client-s3"; import { getExpiredObjectKeys } from "../../src/server/services/s3-storage-retention.service"; +import { + deleteS3ObjectsWithClient, + isMissingContentMd5Error, +} from "../../src/server/services/s3-storage.service"; const now = Date.parse("2026-07-15T00:00:00.000Z"); @@ -70,3 +75,45 @@ test("maximum backup count keeps the newest objects", () => { assert.deepEqual(keys, ["oldest.tar.gz"]); }); +test("S3 deletion falls back to individual requests when batch MD5 is rejected", async () => { + const commands: unknown[] = []; + const client = { + async send(command: unknown) { + commands.push(command); + if (command instanceof DeleteObjectsCommand) { + throw Object.assign(new Error("Missing required header for this request: Content-MD5"), { + Code: "InvalidRequest", + }); + } + return {}; + }, + }; + + await deleteS3ObjectsWithClient( + client as never, + "bucket", + ["backups/one.tar.gz", "backups/two.tar.gz"], + ); + + assert.equal(commands.length, 3); + assert.ok(commands[0] instanceof DeleteObjectsCommand); + assert.ok(commands[1] instanceof DeleteObjectCommand); + assert.ok(commands[2] instanceof DeleteObjectCommand); +}); + +test("S3 deletion does not hide unrelated batch failures", async () => { + const failure = Object.assign(new Error("Access denied"), { + Code: "AccessDenied", + }); + const client = { + async send() { + throw failure; + }, + }; + + assert.equal(isMissingContentMd5Error(failure), false); + await assert.rejects( + deleteS3ObjectsWithClient(client as never, "bucket", ["backup.tar.gz"]), + failure, + ); +});