fix(apps): preserve artwork on cleanup failure
This commit is contained in:
parent
e170583399
commit
8d3487cdfd
|
|
@ -37,9 +37,6 @@ export function applyFileBatchAtomically(changes, options = {}) {
|
|||
}
|
||||
}
|
||||
|
||||
for (const entry of prepared) {
|
||||
if (entry.existed && fsApi.existsSync(entry.backup)) fsApi.unlinkSync(entry.backup);
|
||||
}
|
||||
} catch (error) {
|
||||
const rollbackErrors = [];
|
||||
for (const entry of [...prepared].reverse()) {
|
||||
|
|
@ -54,4 +51,16 @@ export function applyFileBatchAtomically(changes, options = {}) {
|
|||
const suffix = rollbackErrors.length ? ` Rollback errors: ${rollbackErrors.join("; ")}` : "";
|
||||
throw new Error(`Connector artwork import failed; the previous batch was restored.${suffix}`, { cause: error });
|
||||
}
|
||||
|
||||
// Cleanup happens only after the transaction is committed. A cleanup error
|
||||
// must never enter rollback after another backup has already been removed.
|
||||
const cleanupErrors = [];
|
||||
for (const entry of prepared) {
|
||||
try {
|
||||
if (entry.existed && fsApi.existsSync(entry.backup)) fsApi.unlinkSync(entry.backup);
|
||||
} catch (error) {
|
||||
cleanupErrors.push(`${entry.backup}: ${error.message}`);
|
||||
}
|
||||
}
|
||||
return cleanupErrors;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -56,3 +56,28 @@ test("failed artwork batches restore every previous destination", (t) => {
|
|||
assert.equal(fs.readFileSync(second, "utf8"), "second-old");
|
||||
assert.deepEqual(fs.readdirSync(directory).sort(), ["first.svg", "second.svg"]);
|
||||
});
|
||||
|
||||
test("backup cleanup failures preserve committed artwork", (t) => {
|
||||
const directory = fs.mkdtempSync(path.join(os.tmpdir(), "paperclip-brand-cleanup-"));
|
||||
t.after(() => fs.rmSync(directory, { recursive: true, force: true }));
|
||||
const target = path.join(directory, "icon.svg");
|
||||
fs.writeFileSync(target, "old");
|
||||
const fsApi = {
|
||||
...fs,
|
||||
unlinkSync(file) {
|
||||
if (file.endsWith(".bak")) throw new Error("simulated cleanup failure");
|
||||
return fs.unlinkSync(file);
|
||||
},
|
||||
};
|
||||
|
||||
const cleanupErrors = applyFileBatchAtomically(
|
||||
[{ target, bytes: Buffer.from("new") }],
|
||||
{ fsApi, transactionId: "cleanup-test" },
|
||||
);
|
||||
assert.equal(fs.readFileSync(target, "utf8"), "new");
|
||||
assert.equal(cleanupErrors.length, 1);
|
||||
assert.equal(
|
||||
fs.readFileSync(`${target}.paperclip-cleanup-test.bak`, "utf8"),
|
||||
"old",
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -27,5 +27,10 @@ for (const [asset, bytes] of pending) {
|
|||
changed++;
|
||||
changes.push({ target, bytes });
|
||||
}
|
||||
if (args.includes("--apply")) applyFileBatchAtomically(changes);
|
||||
if (args.includes("--apply")) {
|
||||
const cleanupErrors = applyFileBatchAtomically(changes);
|
||||
if (cleanupErrors.length) {
|
||||
console.warn(`Artwork applied, but ${cleanupErrors.length} backup file(s) could not be removed:\n${cleanupErrors.join("\n")}`);
|
||||
}
|
||||
}
|
||||
console.log(`${args.includes("--apply") ? "Applied" : "Dry run"}: ${count} identities validated; ${changed} asset files ${args.includes("--apply") ? "updated" : "would change"}.`);
|
||||
|
|
|
|||
Loading…
Reference in New Issue