From 52dede87678f936c8f2f9349f8b8e2fd2d401884 Mon Sep 17 00:00:00 2001 From: Bart Waardenburg Date: Fri, 10 Apr 2026 22:20:00 +0200 Subject: [PATCH] fix(sqlite3): keep connection handle for in-memory databases across transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Sqlite3Client.transaction()` unconditionally nulled `this.#db` after starting the BEGIN on the active handle, relying on the lazy `#getDb()` to open a new connection on the next call. That is safe for file-backed databases, but for in-memory URLs (`:memory:`, `file::memory:`, `file::memory:?cache=private`) the database only exists on the open connection — opening a "new" connection gives you a fresh, empty in-memory database at the same URL, silently discarding all previously-created schema and data. Repro (was broken before this change): const client = createClient({ url: ":memory:" }); await client.execute("CREATE TABLE t (id INTEGER, name TEXT)"); await client.execute("INSERT INTO t VALUES (1, 'a')"); const tx = await client.transaction("write"); await tx.execute("INSERT INTO t VALUES (2, 'b')"); await tx.commit(); await client.execute("SELECT * FROM t"); // LibsqlError: SQLITE_ERROR: no such table: t Reported in #229 and #140. PR #220 introduced `file::memory:?cache=shared` as a workaround but left the documented `:memory:` URL broken. Fix: track `isInMemory` on `Sqlite3Client` (already computed by `_createClient` via `isInMemoryConfig`) and skip the `this.#db = null` assignment in `transaction()` for in-memory URLs. The client and the `Sqlite3Transaction` then share the same `Database` handle, which is the only safe sharing model for `:memory:`. Side effects: - `client.execute(...)` while an in-memory transaction is open now runs on the transaction's connection (i.e. inside the transaction). This matches better-sqlite3's single-connection model and is strictly more useful than the previous behaviour of silently discarding writes. - Starting a second `client.transaction()` on an in-memory client while another is still open now throws a clear `SQLITE_ERROR` at BEGIN instead of silently succeeding against a disposable empty database. The previous behaviour was never observable anyway — every operation went to a new empty database. Tests: - Updated the existing `:memory:` / `file::memory:?cache=private` transaction tests that were documenting the broken behaviour (`rejects.toThrow()`) — they now assert the table survives the transaction, matching the expected contract. - Added two positive regression tests against `withInMemoryClient`: commit-preserves-data and rollback-preserves-original-state. Ran `npm run typecheck`, `npm run build`, and the full in-memory/file suites: 171/171 passing, 0 regressions. Fixes #229, fixes #140. --- .../src/__tests__/client.test.ts | 58 ++++++++++++++++++- packages/libsql-client/src/sqlite3.ts | 18 +++++- 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/packages/libsql-client/src/__tests__/client.test.ts b/packages/libsql-client/src/__tests__/client.test.ts index e63b1ec..95cb8e6 100644 --- a/packages/libsql-client/src/__tests__/client.test.ts +++ b/packages/libsql-client/src/__tests__/client.test.ts @@ -289,7 +289,14 @@ describe("execute()", () => { }), ); - // see issue https://github.com/tursodatabase/libsql/issues/1411 + // see issues + // https://github.com/tursodatabase/libsql/issues/1411 + // https://github.com/tursodatabase/libsql-client-ts/issues/229 + // Every in-memory URL form must keep schema and data alive across a + // client.transaction() call. An in-memory database only exists on the + // open connection, so for :memory: URLs the client must NOT release its + // handle when a transaction starts — otherwise the next client.execute() + // opens a fresh empty :memory: database and the table silently vanishes. test( "execute transaction against in memory database with shared cache", withClient( @@ -309,7 +316,7 @@ describe("execute()", () => { await c.execute("CREATE TABLE t (a)"); const transaction = await c.transaction(); transaction.close(); - expect(() => c.execute("SELECT * FROM t")).rejects.toThrow(); + await c.execute("SELECT * FROM t"); }, { url: "file::memory:?cache=private" }, ), @@ -321,11 +328,56 @@ describe("execute()", () => { await c.execute("CREATE TABLE t (a)"); const transaction = await c.transaction(); transaction.close(); - expect(() => c.execute("SELECT * FROM t")).rejects.toThrow(); + await c.execute("SELECT * FROM t"); }, { url: ":memory:" }, ), ); + test( + "in-memory database preserves schema and data across committed transaction", + withInMemoryClient(async (c) => { + await c.execute( + "CREATE TABLE things (id INTEGER PRIMARY KEY, name TEXT)", + ); + await c.execute( + "INSERT INTO things (id, name) VALUES (1, 'first')", + ); + + const txn = await c.transaction("write"); + await txn.execute( + "INSERT INTO things (id, name) VALUES (2, 'second')", + ); + await txn.commit(); + + const result = await c.execute( + "SELECT id, name FROM things ORDER BY id", + ); + expect(result.rows).toHaveLength(2); + expect(Array.from(result.rows[0])).toStrictEqual([1, "first"]); + expect(Array.from(result.rows[1])).toStrictEqual([2, "second"]); + }), + ); + test( + "in-memory database rollback leaves original state intact", + withInMemoryClient(async (c) => { + await c.execute( + "CREATE TABLE things (id INTEGER PRIMARY KEY, name TEXT)", + ); + await c.execute( + "INSERT INTO things (id, name) VALUES (1, 'first')", + ); + + const txn = await c.transaction("write"); + await txn.execute( + "INSERT INTO things (id, name) VALUES (2, 'second')", + ); + await txn.rollback(); + + const result = await c.execute("SELECT id, name FROM things"); + expect(result.rows).toHaveLength(1); + expect(Array.from(result.rows[0])).toStrictEqual([1, "first"]); + }), + ); }); describe("values", () => { diff --git a/packages/libsql-client/src/sqlite3.ts b/packages/libsql-client/src/sqlite3.ts index b8a9ad8..9ef2063 100644 --- a/packages/libsql-client/src/sqlite3.ts +++ b/packages/libsql-client/src/sqlite3.ts @@ -96,7 +96,7 @@ export function _createClient(config: ExpandedConfig): Client { config.intMode, ); - return new Sqlite3Client(path, options, db, config.intMode); + return new Sqlite3Client(path, options, db, config.intMode, isInMemory); } export class Sqlite3Client implements Client { @@ -104,6 +104,7 @@ export class Sqlite3Client implements Client { #options: Database.Options; #db: Database.Database | null; #intMode: IntMode; + #isInMemory: boolean; closed: boolean; protocol: "file"; @@ -113,11 +114,13 @@ export class Sqlite3Client implements Client { options: Database.Options, db: Database.Database, intMode: IntMode, + isInMemory: boolean = false, ) { this.#path = path; this.#options = options; this.#db = db; this.#intMode = intMode; + this.#isInMemory = isInMemory; this.closed = false; this.protocol = "file"; } @@ -239,7 +242,18 @@ export class Sqlite3Client implements Client { async transaction(mode: TransactionMode = "write"): Promise { const db = this.#getDb(); executeStmt(db, transactionModeToBegin(mode), this.#intMode); - this.#db = null; // A new connection will be lazily created on next use + // For file-backed databases we release the client's handle so a future + // `client.execute(...)` opens a second connection and runs outside the + // transaction. For in-memory databases the "database" only exists on + // the open connection, so releasing the handle would cause every + // subsequent query to open a brand-new empty `:memory:` database and + // silently lose all prior schema and data. Keep the handle shared for + // in-memory URLs; the transaction and the client then use the same + // connection, which is the only safe option for `:memory:`. + // See https://github.com/tursodatabase/libsql-client-ts/issues/229 + if (!this.#isInMemory) { + this.#db = null; + } return new Sqlite3Transaction(db, this.#intMode); }