diff --git a/resources/DatabaseTransaction.ts b/resources/DatabaseTransaction.ts index 92bf948015..6fefdc9fa4 100644 --- a/resources/DatabaseTransaction.ts +++ b/resources/DatabaseTransaction.ts @@ -253,11 +253,12 @@ export type TransactionWrite = { entry?: Partial; before?: () => void | Promise; beforeIntermediate?: () => void | Promise; - commit?: (txnTime: number, existingEntry: Partial, retry: boolean, transaction: any) => void; + commit?: (txnTime: number, existingEntry: Partial, retry: boolean, transaction: any) => MaybePromise; validate?: (txnTime: number) => void; fullUpdate?: boolean; saved?: boolean; deferSave?: boolean; + skipReplicationConfirmation?: boolean; nodeName?: string; nodeId?: number; promise?: Promise; @@ -307,8 +308,8 @@ export function priorStagedWrite(operation: TransactionWrite): TransactionWrite * `[-0]` and `[null]` vs `[NaN]` are DIFFERENT stored keys that value-ish encodings (JSON, string * coercion) collapse, cross-contaminating unrelated records. So every key is mapped through the * same encoder the stores use; latin1 keeps the bytes injective in a string. Symbol keys (internal - * metadata writes) can't be key-encoded and keep native identity; so does null (topic-less - * publishes), which never stages a record. + * metadata writes) can't be key-encoded and keep native identity. Null is reserved for topic-less + * publishes and audit-only markers. */ export function writeKeyId(key: Id): unknown { if (typeof key === 'symbol' || key == null) return key; @@ -574,6 +575,51 @@ export class DatabaseTransaction implements Transaction { this.overloadChecked = true; // only check this once, don't interrupt ongoing transactions that have already made writes } + /** + * The stored entry of the last write eligible for replication confirmation. Two kinds of write are + * skipped (rather than ending the search) so a trailing one cannot suppress confirmation for + * replicable writes staged earlier: writes that explicitly opt out (audit-only markers, which stage + * no record), and writes with no stored entry at all — a delete leaves a readable tombstone on any + * audited or delete-tracking table, so only a delete on a table with neither is entry-less. Such a + * `put(A); delete(B)` confirms on A's entry, whose version is this transaction's (every write in a + * transaction is stamped with one version). + */ + lastConfirmableEntry(): Partial | undefined { + for (let i = this.writes.length - 1; i >= 0; i--) { + const write = this.writes[i]; + if (!write || write.skipReplicationConfirmation) continue; + const entry = write.store.getEntry(write.key); + if (entry) return entry; + } + } + + /** + * Stage an async operation that commit() must wait for. Staging can happen a turn or more before + * commit() attaches its Promise.all, so the no-op rejection handler is attached here: without it a + * rejection in that window is an unhandled rejection (fatal under --unhandled-rejections=strict). + * The rejection still surfaces through commit()'s Promise.all. + */ + stageCompletion(completion: Promise) { + completion.then(undefined, () => {}); + this.completions.push(completion); + } + + /** + * Discard staged completions that no commit() will ever aggregate (abort path). Their rejections + * are already no-op-handled by stageCompletion(), so without this they fail silently; log instead. + * Clearing them also keeps a reused transaction's next commit() from rejecting with the previous + * batch's error. + */ + drainCompletions(): void { + if (this.completions.length === 0) return; + const completions = this.completions; + this.completions = []; + for (const completion of completions) + completion.then(undefined, (error) => + harperLogger.warn?.('A staged transaction completion failed after the transaction was aborted', error) + ); + } + addWrite(operation: TransactionWrite) { if (this.timedOut) throw transactionOpenTooLongError(); // A write is activity: it re-arms the idle limit on this link even though the reads it @@ -640,11 +686,12 @@ export class DatabaseTransaction implements Transaction { return; } let result: Promise = operation.before?.() as Promise; - if (result?.then) this.completions.push(result); + if (result?.then) this.stageCompletion(result); result = operation.beforeIntermediate?.() as Promise; - if (result?.then) this.completions.push(result); + if (result?.then) this.stageCompletion(result); } - operation.commit(txnTime, operation.entry, this.retries > 0, transaction); + const completion = operation.commit(txnTime, operation.entry, this.retries > 0, transaction) as Promise; + if (typeof completion?.then === 'function') this.stageCompletion(completion); // Sticky record that THIS write staged with its audit entry appended (log entries batch on the // native transaction and are durably written by its commit attempt — even a failed one — so // they survive the abort-after-failed-commit of the retry paths). isRetry stagings @@ -857,14 +904,10 @@ export class DatabaseTransaction implements Transaction { // if we want to wait for replication confirmation, we need to track the transaction times // and when replication notifications come in, we count the number of confirms until we reach the desired number const databaseName = this.writes[0].store.rootStore.databaseName; - const lastWrite = this.writes[this.writes.length - 1]; - if (confirmReplication && lastWrite) { + const lastEntry = this.lastConfirmableEntry(); + if (confirmReplication && lastEntry) { completions.push( - confirmReplication( - databaseName, - (lastWrite.store.getEntry(lastWrite.key) as any).version, - this.replicatedConfirmation - ) + confirmReplication(databaseName, (lastEntry as any).version, this.replicatedConfirmation) ); } } @@ -1005,7 +1048,13 @@ export class DatabaseTransaction implements Transaction { } abort(): void { while (this.readTxnsUsed > 0) this.doneReadTxn(); // release the read snapshot when we abort, we assume we don't need it + // A write-only transaction never took a read reference (getReadTxn was never called), so the loop + // above releases nothing even though save() created a native handle; release it here instead of + // leaking the handle and its snapshot until GC. abortChainAfterRetries() detaches the handle + // before calling abort(), so this is a no-op there rather than a double-abort. + if (this.transaction) this.releaseReadTxn(); this.open = TRANSACTION_STATE.CLOSED; + this.drainCompletions(); for (const write of this.writes) { if (write?.savedBlobs) cleanupUnusedBlobs(write.savedBlobs, collectRetainedFileIds(write.store.getEntry(write.key)?.value)); diff --git a/resources/LMDBTransaction.ts b/resources/LMDBTransaction.ts index 7b978f4c52..a290445415 100644 --- a/resources/LMDBTransaction.ts +++ b/resources/LMDBTransaction.ts @@ -19,7 +19,6 @@ export const TRANSACTION_STATE = { OPEN: 1, // the transaction is open and can be used for reads and writes LINGERING: 2, // the transaction has completed a read, but can be used for immediate writes }; -let outstandingCommit; let confirmReplication; export function replicationConfirmation(callback) { confirmReplication = callback; @@ -184,10 +183,18 @@ export class LMDBTransaction extends DatabaseTransaction { this.open = options?.doneWriting ? TRANSACTION_STATE.LINGERING : TRANSACTION_STATE.OPEN; let resolution; const completions = []; + let commitCompletions: Promise[]; let writeIndex = 0; this.writes = this.writes.filter((write) => write); // filter out removed entries const doWrite = (write) => { - write.commit(txnTime, write.entry, retries); + const completion = write.commit(txnTime, write.entry, retries); + if (typeof completion?.then === 'function') { + // the aggregating Promise.all is attached a turn or more later (after the conditional batch + // or the exclusive transaction resolves), so handle rejection here to keep the gap from + // producing an unhandled rejection; it still surfaces through that Promise.all + completion.then(undefined, () => {}); + (commitCompletions ??= []).push(completion); + } }; // this uses optimistic locking to submit a transaction, conditioning each write on the expected version const nextCondition = () => { @@ -222,11 +229,22 @@ export class LMDBTransaction extends DatabaseTransaction { // will fail and retry due to contention. This is used to determine when to give up on optimistic writes and // use a real (async) transaction to get exclusive access to the data if (db?.retryRisk) db.retryRisk *= 0.99; // gradually decay the retry risk - if (this.writes.length + (db?.retryRisk || 0) < MAX_OPTIMISTIC_SIZE >> retries) nextCondition(); - else { + if (this.writes.length + (db?.retryRisk || 0) < MAX_OPTIMISTIC_SIZE >> retries) { + nextCondition(); + if (commitCompletions) { + if (resolution) { + resolution = Promise.resolve(resolution).then((committed) => + Promise.all(commitCompletions).then(() => committed) + ); + } else { + // Must resolve truthy or the conflict branch re-runs the writes. + resolution = Promise.all(commitCompletions).then(() => true); + } + } + } else { // if it is too big to expect optimistic writes to work, or we have done too many retries we use // a real LMDB transaction to get exclusive access to reading and writing - resolution = this.writes[0].store.transaction(() => { + const transactionResolution = this.writes[0].store.transaction(() => { for (const write of this.writes) { // we load latest data while in the transaction write.entry = write.store.getEntry(write.key); @@ -234,17 +252,13 @@ export class LMDBTransaction extends DatabaseTransaction { } return true; // success. always success }); + resolution = transactionResolution.then((committed) => + commitCompletions ? Promise.all(commitCompletions).then(() => committed) : committed + ); } } if (resolution) { - if (!outstandingCommit) { - outstandingCommit = resolution; - outstandingCommit.then(() => { - outstandingCommit = null; - }); - } - return resolution.then((resolution) => { if (resolution) { if (this.next) { @@ -257,14 +271,10 @@ export class LMDBTransaction extends DatabaseTransaction { // if we want to wait for replication confirmation, we need to track the transaction times // and when replication notifications come in, we count the number of confirms until we reach the desired number const databaseName = this.writes[0].store.rootStore.databaseName; - const lastWrite = this.writes[this.writes.length - 1]; - if (confirmReplication && lastWrite) + const lastEntry = this.lastConfirmableEntry(); + if (confirmReplication && lastEntry) completions.push( - confirmReplication( - databaseName, - (lastWrite.store.getEntry(lastWrite.key) as any).localTime, - this.replicatedConfirmation - ) + confirmReplication(databaseName, (lastEntry as any).localTime, this.replicatedConfirmation) ); } // commit succeeded; clean up files for any writes whose commit-handler took an early-return, @@ -315,6 +325,7 @@ export class LMDBTransaction extends DatabaseTransaction { abort(): void { while (this.readTxnsUsed > 0) this.doneReadTxn(); // release the read snapshot when we abort, we assume we don't need it this.open = TRANSACTION_STATE.CLOSED; + this.drainCompletions(); // any blobs that were pre-saved as part of these writes will never be referenced; schedule deletion // (retaining any fileId the current on-disk record still references — an aborted write may carry an // already-saved blob shared with the surviving record; see harper-pro#406). diff --git a/resources/Table.ts b/resources/Table.ts index c96c7e2849..bcf0b5971c 100644 --- a/resources/Table.ts +++ b/resources/Table.ts @@ -4511,7 +4511,7 @@ export function makeTable(options) { logger.trace?.(`Publishing message to id: ${id}, timestamp: ${new Date(txnTime).toISOString()}`); // always audit this, but don't change existing version // TODO: Use direct writes in the future (copying binary data is hard because it invalidates the cache) - updateRecord( + return updateRecord( id, existingEntry?.value ?? null, existingEntry, @@ -4555,8 +4555,9 @@ export function makeTable(options) { tableTxn.addWrite({ key: null, store: primaryStore, + skipReplicationConfirmation: true, commit: (txnTime: number, _existingEntry: any, _retry: any, transaction: any) => { - updateRecord( + return updateRecord( null, // recordId: null — a whole-table signal, not a per-row change undefined, // no record to store: this writes the audit entry only undefined, diff --git a/unitTests/apiTests/cache-test.mjs b/unitTests/apiTests/cache-test.mjs index 8eff9dc5c6..a2fba811f6 100644 --- a/unitTests/apiTests/cache-test.mjs +++ b/unitTests/apiTests/cache-test.mjs @@ -3,7 +3,6 @@ import { assert } from 'chai'; import axios from 'axios'; import { setupTestApp, baseUrl } from './setupTestApp.mjs'; -import { setTimeout as delay } from 'node:timers/promises'; import { waitFor } from '../waitFor.js'; describe('test REST calls with cache table', () => { @@ -35,8 +34,13 @@ describe('test REST calls with cache table', () => { delete data.ageInMonths; // harper#1484: computed scalars now also surface on default reads response = await axios.put(`${baseUrl}/FourProp/3`, data); assert.equal(response.status, 204); - await delay(20); - response = await axios(`${baseUrl}/SimpleCache/3`); + response = await waitFor( + async () => { + const cacheResponse = await axios(`${baseUrl}/SimpleCache/3`, { validateStatus: (status) => status < 500 }); + return cacheResponse.status === 200 && cacheResponse.data?.name === 'name change' ? cacheResponse : false; + }, + { timeout: 5000, interval: 20, message: 'source update did not reach SimpleCache' } + ); assert.equal(response.status, 200); assert.equal(response.data.id, 3); assert.equal(response.data.name, 'name change'); diff --git a/unitTests/resources/reloadMarker.test.js b/unitTests/resources/reloadMarker.test.js index e132a2313a..e97621a9e6 100644 --- a/unitTests/resources/reloadMarker.test.js +++ b/unitTests/resources/reloadMarker.test.js @@ -3,9 +3,17 @@ const { setupTestDBPath } = require('../testUtils'); const { table } = require('#src/resources/databases'); const { setMainIsWorker } = require('#js/server/threads/manageThreads'); const { LOCAL_ONLY } = require('#src/resources/auditStore'); -const { setTimeout: delay } = require('node:timers/promises'); +const { + DatabaseTransaction, + replicationConfirmation: rocksReplicationConfirmation, +} = require('#src/resources/DatabaseTransaction'); +const { replicationConfirmation: lmdbReplicationConfirmation } = require('#src/resources/LMDBTransaction'); +const { waitFor } = require('../waitFor'); require('#src/server/serverHelpers/serverUtilities'); +const isLMDB = process.env.HARPER_STORAGE_ENGINE === 'lmdb'; +const setReplicationConfirmation = isLMDB ? lmdbReplicationConfirmation : rocksReplicationConfirmation; + // A table-reload marker is a whole-table signal (harper-pro#489): one LOCAL_ONLY audit entry of type // 'reload' with no record, delivered to every subscriber on the table so they re-read it. It exists so // copyApply (which back-fills base-copy rows as snapshots with no per-row audit entries) can cover the @@ -17,15 +25,6 @@ describe('table-reload marker (harper-pro#489)', () => { setMainIsWorker(true); }); - async function waitFor(predicate, message, timeout = 5000) { - const start = Date.now(); - while (Date.now() - start < timeout) { - if (await predicate()) return; - await delay(20); - } - throw new Error('waitFor timed out: ' + message); - } - it('delivers a raw reload event to every subscriber on a system table', async function () { // System-DB subscribers (knownNodes peer discovery, hdb_certificate CA install) consume the raw // marker and run their own bespoke whole-table rescan, so for the system DB the bare 'reload' is @@ -46,8 +45,16 @@ describe('table-reload marker (harper-pro#489)', () => { await ReloadTable.writeReloadMarker(); - await waitFor(() => rootEvents.some((e) => e.type === 'reload'), 'root subscriber receives reload'); - await waitFor(() => keyEvents.some((e) => e.type === 'reload'), 'keyed subscriber receives reload'); + await waitFor(() => rootEvents.some((e) => e.type === 'reload'), { + timeout: 5000, + interval: 20, + message: 'root subscriber receives reload', + }); + await waitFor(() => keyEvents.some((e) => e.type === 'reload'), { + timeout: 5000, + interval: 20, + message: 'keyed subscriber receives reload', + }); const reload = rootEvents.find((e) => e.type === 'reload'); assert.equal(reload.value, undefined, 'reload carries no record value'); @@ -72,10 +79,11 @@ describe('table-reload marker (harper-pro#489)', () => { await ReloadTable.writeReloadMarker(); - await waitFor( - () => events.some((e) => e.type === 'put' && e.id === 'r1'), - 'subscriber receives the existing row as a put via the reload re-snapshot' - ); + await waitFor(() => events.some((e) => e.type === 'put' && e.id === 'r1'), { + timeout: 5000, + interval: 20, + message: 'subscriber receives the existing row as a put via the reload re-snapshot', + }); assert.ok( !events.some((e) => e.type === 'reload'), 'the bare reload marker is suppressed on a user table (re-snapshotted instead)' @@ -83,29 +91,100 @@ describe('table-reload marker (harper-pro#489)', () => { }); it('persists the marker as a local-only audit entry that never replicates', async function () { + for (const forceContention of isLMDB ? [false, true] : [false]) { + const ReloadTable = table({ + table: `ReloadMarkerAudit${forceContention ? 'Contention' : 'Optimistic'}`, + attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }], + }); + // touch the audit stream first so the table has an audit store wired up + await ReloadTable.put({ id: 1, name: 'row' }); + const originalRetryRisk = ReloadTable.primaryStore.retryRisk; + if (isLMDB) ReloadTable.primaryStore.retryRisk = forceContention ? 101 : 0; + try { + await ReloadTable.writeReloadMarker(); + } finally { + ReloadTable.primaryStore.retryRisk = originalRetryRisk; + } + + // The txn-log store's empty getRange positions at the tail (for live subscription), so scan from an + // explicit numeric start to read the existing entries. + let marker; + for (const entry of ReloadTable.auditStore.getRange({ start: 1 })) { + if (entry.type === 'reload' && entry.tableId === ReloadTable.tableId) { + marker = entry; + break; + } + } + assert.ok(marker, 'a reload audit entry was written'); + // LOCAL_ONLY makes the replication send path skip it by a bitmask test (no decode of an unknown + // type on a peer): the marker is a local signal only. + assert.ok(marker.extendedType & LOCAL_ONLY, 'reload marker is LOCAL_ONLY (never forwarded to peers)'); + assert.ok(marker.recordId == null, 'reload marker has a null recordId'); + // the regular row keeps its own audit entry; the marker did not disturb it + assert.ok((await ReloadTable.getHistoryOfRecord(1)).length >= 1, 'the real row still has its audit entry'); + } + }); + + it('confirms a stored null-key publish but not a local-only marker', async function () { const ReloadTable = table({ - table: 'ReloadMarkerAudit', - attributes: [{ name: 'id', isPrimaryKey: true }, { name: 'name' }], + table: 'ReloadMarkerConfirmation', + attributes: [{ name: 'id', isPrimaryKey: true }], }); - // touch the audit stream first so the table has an audit store wired up - await ReloadTable.put({ id: 1, name: 'row' }); - await ReloadTable.writeReloadMarker(); + let confirmationCalls = 0; + let confirmedVersion; + setReplicationConfirmation((_databaseName, version) => { + confirmationCalls++; + confirmedVersion = version; + }); + try { + const publishContext = { transaction: new DatabaseTransaction() }; + ReloadTable.publish(null, { message: 'root topic' }, publishContext); + const publishTransaction = isLMDB ? publishContext.transaction.next : publishContext.transaction; + publishTransaction.replicatedConfirmation = 1; + await publishTransaction.commit({ doneWriting: true }); + assert.equal(confirmationCalls, 1, 'a stored root-topic publish is confirmed'); + assert.ok(confirmedVersion != null, 'the confirmation uses the stored entry version'); - // The txn-log store's empty getRange positions at the tail (for live subscription), so scan from an - // explicit numeric start to read the existing entries. - let marker; - for (const entry of ReloadTable.auditStore.getRange({ start: 1 })) { - if (entry.type === 'reload') { - marker = entry; - break; - } + const context = { transaction: new DatabaseTransaction() }; + ReloadTable.writeReloadMarker(context); + const markerTransaction = isLMDB ? context.transaction.next : context.transaction; + markerTransaction.replicatedConfirmation = 1; + await markerTransaction.commit({ doneWriting: true }); + assert.equal(confirmationCalls, 1, 'a LOCAL_ONLY marker has nothing to confirm with peers'); + + // a trailing marker is exempt from confirmation, but it must not suppress confirmation for the + // replicable write staged before it in the same transaction + const mixedContext = { transaction: new DatabaseTransaction() }; + await ReloadTable.put(2, { id: 2 }, mixedContext); + ReloadTable.writeReloadMarker(mixedContext); + const mixedTransaction = isLMDB ? mixedContext.transaction.next : mixedContext.transaction; + mixedTransaction.replicatedConfirmation = 1; + confirmedVersion = undefined; + await mixedTransaction.commit({ doneWriting: true }); + assert.equal(confirmationCalls, 2, 'the replicable write staged before the marker is still confirmed'); + assert.ok(confirmedVersion != null, 'confirmation uses the stored entry of the replicable write'); + + // the other write the confirmation walk skips: one with no stored entry at all — a delete + // leaves nothing to read back only when the table has neither audit nor delete tracking + const NoAuditTable = table({ + table: 'ReloadMarkerNoAuditDelete', + attributes: [{ name: 'id', isPrimaryKey: true }], + audit: false, + }); + await NoAuditTable.put(1, { id: 1 }); + const deleteContext = { transaction: new DatabaseTransaction() }; + await NoAuditTable.put(2, { id: 2 }, deleteContext); + await NoAuditTable.delete(1, deleteContext); + const deleteTransaction = isLMDB ? deleteContext.transaction.next : deleteContext.transaction; + deleteTransaction.replicatedConfirmation = 1; + confirmedVersion = undefined; + await deleteTransaction.commit({ doneWriting: true }); + assert.equal(confirmationCalls, 3, 'the put staged before an entry-less delete is still confirmed'); + // LMDB confirms on the audit store's local time, which an `audit: false` table's records do not + // carry at all (pre-existing: every confirmed write on such a table passes undefined). + if (!isLMDB) assert.ok(confirmedVersion != null, 'confirmation uses the stored entry of the put, not the delete'); + } finally { + setReplicationConfirmation(undefined); } - assert.ok(marker, 'a reload audit entry was written'); - // LOCAL_ONLY makes the replication send path skip it by a bitmask test (no decode of an unknown - // type on a peer): the marker is a local signal only. - assert.ok(marker.extendedType & LOCAL_ONLY, 'reload marker is LOCAL_ONLY (never forwarded to peers)'); - assert.ok(marker.recordId == null, 'reload marker has a null recordId'); - // the regular row keeps its own audit entry; the marker did not disturb it - assert.ok((await ReloadTable.getHistoryOfRecord(1)).length >= 1, 'the real row still has its audit entry'); }); }); diff --git a/unitTests/resources/transaction.test.js b/unitTests/resources/transaction.test.js index ad5baefb71..c0cb5fc571 100644 --- a/unitTests/resources/transaction.test.js +++ b/unitTests/resources/transaction.test.js @@ -6,6 +6,7 @@ const { table } = require('#src/resources/databases'); const { setMainIsWorker } = require('#js/server/threads/manageThreads'); const { transaction } = require('#src/resources/transaction'); const { DatabaseTransaction } = require('#src/resources/DatabaseTransaction'); +const { LMDBTransaction } = require('#src/resources/LMDBTransaction'); const { IterableEventQueue } = require('#src/resources/IterableEventQueue'); const { RocksDatabase } = require('@harperfast/rocksdb-js'); const harperLogger = require('#src/utility/logging/harper_logger'); @@ -74,6 +75,119 @@ describe('Transactions', () => { assert.equal(answer.name, 'the answer'); assert.equal(answer.computed, 'the answer computed'); }); + it('waits for promise-returning commit callbacks on RocksDB', async function () { + if (isLMDB) return this.skip(); + const transaction = new DatabaseTransaction(); + transaction.db = TxnTest.primaryStore; + let settled = false; + let release; + const completion = new Promise((resolve) => (release = resolve)).then(() => { + settled = true; + }); + transaction.addWrite({ + key: 'async-commit-callback', + store: TxnTest.primaryStore, + commit: () => completion, + }); + const committed = transaction.commit({ doneWriting: true }); + setImmediate(release); + await committed; + assert.equal(settled, true, 'commit resolved only after the callback completion settled'); + }); + it('surfaces a commit-callback rejection without an unhandled rejection', async function () { + // each engine has its own no-op rejection handler on the staged completion + // (DatabaseTransaction.stageCompletion, LMDBTransaction's doWrite) + const transaction = isLMDB ? new LMDBTransaction() : new DatabaseTransaction(); + transaction.db = TxnTest.primaryStore; + const unhandled = []; + const onUnhandled = (reason) => unhandled.push(reason); + process.on('unhandledRejection', onUnhandled); + try { + transaction.addWrite({ + key: 'rejecting-commit-callback', + store: TxnTest.primaryStore, + commit: () => { + // keyed write: LMDB stages it in the conditional batch, so its aggregating Promise.all is + // attached only after that batch resolves — a turn or more after this rejection exists + if (isLMDB) TxnTest.primaryStore.put('rejecting-commit-callback', { name: 'staged' }); + return Promise.reject(new Error('audit write failed')); + }, + }); + // the staging-to-commit gap: a rejection with no consumer is reported at the end of this turn + await new Promise((resolve) => setImmediate(() => setImmediate(resolve))); + assert.deepEqual(unhandled, [], 'the staged completion has a rejection handler before commit'); + await assert.rejects( + () => transaction.commit({ doneWriting: true }), + /audit write failed/, + 'the rejection still propagates out of commit' + ); + // the commit-to-aggregation gap, which is the one LMDB's handler covers + await new Promise((resolve) => setImmediate(() => setImmediate(resolve))); + assert.deepEqual(unhandled, [], 'the completion stays handled across the commit aggregation gap'); + } finally { + process.off('unhandledRejection', onUnhandled); + } + }); + it('drains staged completions when a transaction is aborted', async function () { + if (isLMDB) return this.skip(); // LMDB creates its commit completions at commit time, not at write time + const transaction = new DatabaseTransaction(); + transaction.db = TxnTest.primaryStore; + const unhandled = []; + const onUnhandled = (reason) => unhandled.push(reason); + process.on('unhandledRejection', onUnhandled); + const warnings = []; + const originalWarn = harperLogger.warn; + harperLogger.warn = (...args) => warnings.push(args); + try { + transaction.addWrite({ + key: 'aborted-commit-callback', + store: TxnTest.primaryStore, + commit: () => Promise.reject(new Error('audit write failed after abort')), + }); + assert.equal(transaction.completions.length, 1, 'the completion is staged before the abort'); + assert.ok(transaction.transaction, 'the write-only transaction holds a native handle with no read reference'); + transaction.abort(); + assert.equal(transaction.transaction, null, 'the abort releases the native handle rather than leaking it'); + assert.deepEqual( + transaction.completions, + [], + 'an aborted batch cannot carry its completions into a later commit on a reused transaction' + ); + await new Promise((resolve) => setImmediate(() => setImmediate(resolve))); + assert.deepEqual(unhandled, [], 'the abandoned rejection does not escape as an unhandled rejection'); + assert.ok( + warnings.some((args) => /aborted/.test(args[0])), + 'the abandoned rejection is logged rather than silently dropped' + ); + } finally { + harperLogger.warn = originalWarn; + process.off('unhandledRejection', onUnhandled); + } + }); + it('waits for promise-returning commit callbacks on a keyed LMDB write', async function () { + if (!isLMDB) return this.skip(); + const transaction = new LMDBTransaction(); + transaction.db = TxnTest.primaryStore; + const order = []; + let release; + const completion = new Promise((resolve) => (release = resolve)).then(() => order.push('completion')); + transaction.addWrite({ + key: 'lmdb-async-commit-callback', + store: TxnTest.primaryStore, + // keyed write: the conditional batch stages it, so this covers doWrite's non-null-key path + commit: () => { + TxnTest.primaryStore.put('lmdb-async-commit-callback', { name: 'staged' }); + return completion; + }, + }); + // gated on the batch rather than a timer: a fixed delay a loaded runner outruns would pass silently + const committed = transaction.commit({ doneWriting: true }).then(() => order.push('commit')); + await TxnTest.primaryStore.flushed; + assert.deepEqual(order, [], 'commit has not resolved while the callback completion is pending'); + release(); + await committed; + assert.deepEqual(order, ['completion', 'commit'], 'commit resolved only after the callback completion'); + }); it('Can run txn with three tables and two databases', async function () { const context = {}; let start = Date.now();