From ec553dd9c027aae922c97fbdab558ca6cbbee219 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Sat, 11 Jul 2026 11:21:37 -0300 Subject: [PATCH] fix(db): share sql.js preinit across callers, fix named-param bind (#6628, #6802) (#6899) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - preInitSqlJs() now memoizes an in-flight Promise (not just the resolved adapter) per filePath, so concurrent BATCH/STARTUP/HealthCheck/ ProviderLimitsSync callers at boot share one full-file read+WASM decode instead of each independently reloading the whole database — the thundering-herd amplifier of the OOM condition #6632 already partly fixed, left un-implemented by the reporter's own proposed fix (#6628). - sqljsAdapter's run/get/all now unwrap a lone named-parameter object (e.g. .all({ isActive: 1 }) for "WHERE is_active = @isActive", the same call shape getProviderConnections() already uses against better-sqlite3) before calling sql.js's stmt.bind(), expanding it to the @/:/$ sigil variants sql.js's own named-bind path requires. Previously the object was wrapped into an array and sql.js took the positional-bind path, throwing "Wrong API use : tried to bind a value of an unknown type ([object Object])." whenever the sql.js WASM fallback driver was active — exactly the error #6802 reported (misattributed to better-sqlite3). Regression tests added to tests/unit/db-adapters/driverFactory.test.ts and tests/unit/db-adapters/sqljsAdapter.test.ts, both proven RED against the prior code and GREEN after the fix. --- changelog.d/fixes/6628-6628-sqljs-adapter.md | 2 + src/lib/db/adapters/driverFactory.ts | 39 ++++++++- src/lib/db/adapters/sqljsAdapter.ts | 60 +++++++++++++- src/types/sqljs.d.ts | 2 +- tests/unit/db-adapters/driverFactory.test.ts | 50 ++++++++++++ tests/unit/db-adapters/sqljsAdapter.test.ts | 86 ++++++++++++++++++++ 6 files changed, 231 insertions(+), 8 deletions(-) create mode 100644 changelog.d/fixes/6628-6628-sqljs-adapter.md diff --git a/changelog.d/fixes/6628-6628-sqljs-adapter.md b/changelog.d/fixes/6628-6628-sqljs-adapter.md new file mode 100644 index 0000000000..55d6a3ce9a --- /dev/null +++ b/changelog.d/fixes/6628-6628-sqljs-adapter.md @@ -0,0 +1,2 @@ +- fix(db): share one in-flight sql.js load across concurrent `preInitSqlJs()` callers to stop the boot-time thundering-herd re-decode of the whole database file (#6628) +- fix(db): unwrap lone named-parameter objects before `sql.js` `stmt.bind()` so `@`/`:`/`$`-style named placeholders bind correctly instead of throwing "Wrong API use" (#6802) diff --git a/src/lib/db/adapters/driverFactory.ts b/src/lib/db/adapters/driverFactory.ts index df0c9e0856..574abfc0f4 100644 --- a/src/lib/db/adapters/driverFactory.ts +++ b/src/lib/db/adapters/driverFactory.ts @@ -10,6 +10,7 @@ const _require = createRequire(import.meta.url); declare global { var __omnirouteSqlJsAdapters: Map | undefined; + var __omnirouteSqlJsInitPromises: Map> | undefined; } function getSqlJsCache(): Map { @@ -19,6 +20,20 @@ function getSqlJsCache(): Map { return globalThis.__omnirouteSqlJsAdapters; } +/** + * Cache das Promises de inicialização EM VOO (não resolvidas ainda), por filePath. + * Separado de getSqlJsCache() (que só guarda o adapter já resolvido) para que + * chamadores concorrentes (BATCH/STARTUP/HealthCheck/ProviderLimitsSync no boot) + * compartilhem UMA única leitura+decode do arquivo em vez de cada um chamar + * fs.readFileSync + WASM decode independentemente (#6628 — thundering herd). + */ +function getSqlJsPendingCache(): Map> { + if (!globalThis.__omnirouteSqlJsInitPromises) { + globalThis.__omnirouteSqlJsInitPromises = new Map(); + } + return globalThis.__omnirouteSqlJsInitPromises; +} + /** Tenta abrir com better-sqlite3 e node:sqlite sincronamente. Retorna null se ambos falharem. */ export function tryOpenSync( filePath: string, @@ -75,10 +90,26 @@ export async function preInitSqlJs(filePath: string): Promise { cache.delete(filePath); } - const { createSqlJsAdapter } = await import("./sqljsAdapter"); - const adapter = await createSqlJsAdapter(filePath); - cache.set(filePath, adapter); - return adapter; + // Share one in-flight load across concurrent callers for the same filePath + // (#6628): without this, each of BATCH/STARTUP/HealthCheck/ProviderLimitsSync + // independently fs.readFileSync + WASM-decode the same (possibly 300+MB) file + // at boot, multiplying peak memory pressure by the number of racing callers. + const pending = getSqlJsPendingCache(); + const inflight = pending.get(filePath); + if (inflight) return inflight; + + const initPromise = (async () => { + const { createSqlJsAdapter } = await import("./sqljsAdapter"); + const adapter = await createSqlJsAdapter(filePath); + cache.set(filePath, adapter); + return adapter; + })(); + pending.set(filePath, initPromise); + try { + return await initPromise; + } finally { + pending.delete(filePath); + } } /** Retorna adapter sql.js pré-inicializado ou null se ainda não inicializado. */ diff --git a/src/lib/db/adapters/sqljsAdapter.ts b/src/lib/db/adapters/sqljsAdapter.ts index 0d2ea317db..181481ab1a 100644 --- a/src/lib/db/adapters/sqljsAdapter.ts +++ b/src/lib/db/adapters/sqljsAdapter.ts @@ -31,6 +31,57 @@ function resolveSqlJsWasmPath(): string { return candidatePaths[0]; } +/** + * better-sqlite3's named-parameter convention lets callers bind with the bare + * property name (e.g. `{ isActive: 1 }` for a SQL placeholder written as + * `@isActive`, `:isActive`, or `$isActive` — better-sqlite3 strips the sigil + * internally). sql.js's own named-bind path (`sqlite3_bind_parameter_index`) + * requires the FULL name INCLUDING the sigil, and silently no-ops (does not + * throw) for a key it can't resolve. Expand each bare key to all three + * sigil-prefixed variants so sql.js matches whichever sigil the SQL actually + * used, while passing through any key the caller already prefixed unchanged. + */ +function withNamedParamPrefixes(obj: Record): Record { + const expanded: Record = {}; + for (const [key, value] of Object.entries(obj)) { + if (/^[:@$]/.test(key)) { + expanded[key] = value; + continue; + } + expanded[`@${key}`] = value; + expanded[`:${key}`] = value; + expanded[`$${key}`] = value; + } + return expanded; +} + +/** + * sql.js's own `stmt.bind()` dispatches on shape: an Array means positional + * bind (each element -> bind index N), a plain object means named-parameter + * bind. Callers here always pass their rest-args as an array, so a caller + * doing `.all({ isActive: 1 })` for a named placeholder (mirrors + * better-sqlite3's spread-args named-bind convention, see + * betterSqliteAdapter.ts) ends up handing sql.js `[{isActive:1}]` — an ARRAY + * containing the object — which sql.js treats as a single positional value + * and rejects with "Wrong API use : tried to bind a value of an unknown + * type (...)." (#6802). Unwrap a lone plain-object param back to the object + * itself (sigil-expanded) so sql.js takes its named-bind path instead. + */ +function toBindValue(params: unknown[]): unknown[] | Record | undefined { + if (!params.length) return undefined; + const [first] = params; + const isLoneNamedParamsObject = + params.length === 1 && + first !== null && + typeof first === "object" && + !Array.isArray(first) && + !Buffer.isBuffer(first) && + !(first instanceof Uint8Array); + return isLoneNamedParamsObject + ? withNamedParamPrefixes(first as Record) + : params; +} + async function loadSqlJs(): Promise { if (_sqlJsLib) return _sqlJsLib; const initSqlJs = ((await import("sql.js")) as { default: (typeof import("sql.js"))["default"] }) @@ -103,7 +154,8 @@ export async function createSqlJsAdapter(filePath: string): Promise): void; step(): boolean; getAsObject(): Record; free(): void; diff --git a/tests/unit/db-adapters/driverFactory.test.ts b/tests/unit/db-adapters/driverFactory.test.ts index d38fe3a846..2716abfe86 100644 --- a/tests/unit/db-adapters/driverFactory.test.ts +++ b/tests/unit/db-adapters/driverFactory.test.ts @@ -85,6 +85,56 @@ describe("driverFactory", () => { adapter.close(); }); + // #6628 (remaining gap): concurrent preInitSqlJs() calls for the same + // filePath must share ONE in-flight load instead of each caller + // independently fs.readFileSync + WASM-decoding the whole file — the + // thundering-herd amplifier of the OOM condition #6632 already partly + // fixed (restore-cycle-breaker + OOM early-abort), left un-implemented by + // the reporter's own proposed promise-sharing fix. + test("preInitSqlJs shares one in-flight load across concurrent callers", async (t) => { + const os = await import("node:os"); + const path = await import("node:path"); + // The dynamic-import namespace object is read-only; grab the mutable CJS + // `.default` (== module.exports) so readFileSync can be monkeypatched. + const fsNs = await import("node:fs"); + const fs = fsNs.default; + + const tmpFile = path.join(os.tmpdir(), `sqljs_race_${Date.now()}.sqlite`); + fs.writeFileSync(tmpFile, Buffer.alloc(1024 * 1024, 1)); + t.after(() => { + try { + fs.unlinkSync(tmpFile); + } catch {} + }); + + let readCountForTarget = 0; + const originalReadFileSync = fs.readFileSync; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (fs as any).readFileSync = (...args: Parameters) => { + if (args[0] === tmpFile) readCountForTarget += 1; + return originalReadFileSync(...args); + }; + t.after(() => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (fs as any).readFileSync = originalReadFileSync; + }); + + const [a, b, c] = await Promise.all([ + preInitSqlJs(tmpFile), + preInitSqlJs(tmpFile), + preInitSqlJs(tmpFile), + ]); + + assert.equal( + readCountForTarget, + 1, + `expected exactly 1 shared full-file read for 3 concurrent preInitSqlJs() calls, got ${readCountForTarget}` + ); + assert.equal(a, b, "concurrent callers must resolve to the SAME adapter instance"); + assert.equal(b, c, "concurrent callers must resolve to the SAME adapter instance"); + a.close(); + }); + test("cross-driver: escreve com adapter sync, relê com sql.js", async (t) => { const os = await import("node:os"); const path = await import("node:path"); diff --git a/tests/unit/db-adapters/sqljsAdapter.test.ts b/tests/unit/db-adapters/sqljsAdapter.test.ts index 27512c6143..51f53d5f7f 100644 --- a/tests/unit/db-adapters/sqljsAdapter.test.ts +++ b/tests/unit/db-adapters/sqljsAdapter.test.ts @@ -89,4 +89,90 @@ describe("sqljsAdapter", () => { assert.equal(row.name, "test-value"); reader.close(); }); + + // #6802: named-parameter object binds ("... WHERE is_active = @isActive" + // called as .all({ isActive: 1 })) must work the same way they do against + // better-sqlite3, instead of throwing sql.js's own + // "Wrong API use : tried to bind a value of an unknown type (...)." error. + describe("named-parameter object bind (#6802)", () => { + test("all() with a single named-params object mirrors getProviderConnections", async () => { + const adapter = await createSqlJsAdapter(":memory:"); + adapter.exec( + "CREATE TABLE provider_connections (id INTEGER PRIMARY KEY, provider TEXT, is_active INTEGER)" + ); + adapter + .prepare("INSERT INTO provider_connections (provider, is_active) VALUES (?, ?)") + .run("glm", 1); + adapter + .prepare("INSERT INTO provider_connections (provider, is_active) VALUES (?, ?)") + .run("openai", 0); + + const sql = + "SELECT * FROM provider_connections WHERE is_active = @isActive ORDER BY id ASC"; + const rows = adapter.prepare(sql).all({ isActive: 1 }) as Array<{ provider: string }>; + + assert.equal(rows.length, 1, "expected exactly 1 active provider connection"); + assert.equal(rows[0].provider, "glm"); + adapter.close(); + }); + + test("get() with a single named-params object resolves the row", async () => { + const adapter = await createSqlJsAdapter(":memory:"); + adapter.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, val TEXT)"); + adapter.prepare("INSERT INTO t (val) VALUES (?)").run("named-get"); + + const row = adapter.prepare("SELECT val FROM t WHERE id = @id").get({ id: 1 }) as { + val: string; + }; + assert.equal(row.val, "named-get"); + adapter.close(); + }); + + test("run() with a single named-params object binds correctly", async () => { + const adapter = await createSqlJsAdapter(":memory:"); + adapter.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, val TEXT)"); + const result = adapter + .prepare("INSERT INTO t (val) VALUES (@val)") + .run({ val: "named-run" }); + assert.equal(result.changes, 1); + + const row = adapter + .prepare("SELECT val FROM t WHERE id = ?") + .get(result.lastInsertRowid) as { val: string }; + assert.equal(row.val, "named-run"); + adapter.close(); + }); + + test("supports :name and $name sigils in addition to @name", async () => { + const adapter = await createSqlJsAdapter(":memory:"); + adapter.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, val TEXT)"); + adapter.prepare("INSERT INTO t (val) VALUES (?)").run("colon-sigil"); + adapter.prepare("INSERT INTO t (val) VALUES (?)").run("dollar-sigil"); + + const colonRow = adapter.prepare("SELECT val FROM t WHERE val = :val").get({ + val: "colon-sigil", + }) as { val: string }; + assert.equal(colonRow.val, "colon-sigil"); + + const dollarRow = adapter.prepare("SELECT val FROM t WHERE val = $val").get({ + val: "dollar-sigil", + }) as { val: string }; + assert.equal(dollarRow.val, "dollar-sigil"); + adapter.close(); + }); + + test("existing positional-array binding still works unchanged", async () => { + const adapter = await createSqlJsAdapter(":memory:"); + adapter.exec("CREATE TABLE t (id INTEGER PRIMARY KEY, a TEXT, b TEXT)"); + const result = adapter.prepare("INSERT INTO t (a, b) VALUES (?, ?)").run("x", "y"); + assert.equal(result.changes, 1); + + const row = adapter + .prepare("SELECT a, b FROM t WHERE id = ?") + .get(result.lastInsertRowid) as { a: string; b: string }; + assert.equal(row.a, "x"); + assert.equal(row.b, "y"); + adapter.close(); + }); + }); });