fix(driver-sqlite-wasm): defer flush during transactions (#1494)#1501
Merged
Conversation
feat(studio): AI-draft review/diff mode in the object designer (v1) (#1456) objectui@fdd083657e2da9832059492d4c88e818a5990a8d
Under `persist: 'on-write'` an authenticated insert could fail with `COMMIT; - cannot commit - no transaction is active`. Root cause: sql.js `Database.export()` closes and reopens the database (it has no in-place serialize). Closing a connection rolls back any open transaction, so the fire-and-forget `void flush()` triggered after a write inside a Knex transaction (e.g. the autonumber sequence `BEGIN…COMMIT`) aborted that transaction, leaving the trailing COMMIT to run with no active transaction. The failure is timing-dependent, which is why objects with extra in-transaction writes (autonumber + sharing-rule recompute, like crm_account) tripped it while a bare insert usually did not. Fix: make the connection transaction-aware. Track BEGIN/COMMIT/ROLLBACK/SAVEPOINT/RELEASE; `flush()` now defers while a transaction is open and runs once it fully closes. `close()` clears the state so the final flush still persists committed data. The dialect notifies the connection of every transaction-control statement before markDirty. Adds a regression test covering multi-statement transactions, on-write disk persistence, autonumber inserts, and nested savepoints. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
added
documentation
Follow-up to the #1494 fix. Two issues surfaced in CI / under load: - A COMMIT triggered two flushes (one from noteTransactionControl, one from the dialect's trailing markDirty('run')). Overlapping sql.js export() close/reopens raced and could persist 0 bytes. Route transaction-control statements through noteTransactionControl only, so the transaction lifecycle owns flushing and fires exactly once on close. - Replace the pendingFlush + recursive re-flush mechanism with a single strictly-serialized flush chain. export() mutates the live connection (close/reopen), so two exports must never overlap, and flush() must not resolve until the caller's own write has hit disk (deterministic for close()/tests). Key the post-commit flush off flushDeferred (not dirty) so on-disconnect mode still defers to close() instead of flushing on every COMMIT. Full driver suite green across repeated full-suite runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1494 — under the
sqlite-wasmdriver, an authenticated insert could fail withCOMMIT; - cannot commit - no transaction is active.Root cause
It's a
sql.jsexport()colliding with an open transaction, not a sharing-rule/hook bug.objectstack startwith a file-backed DB usespersist: 'on-write'(standalone-stack.ts:182).'on-write'mode every write fires a fire-and-forgetvoid this.flush().flush()callsdb.export(). In sql.js 1.14.1export()is implemented as finalize-statements →sqlite3_close→ reopen — it closes and reopens the database (no in-place serialize).getNextSequenceValueBEGIN…COMMIT), the transaction is silently aborted and the trailingCOMMITruns with no active transaction.The failure is timing-dependent, which is why objects with extra in-transaction writes (autonumber + sharing-rule recompute, like
crm_account) tripped it reliably while a barecrm_leadinsert usually committed before the flush fired. The reporter's sharing-rule hypothesis was a correlated symptom, not the cause.Verified directly:
db.export()insideBEGIN…INSERTmakes the followingCOMMITthrow the exact error, reproduced end-to-end through the driver.Fix
Make the connection transaction-aware so
export()never runs mid-transaction:wasm-connection.ts: trackBEGIN/COMMIT/ROLLBACK/SAVEPOINT/RELEASE(noteTransactionControl).flush()now defers while a transaction is open and runs once it fully closes.close()clears the state so the final flush still persists committed data.knex-wasm-dialect.ts: the dialect notifies the connection of every transaction-control statement, beforemarkDirty(so aCOMMIThas already cleared the flag when the deferred flush fires).Test plan
sqlite-wasm-driver-transaction-persist.test.ts— multi-statement transactions, on-write disk persistence, autonumber inserts, nested savepoints. Fails without the fix.🤖 Generated with Claude Code