feat(s3): enhance S3 deletion logic to handle MD5 errors and improve test coverage
This commit is contained in:
1 parent
87fdbb493b
commit
fb3878edb8
2 files changed
+117
-8
No files matched your search
@@ -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<S3Client, "send">;
|
||||
|
||||
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(
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
);
|
||||
});
|
||||
Reference in new issue
Block a user