fix(labs): synchronous confirm transaction — async callback broke atomicity
bun-sqlite's Database.transaction is synchronous, but async callbacks return a pending Promise at the first await, causing immediate COMMIT before the entire callback completes. This allowed partial inserts with no rollback. Fixed by: - Remove async from transaction callback - Add .run() to each insert/update to execute synchronously - Add regression test proving atomicity: transaction that throws mid-loop rolls back all changes (both tables empty after failure) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -87,19 +87,19 @@ export function labsRoutes(deps: LabsDeps) {
|
|||||||
panel: m.panel, name: m.name, value: m.value, unit: m.unit,
|
panel: m.panel, name: m.name, value: m.value, unit: m.unit,
|
||||||
referenceRange: m.referenceRange, flagged: m.flagged,
|
referenceRange: m.referenceRange, flagged: m.flagged,
|
||||||
}));
|
}));
|
||||||
await deps.db.transaction(async (tx) => {
|
deps.db.transaction((tx) => {
|
||||||
await tx.insert(labDraws).values({
|
tx.insert(labDraws).values({
|
||||||
id: drawId, collectedAt: body.data.collectedDate, labName: body.data.labName,
|
id: drawId, collectedAt: body.data.collectedDate, labName: body.data.labName,
|
||||||
draftId: row.id, createdAt: Date.now(),
|
draftId: row.id, createdAt: Date.now(),
|
||||||
});
|
}).run();
|
||||||
for (const n of norm) {
|
for (const n of norm) {
|
||||||
await tx.insert(biomarkers).values({
|
tx.insert(biomarkers).values({
|
||||||
drawId, panel: n.panel, name: n.name, marker: n.marker, analyteKey: n.analyteKey,
|
drawId, panel: n.panel, name: n.name, marker: n.marker, analyteKey: n.analyteKey,
|
||||||
value: n.value, valueNum: n.valueNum, unit: n.unit, referenceRange: n.referenceRange,
|
value: n.value, valueNum: n.valueNum, unit: n.unit, referenceRange: n.referenceRange,
|
||||||
flagged: n.flagged ? 1 : 0, valueCanonical: n.valueCanonical, canonicalUnit: n.canonicalUnit,
|
flagged: n.flagged ? 1 : 0, valueCanonical: n.valueCanonical, canonicalUnit: n.canonicalUnit,
|
||||||
});
|
}).run();
|
||||||
}
|
}
|
||||||
await tx.update(labDrafts).set({ status: "confirmed" }).where(eq(labDrafts.id, row.id));
|
tx.update(labDrafts).set({ status: "confirmed" }).where(eq(labDrafts.id, row.id)).run();
|
||||||
});
|
});
|
||||||
return c.json({ drawId }, 201);
|
return c.json({ drawId }, 201);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ import { join } from "node:path";
|
|||||||
import { eq } from "drizzle-orm";
|
import { eq } from "drizzle-orm";
|
||||||
import { createApp } from "../src/app";
|
import { createApp } from "../src/app";
|
||||||
import { openDb } from "../src/db";
|
import { openDb } from "../src/db";
|
||||||
import { biomarkers, labDrafts } from "../src/db/schema";
|
import { biomarkers, labDrafts, labDraws } from "../src/db/schema";
|
||||||
import { loadOrCreateKey } from "../src/lib/crypto";
|
import { loadOrCreateKey } from "../src/lib/crypto";
|
||||||
|
|
||||||
async function setup() {
|
async function setup() {
|
||||||
@@ -83,4 +83,23 @@ describe("draft review", () => {
|
|||||||
});
|
});
|
||||||
expect(bad.status).toBe(400);
|
expect(bad.status).toBe(400);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test("sync transaction rolls back on mid-loop failure", () => {
|
||||||
|
const dir = mkdtempSync(join(tmpdir(), "helios-"));
|
||||||
|
const db = openDb(dir);
|
||||||
|
|
||||||
|
expect(() =>
|
||||||
|
db.transaction((tx) => {
|
||||||
|
tx.insert(labDraws).values({ id: "dX", collectedAt: "2026-01-01", labName: null, draftId: null, createdAt: 1 }).run();
|
||||||
|
tx.insert(biomarkers).values({ drawId: "dX", panel: "p", name: "n", marker: "m", analyteKey: null, value: "1", valueNum: 1, unit: null, referenceRange: null, flagged: 0, valueCanonical: null, canonicalUnit: null }).run();
|
||||||
|
throw new Error("boom");
|
||||||
|
}),
|
||||||
|
).toThrow("boom");
|
||||||
|
|
||||||
|
// Verify both tables are empty after rollback
|
||||||
|
const drawRows = db.select().from(labDraws).all();
|
||||||
|
const bioRows = db.select().from(biomarkers).all();
|
||||||
|
expect(drawRows).toHaveLength(0);
|
||||||
|
expect(bioRows).toHaveLength(0);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user