fix(backups): a landed object stays a success when the ledger write does not

The per-account ledger row shared the upload's try. A pool timeout or a
connection reset on that upsert landed in the same catch as a refused upload:
the account was counted as failed, its reason pushed into the run's failure
list, and the console would go on to read never or stale for an account whose
encrypted copy was sitting in the bucket the whole time. That is the same
inversion the ledger was added to remove, on the write side.

The upload's boundary now ends where the object is durable. uploadEncryptedBackup
resolves only after the multipart completes, so the count and the largest-object
figure are taken there, and the row is written afterwards under its own boundary.
A write that fails warns with the account it belongs to and adds a run-level
count, which is a different fact from a failed upload and reads as one.

Two arms that were correct by construction and unasserted now have tests: a
failed upload writes no success row, so the previous timestamp stands, and a
run whose ledger write rejects after the object landed still reports it
uploaded.
This commit is contained in:
Marc-André Bombeck committed 2026-09-10 10:04:06 +02:00
1 parent 34d647a884
commit 86823db34e
2 files changed
+79 -18

No files matched your search

+36 -1
View File
@@ -306,8 +306,9 @@ describe("runOffhostBackup", () => {
it("counts per-user failures without aborting the whole run", async () => {
const s3 = makeS3Mock();
const upsert = vi.fn();
const prisma = {
offhostBackupState: { upsert: vi.fn() },
offhostBackupState: { upsert },
user: {
findMany: vi.fn().mockResolvedValue([{ id: "u1" }, { id: "u2" }]),
},
@@ -331,6 +332,40 @@ describe("runOffhostBackup", () => {
expect(report.uploaded).toBe(1);
expect(report.failed).toBe(1);
// The guarantee an operator leans on: an account whose upload failed keeps
// whatever `lastSuccessAt` it already had. A failed upload writes no
// success into the ledger, so yesterday's good copy is still what the
// console reports — the run does not overwrite it with today's failure.
expect(upsert).toHaveBeenCalledTimes(1);
expect(
upsert.mock.calls.map(
(call) => (call[0] as { where: { userId: string } }).where.userId,
),
).toEqual(["u1"]);
});
it("still reports a landed object as uploaded when the ledger write fails", async () => {
const s3 = makeS3Mock();
const prisma = {
offhostBackupState: {
upsert: vi.fn().mockRejectedValue(new Error("pool timeout")),
},
user: { findMany: vi.fn().mockResolvedValue([{ id: "u1" }]) },
};
const report = await runOffhostBackup(
prisma as never,
s3,
new Date("2026-05-08T00:00:00Z"),
);
// The object is in the bucket. Only the record of it is missing, and that
// is a different fact: counting it as a failure would tell the operator
// their disaster-recovery copy does not exist when it does.
expect(report.uploaded).toBe(1);
expect(report.failed).toBe(0);
expect(report.failures).toEqual([]);
expect(s3.store.has("2026-05-08/user-u1.json.enc")).toBe(true);
});
it("refuses an account whose object outgrows one multipart upload", async () => {
+43 -17
View File
@@ -28,7 +28,7 @@ import { createGzip, gunzipSync, gzipSync } from "node:zlib";
import type { PrismaClient } from "@/generated/prisma/client";
import { createRawStreamEncryptor, decryptRawStream } from "@/lib/crypto";
import { streamFullBackupJson } from "@/lib/export/full-backup-stream";
import { getEvent } from "@/lib/logging/context";
import { annotate, getEvent } from "@/lib/logging/context";
const ALGORITHM = "aes-256-gcm";
const IV_LENGTH = 12;
@@ -576,10 +576,12 @@ export async function runOffhostBackup(
let oversized = 0;
let largestObjectBytes = 0;
const failures: Array<{ userId: string; message: string }> = [];
let ledgerWriteFailures = 0;
const evt = getEvent();
for (const user of users) {
let objectBytes: number | null = null;
try {
const objectBytes = await uploadEncryptedBackup(
objectBytes = await uploadEncryptedBackup(
s3,
`${dateKey}/user-${user.id}.json.enc`,
cfg.encryptionKey,
@@ -594,21 +596,6 @@ export async function runOffhostBackup(
}),
options,
);
// The per-account ledger, written the moment the object exists. The run
// itself only ever reported counts, and a count cannot say WHICH account
// has no copy — which is the one question an operator asks about a
// backup. `new Date()` rather than the run's `now`, because on a large
// cohort the two are hours apart and the row is meant to say when this
// account's object landed.
await prisma.offhostBackupState.upsert({
where: { userId: user.id },
update: { lastSuccessAt: new Date(), sizeBytes: objectBytes },
create: {
userId: user.id,
lastSuccessAt: new Date(),
sizeBytes: objectBytes,
},
});
largestObjectBytes = Math.max(largestObjectBytes, objectBytes);
uploaded++;
} catch (err) {
@@ -622,6 +609,45 @@ export async function runOffhostBackup(
`offhost-backup user ${user.id} failed: ${message.slice(0, 200)}`,
);
}
// The per-account ledger, and deliberately NOT inside the upload's `try`.
// `uploadEncryptedBackup` resolves only after the multipart completes, so
// by here the object is durably in the bucket and this account's backup
// has succeeded whatever the database does next. A pool timeout on the
// row would otherwise un-count a copy that exists — the run would report
// one failure too many and the console would read `never` for an account
// whose disaster-recovery object is fine, which is exactly the inversion
// the ledger exists to remove.
//
// `new Date()` rather than the run's `now`, because on a large cohort the
// two are hours apart and the row is meant to say when this account's
// object landed.
if (objectBytes !== null) {
const bytes = objectBytes;
try {
await prisma.offhostBackupState.upsert({
where: { userId: user.id },
update: { lastSuccessAt: new Date(), sizeBytes: bytes },
create: {
userId: user.id,
lastSuccessAt: new Date(),
sizeBytes: bytes,
},
});
} catch (err) {
ledgerWriteFailures++;
const message = (err as Error).message ?? "unknown";
evt?.addWarning(
`offhost-backup ledger write failed for ${user.id}: ${message.slice(0, 200)}`,
);
}
}
}
if (ledgerWriteFailures > 0) {
// A distinct fact from a failed upload, and one an operator has to be able
// to tell apart: the copies are in the bucket, the console's per-account
// view of them is behind.
annotate({ meta: { offhost_ledger_write_failures: ledgerWriteFailures } });
}
return {