fix: transaction timeout rolls back applied operations (no torn state)
Transaction.execute() checked its time budget at the top of the operation loop and threw TransactionTimeoutError from OUTSIDE the per-operation try/catch, so a mid-flight timeout bypassed rollback entirely — only per-operation failures rolled back. A bulk transact that crossed its 30s budget mid-flight left the operations already applied to canonical storage in place while the generation was never stamped: torn, generation-less state. The generation-store commit path's abort cleanup explicitly assumes a throw from execute() already restored the applied operations byte-identically (it only discards the uncommitted staging directory), so the missing rollback broke that invariant. Give execute() a single rollback point: the operation loop is the sole rollback-guarded region, and any error escaping it — an operation failure OR a mid-flight timeout — rolls back every applied operation in reverse order, then surfaces the original error (a rollback failure still supersedes it via TransactionRollbackError). The per-exit-path rollback that let the timeout throw slip past is gone; atomicity now holds by construction for every error type. An aborted transaction leaves generation() unchanged and storage byte-identical to its pre-transaction state. Regression (tests/unit/transaction/timeout-rollback.test.ts): a mid-flight timeout leaves an in-memory canonical store byte-identical with the tx in the rolled_back terminal state; the operation-failure and rollback-failure paths through the same single rollback point; and a clean transaction still commits.
This commit is contained in:
parent
41dc307bb9
commit
508a8e363e
2 changed files with 211 additions and 23 deletions
|
|
@ -98,49 +98,60 @@ export class Transaction implements TransactionContext {
|
|||
}
|
||||
|
||||
try {
|
||||
// Execute each operation in order
|
||||
// Execute each operation in order. This loop is the sole rollback-guarded
|
||||
// region: ANY error that escapes it — an operation failure OR a
|
||||
// mid-flight timeout — is caught below and rolls back every operation
|
||||
// applied so far, in reverse order. That single guarantee is the
|
||||
// transaction's atomicity contract, and the generation-store commit path
|
||||
// depends on it: its abort cleanup assumes a throw from here already
|
||||
// restored the applied operations byte-identically (it only discards the
|
||||
// uncommitted staging directory). A rollback per exit path — the previous
|
||||
// design — let the timeout throw slip past rollback and strand the
|
||||
// already-applied writes as torn, generation-less state in canonical
|
||||
// storage.
|
||||
for (let i = 0; i < this.operations.length; i++) {
|
||||
// Check timeout
|
||||
// Budget check BEFORE starting the next operation. A trip here throws
|
||||
// into the catch below and rolls back like any other failure — it must
|
||||
// never bypass rollback.
|
||||
if (Date.now() - this.startTime > this.options.timeout) {
|
||||
throw new TransactionTimeoutError(this.options.timeout, i)
|
||||
}
|
||||
|
||||
const operation = this.operations[i]
|
||||
|
||||
if (this.options.logging) {
|
||||
prodLog.info(`[Transaction] Executing operation ${i}: ${operation.name || 'unnamed'}`)
|
||||
}
|
||||
|
||||
let rollbackAction: RollbackAction | undefined
|
||||
try {
|
||||
if (this.options.logging) {
|
||||
prodLog.info(`[Transaction] Executing operation ${i}: ${operation.name || 'unnamed'}`)
|
||||
}
|
||||
|
||||
// Execute operation
|
||||
const rollbackAction = await operation.execute()
|
||||
|
||||
// Record rollback action (if provided)
|
||||
if (rollbackAction) {
|
||||
this.rollbackActions.push(rollbackAction)
|
||||
}
|
||||
|
||||
rollbackAction = await operation.execute()
|
||||
} catch (error) {
|
||||
// Operation failed - rollback and re-throw
|
||||
const executionError = new TransactionExecutionError(
|
||||
// Normalize an operation failure to a TransactionExecutionError; the
|
||||
// outer catch performs the (single) rollback and surfaces it.
|
||||
throw new TransactionExecutionError(
|
||||
`Operation ${i} failed: ${(error as Error).message}`,
|
||||
i,
|
||||
operation.name,
|
||||
error as Error
|
||||
)
|
||||
}
|
||||
|
||||
await this.rollback(executionError)
|
||||
throw executionError
|
||||
// Record rollback action (if the operation provided one)
|
||||
if (rollbackAction) {
|
||||
this.rollbackActions.push(rollbackAction)
|
||||
}
|
||||
}
|
||||
|
||||
// All operations succeeded - commit
|
||||
this.commit()
|
||||
|
||||
} catch (error) {
|
||||
// Error already handled in rollback
|
||||
// Single rollback point: undo everything applied so far, in reverse
|
||||
// order, then surface the original error. A rollback failure supersedes
|
||||
// it (rollback() throws TransactionRollbackError wrapping this error).
|
||||
await this.rollback(error as Error)
|
||||
throw error
|
||||
}
|
||||
|
||||
// All operations succeeded — commit.
|
||||
this.commit()
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue