mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(import): keep a failing audit write from replacing the import's answer
The CSV import wrote its audit row in a finally block, so an audit write that threw discarded whatever the import had settled on: the result of a finished import, the 413 of a file over the cap, or the error of one cut off part way. The failure is now noted on the wide event and the import answers as it would have.
This commit is contained in:
1 parent
e91957c017
commit
08ea2b6620
2 files changed
+69
-17
No files matched your search
@@ -47,7 +47,11 @@ const DONE = {
|
||||
failed: 0,
|
||||
} as unknown as RecordImportResult;
|
||||
|
||||
beforeEach(() => auditLog.mockReset());
|
||||
// A block body: a function returned from `beforeEach` runs as its teardown,
|
||||
// and `mockReset()` returns the mock itself.
|
||||
beforeEach(() => {
|
||||
auditLog.mockReset();
|
||||
});
|
||||
|
||||
describe("handleRecordImport audit", () => {
|
||||
it("audits a finished import as complete", async () => {
|
||||
@@ -88,6 +92,47 @@ describe("handleRecordImport audit", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("never lets a failing audit write replace the answer or the error", async () => {
|
||||
auditLog.mockImplementation(async () => {
|
||||
throw new Error("audit table unavailable");
|
||||
});
|
||||
const ok = await handleRecordImport(request(), {
|
||||
kind: "labs",
|
||||
module: "labs",
|
||||
schema,
|
||||
run: async ({ onWritten }) => {
|
||||
onWritten(400);
|
||||
return DONE;
|
||||
},
|
||||
});
|
||||
expect(ok.status).toBe(200);
|
||||
// A cut-off import still surfaces its own error.
|
||||
await expect(
|
||||
handleRecordImport(request(), {
|
||||
kind: "labs",
|
||||
module: "labs",
|
||||
schema,
|
||||
run: async ({ onWritten }) => {
|
||||
onWritten(200);
|
||||
throw new Error("client went away");
|
||||
},
|
||||
}),
|
||||
).rejects.toThrow("client went away");
|
||||
// And the size cap still answers 413.
|
||||
const { CsvTooLargeError } = await import("@/lib/import/csv-stream");
|
||||
const tooLarge = await handleRecordImport(request(), {
|
||||
kind: "labs",
|
||||
module: "labs",
|
||||
schema,
|
||||
run: async ({ onWritten }) => {
|
||||
onWritten(200);
|
||||
throw new CsvTooLargeError(16);
|
||||
},
|
||||
});
|
||||
expect(tooLarge.status).toBe(413);
|
||||
expect(auditLog).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("writes no audit row for a dry run or an abort that wrote nothing", async () => {
|
||||
await handleRecordImport(request("1"), {
|
||||
kind: "labs",
|
||||
|
||||
@@ -126,26 +126,33 @@ export async function handleRecordImport<
|
||||
} finally {
|
||||
// The audit row is written whatever happened: an import cut off part way
|
||||
// (the client went away, a proxy timed out, the cap was hit after some
|
||||
// chunks) has already written rows, and the trail says how many.
|
||||
// chunks) has already written rows, and the trail says how many. A
|
||||
// failing audit write is noted on the wide event and never replaces the
|
||||
// import's own answer or error: a throw from `finally` would discard
|
||||
// both, and the rows are written either way.
|
||||
if (
|
||||
!query.dryRun &&
|
||||
((result !== null && result.rows > 0) || written > 0)
|
||||
) {
|
||||
await auditLog(action, {
|
||||
userId: user.id,
|
||||
ipAddress: getClientIp(request),
|
||||
details:
|
||||
result !== null
|
||||
? {
|
||||
rows: result.rows,
|
||||
inserted: result.inserted,
|
||||
duplicates: result.duplicates,
|
||||
failed: result.failed,
|
||||
partial: false,
|
||||
written,
|
||||
}
|
||||
: { partial: true, written },
|
||||
});
|
||||
try {
|
||||
await auditLog(action, {
|
||||
userId: user.id,
|
||||
ipAddress: getClientIp(request),
|
||||
details:
|
||||
result !== null
|
||||
? {
|
||||
rows: result.rows,
|
||||
inserted: result.inserted,
|
||||
duplicates: result.duplicates,
|
||||
failed: result.failed,
|
||||
partial: false,
|
||||
written,
|
||||
}
|
||||
: { partial: true, written },
|
||||
});
|
||||
} catch {
|
||||
annotate({ meta: { import_audit_failed: true } });
|
||||
}
|
||||
}
|
||||
}
|
||||
if (result.rows === 0) {
|
||||
|
||||
Reference in new issue
Block a user