diff --git a/src/lib/import/__tests__/record-import-route.test.ts b/src/lib/import/__tests__/record-import-route.test.ts index 33b74f93b..934739f24 100644 --- a/src/lib/import/__tests__/record-import-route.test.ts +++ b/src/lib/import/__tests__/record-import-route.test.ts @@ -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", diff --git a/src/lib/import/record-import-route.ts b/src/lib/import/record-import-route.ts index 8035becd4..fe6e85b3a 100644 --- a/src/lib/import/record-import-route.ts +++ b/src/lib/import/record-import-route.ts @@ -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) {