Found while fixing #2292; independent of it, and present on main today.
What happens
ImmediateTransaction.save() is the commit trigger:
save(...args) {
const transaction = args[0];
if (this.isCommitting) {
super.save(transaction, null, true); // <- return value dropped
} else {
this.isCommitting = true;
return when(this.commit(), () => { this.isCommitting = false; });
}
}
Once such a transaction has committed, commit() has left it CLOSED and rotateAfterMidScopeCommit will not reopen it (it is not scopeOwned). It stays in context.transaction, so txnForContext hands it back for the next write to that database. On that write:
addWrite → save(operation) → isCommitting === false → this.commit().
commit()'s save loop re-enters the override with isCommitting === true, so it calls super.save(operation, null, true).
this.open !== OPEN, so that inner save() takes the immediateCommit branch (resources/DatabaseTransaction.ts) and returns this.commit({ …options, transaction }) — the promise that owns the native commit. The isCommitting branch discards it, and the commit loop discards it too.
- The outer
commit() finds this.transaction unset (nothing was attached to a CLOSED instance), so commitResolution is never assigned and it returns a synchronous { txnTime }.
Consequence
await resource.save() (and await table.put(...) on that context) resolves before RocksDB has committed the write:
- a crash in that window loses a write the handler already acknowledged;
- a read-back inside the same handler can see the pre-write record;
- if that commit rejects, nothing handles the rejection — it surfaces as an
unhandledRejection in the worker while the client has already been sent 2xx.
The write itself normally lands, so this is a lost guarantee rather than a lost write in the common case, which is also why it is invisible.
Reaching it
Any second write to the same database through a context whose slot holds an ImmediateTransaction that has already committed once — i.e. an autocommit write path with no transaction() scope. txnForContext installs such an instance whenever the slot is empty or holds the released placeholder and something resolves a transaction without going through the static-API wrappers (an instance load is the usual route).
Candidate fix
Propagate the promise instead of dropping it: return super.save(...) from the isCommitting branch, and have commit()'s save loop stageCompletion() a thenable result so the outer commit awaits it. Both are small, but they are in the commit path every autocommit write takes, so this wants its own test design — instrumenting the native commit to reject and asserting the handler sees it, rather than asserting durability (which holds either way).
Not this issue
The atomicity/join-gate defect in #2292. That fix makes this reachable one extra way (a chained link for a second database is now itself an ImmediateTransaction, so it can be re-entered after closing), but the shape above predates it and the head slot has always had it.
Found while fixing #2292; independent of it, and present on
maintoday.What happens
ImmediateTransaction.save()is the commit trigger:Once such a transaction has committed,
commit()has left itCLOSEDandrotateAfterMidScopeCommitwill not reopen it (it is notscopeOwned). It stays incontext.transaction, sotxnForContexthands it back for the next write to that database. On that write:addWrite→save(operation)→isCommitting === false→this.commit().commit()'s save loop re-enters the override withisCommitting === true, so it callssuper.save(operation, null, true).this.open !== OPEN, so that innersave()takes theimmediateCommitbranch (resources/DatabaseTransaction.ts) and returnsthis.commit({ …options, transaction })— the promise that owns the native commit. TheisCommittingbranch discards it, and the commit loop discards it too.commit()findsthis.transactionunset (nothing was attached to a CLOSED instance), socommitResolutionis never assigned and it returns a synchronous{ txnTime }.Consequence
await resource.save()(andawait table.put(...)on that context) resolves before RocksDB has committed the write:unhandledRejectionin the worker while the client has already been sent 2xx.The write itself normally lands, so this is a lost guarantee rather than a lost write in the common case, which is also why it is invisible.
Reaching it
Any second write to the same database through a context whose slot holds an
ImmediateTransactionthat has already committed once — i.e. an autocommit write path with notransaction()scope.txnForContextinstalls such an instance whenever the slot is empty or holds the released placeholder and something resolves a transaction without going through the static-API wrappers (an instance load is the usual route).Candidate fix
Propagate the promise instead of dropping it:
return super.save(...)from theisCommittingbranch, and havecommit()'s save loopstageCompletion()a thenable result so the outer commit awaits it. Both are small, but they are in the commit path every autocommit write takes, so this wants its own test design — instrumenting the native commit to reject and asserting the handler sees it, rather than asserting durability (which holds either way).Not this issue
The atomicity/join-gate defect in #2292. That fix makes this reachable one extra way (a chained link for a second database is now itself an
ImmediateTransaction, so it can be re-entered after closing), but the shape above predates it and the head slot has always had it.