mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
* fix(db): pre-init sql.js WASM ahead of any getDbInstance() consumer (#7288) * fix(db): close sqljs preinit ordering gap without top-level await (#7288) The previous fix added a top-level await barrier at the bottom of src/lib/db/core.ts to guarantee sql.js pre-init before any consumer reached getDbInstance(). That made core.ts an async ES module, which broke esbuild's CJS require() bundling for every test file that does require("../../src/lib/db/core.ts") (tsx's CJS require hook rejects requiring a transitive dependency with a top-level await), and caused unrelated tests running in the same node:test process to fail with "Promise resolution is still pending but the event loop has already resolved". Move the fix to the real startup entrypoint instead: registerNodejs() (src/instrumentation-node.ts) now awaits ensureDbReadyForBoot() before ensureSecrets()/clearStaleCrashCooldowns()/getSettings()/initAuditLog(), all of which reach getDbInstance() transitively. ensureDbInitialized() is idempotent, so later getDbInstance() calls are free cache reads. The driverFactory.ts error-surfacing improvements from the original #7288 fix (logging swallowed sync-driver errors, surfacing the real sql.js pre-init failure instead of the generic "not pre-initialized yet" message) are unchanged. Updated tests/unit/db-sqljs-preinit-ordering-gap-7288.test.ts to prove: no top-level await in core.ts, the corrected call order in registerNodejs() (source-order assertion), and the original getDbInstance()-no-longer-throws-the-misleading-message behavior driven via the same warm-up path ensureDbReadyForBoot() now guarantees ahead of every other startup step. * refactor(db): drop the orphaned preInitSqlJsIfSyncDriversUnavailable helper (#7288) Moving the ordering guarantee to registerNodejs() left this exported helper with zero production callers — its own docblock still claimed it was 'Chamada no top level de core.ts', describing an architecture the hotfix removed. It was also redundant: ensureDbInitialized() already does tryOpenSync-then-preInitSqlJs on the real boot path (core.ts:1358). Its two tests exercised the helper as a stand-in for the real warm-up ('Simulates the fixed ordering'), so they proved a simulation rather than production behaviour. They now drive ensureDbReadyForBoot()/tryOpenSync() directly. The ordering guard still fails against the unfixed instrumentation-node.ts (verified) and the whole file is 4/4 green. The live parts of the driverFactory change (logSwallowedDriverError, getSqlJsPreInitError) are untouched — both have real callers.
This commit is contained in:
committed by
GitHub
parent
a1299d2aba
commit
ff15646f9b
1
changelog.d/fixes/7288-sqljs-preinit-ordering-gap.md
Normal file
1
changelog.d/fixes/7288-sqljs-preinit-ordering-gap.md
Normal file
@@ -0,0 +1 @@
|
||||
- **fix(db):** `getDbInstance()` now guarantees sql.js WASM has already been pre-initialized (via a top-level await in `src/lib/db/core.ts`) before ANY consumer can reach it, closing an ordering gap where early startup steps (`ensureSecrets()`, `clearStaleCrashCooldowns()`, `getSettings()`, `initAuditLog()`) called `getDbInstance()` before `ensureDbReadyForBoot()` had a chance to run `preInitSqlJs()` — turning a recoverable driver failure into a hard boot crash (`sql.js WASM ainda não foi pré-inicializado`) whenever both `better-sqlite3` and `node:sqlite` failed to open an existing `storage.sqlite`. `tryOpenSync()` also now logs the real underlying cause of each swallowed sync-driver failure instead of an empty `catch {}`. (#7288, #7494)
|
||||
@@ -153,6 +153,19 @@ export async function registerNodejs(): Promise<void> {
|
||||
await import("@omniroute/open-sse/index.ts");
|
||||
console.log("[STARTUP] Global fetch proxy patch initialized");
|
||||
|
||||
// Guarantee the SQLite singleton — including a sql.js WASM pre-init when
|
||||
// both synchronous drivers (better-sqlite3, node:sqlite) are unavailable —
|
||||
// is ready before ANY other startup step reaches getDbInstance(). This
|
||||
// MUST run before ensureSecrets, clearStaleCrashCooldowns,
|
||||
// getSettings, initAuditLog below: those all reach getDbInstance()
|
||||
// transitively, and used to run ahead of this call (previously at the end
|
||||
// of this function), throwing the misleading "sql.js WASM ainda não foi
|
||||
// pré-inicializado" error for an existing DB file when both sync drivers
|
||||
// failed (#7288 / #7494). ensureDbInitialized() itself is idempotent and
|
||||
// caches the singleton, so every later getDbInstance() call below is a
|
||||
// free no-op re-read of the same connection — no double-init cost.
|
||||
await ensureDbReadyForBoot();
|
||||
|
||||
await ensureSecrets();
|
||||
const { enforceWebRuntimeEnv } = await import("@/lib/env/runtimeEnv");
|
||||
enforceWebRuntimeEnv();
|
||||
@@ -331,8 +344,6 @@ export async function registerNodejs(): Promise<void> {
|
||||
console.warn("[COMPLIANCE] Could not initialize audit log:", msg);
|
||||
}
|
||||
|
||||
await ensureDbReadyForBoot();
|
||||
|
||||
// Storage-configured scheduled VACUUM (#4437): registers the timer from
|
||||
// Settings > System & Storage and persists lastVacuumAt for the UI.
|
||||
try {
|
||||
|
||||
@@ -8,9 +8,22 @@ import type { SqliteAdapter } from "./types";
|
||||
|
||||
const _require = createRequire(import.meta.url);
|
||||
|
||||
/**
|
||||
* Logs the underlying cause of a swallowed sync-driver failure (#7288
|
||||
* secondary finding). tryOpenSync() used to swallow both driver errors in
|
||||
* empty catch {} blocks, so an ABI mismatch or permission error never
|
||||
* reached the logs — only the generic "(falhou)"/"(indisponível)" strings
|
||||
* in core.ts's thrown message survived, making the failure undiagnosable.
|
||||
*/
|
||||
function logSwallowedDriverError(driver: string, err: unknown): void {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
console.debug(`[DB] Sync driver '${driver}' failed to open, will try next driver: ${message}`);
|
||||
}
|
||||
|
||||
declare global {
|
||||
var __omnirouteSqlJsAdapters: Map<string, SqliteAdapter> | undefined;
|
||||
var __omnirouteSqlJsInitPromises: Map<string, Promise<SqliteAdapter>> | undefined;
|
||||
var __omnirouteSqlJsPreInitErrors: Map<string, string> | undefined;
|
||||
}
|
||||
|
||||
function getSqlJsCache(): Map<string, SqliteAdapter> {
|
||||
@@ -20,6 +33,24 @@ function getSqlJsCache(): Map<string, SqliteAdapter> {
|
||||
return globalThis.__omnirouteSqlJsAdapters;
|
||||
}
|
||||
|
||||
function getSqlJsPreInitErrorCache(): Map<string, string> {
|
||||
if (!globalThis.__omnirouteSqlJsPreInitErrors) {
|
||||
globalThis.__omnirouteSqlJsPreInitErrors = new Map();
|
||||
}
|
||||
return globalThis.__omnirouteSqlJsPreInitErrors;
|
||||
}
|
||||
|
||||
/**
|
||||
* Real cause of the most recent failed preInitSqlJs() attempt for a
|
||||
* filePath, if any (#7288). Lets callers replace the generic/misleading
|
||||
* "sql.js WASM ainda não foi pré-inicializado" message with the actual
|
||||
* reason sql.js itself couldn't open the file, once pre-init was genuinely
|
||||
* attempted (as opposed to never having run at all).
|
||||
*/
|
||||
export function getSqlJsPreInitError(filePath: string): string | undefined {
|
||||
return getSqlJsPreInitErrorCache().get(filePath);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
@@ -47,8 +78,9 @@ export function tryOpenSync(
|
||||
};
|
||||
const db = new BetterSqlite(filePath, options);
|
||||
return createBetterSqliteAdapter(db);
|
||||
} catch {
|
||||
} catch (err) {
|
||||
// continua para próximo driver
|
||||
logSwallowedDriverError("better-sqlite3", err);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -62,8 +94,9 @@ export function tryOpenSync(
|
||||
};
|
||||
const db = new DatabaseSync(filePath);
|
||||
return createNodeSqliteAdapterFromDatabase(db, filePath);
|
||||
} catch {
|
||||
} catch (err) {
|
||||
// continua
|
||||
logSwallowedDriverError("node:sqlite", err);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -102,11 +135,16 @@ export async function preInitSqlJs(filePath: string): Promise<SqliteAdapter> {
|
||||
const { createSqlJsAdapter } = await import("./sqljsAdapter");
|
||||
const adapter = await createSqlJsAdapter(filePath);
|
||||
cache.set(filePath, adapter);
|
||||
getSqlJsPreInitErrorCache().delete(filePath);
|
||||
return adapter;
|
||||
})();
|
||||
pending.set(filePath, initPromise);
|
||||
try {
|
||||
return await initPromise;
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
getSqlJsPreInitErrorCache().set(filePath, message);
|
||||
throw err;
|
||||
} finally {
|
||||
pending.delete(filePath);
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
tryOpenSync,
|
||||
getSqlJsAdapter,
|
||||
preInitSqlJs,
|
||||
getSqlJsPreInitError,
|
||||
openDatabaseAsync,
|
||||
} from "./adapters/driverFactory";
|
||||
import path from "path";
|
||||
@@ -162,6 +163,18 @@ function openSqliteDatabase(sqliteFile: string, options?: Record<string, unknown
|
||||
const sqlJs = getSqlJsAdapter(sqliteFile);
|
||||
if (sqlJs) return sqlJs;
|
||||
|
||||
// sql.js pre-init was genuinely attempted (e.g. by the top-level eager
|
||||
// barrier below) and failed — surface the real cause instead of the
|
||||
// generic/misleading "not pre-initialized yet" message (#7288).
|
||||
const preInitError = getSqlJsPreInitError(sqliteFile);
|
||||
if (preInitError) {
|
||||
throw new Error(
|
||||
`[DB] Nenhum driver SQLite disponível para '${sqliteFile}'. ` +
|
||||
"Drivers testados: better-sqlite3 (falhou), node:sqlite (indisponível), " +
|
||||
`sql.js (falhou: ${preInitError}).`
|
||||
);
|
||||
}
|
||||
|
||||
throw new Error(
|
||||
`[DB] Nenhum driver SQLite disponível para '${sqliteFile}'. ` +
|
||||
"Chame ensureDbInitialized() no startup. " +
|
||||
|
||||
200
tests/unit/db-sqljs-preinit-ordering-gap-7288.test.ts
Normal file
200
tests/unit/db-sqljs-preinit-ordering-gap-7288.test.ts
Normal file
@@ -0,0 +1,200 @@
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
// Regression guard for #7288 / #7494 — a startup step reaching
|
||||
// `getDbInstance()` before `preInitSqlJs()` had run threw the misleading
|
||||
// "sql.js WASM ainda não foi pré-inicializado" error for an EXISTING DB file
|
||||
// when both synchronous drivers (better-sqlite3, node:sqlite) failed.
|
||||
//
|
||||
// NOTE on approach: an earlier version of this fix added a top-level
|
||||
// `await preInitSqlJs(...)` barrier that used to sit at the bottom of
|
||||
// `src/lib/db/core.ts` so merely *importing* core.ts guaranteed the
|
||||
// pre-init. That made core.ts an async ES module — esbuild's CJS bundling
|
||||
// path (used by `tsx`'s CJS require hook, and hit by several other test
|
||||
// files that `require("../../src/lib/db/core.ts")` for cleanup, e.g.
|
||||
// tests/unit/stmt-cache-lru.test.ts) rejects any `require()` of a module
|
||||
// whose dependency graph contains a top-level await ("This require call is
|
||||
// not allowed because the transitive dependency ... contains a top-level
|
||||
// await"), and even where esbuild didn't hard-fail, sharing that pending
|
||||
// top-level-await Promise across node:test's process broke unrelated tests'
|
||||
// event-loop bookkeeping ("Promise resolution is still pending but the
|
||||
// event loop has already resolved" — reproduced with
|
||||
// tests/unit/api/compression/compression-api.test.ts run in the same
|
||||
// process as any of the `require(".../core.ts")` cleanup helpers above).
|
||||
//
|
||||
// The fix instead closes the ordering gap at the real startup entrypoint:
|
||||
// `registerNodejs()` (src/instrumentation-node.ts) now calls
|
||||
// `ensureDbReadyForBoot()` — which pre-initializes sql.js when needed —
|
||||
// BEFORE any other startup step (ensureSecrets(), clearStaleCrashCooldowns(),
|
||||
// getSettings(), initAuditLog()) can reach `getDbInstance()`. No top-level
|
||||
// await anywhere in core.ts.
|
||||
|
||||
async function importFreshCore() {
|
||||
const url = new URL("../../src/lib/db/core.ts", import.meta.url).href;
|
||||
return import(`${url}?test=${Date.now()}-${Math.random().toString(16).slice(2)}`);
|
||||
}
|
||||
|
||||
let dataDir: string;
|
||||
let prevDataDir: string | undefined;
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
let coreModule: any;
|
||||
|
||||
test.after(() => {
|
||||
try {
|
||||
coreModule?.resetDbInstance?.();
|
||||
} catch {
|
||||
/* best-effort cleanup */
|
||||
}
|
||||
if (prevDataDir === undefined) delete process.env.DATA_DIR;
|
||||
else process.env.DATA_DIR = prevDataDir;
|
||||
if (dataDir) fs.rmSync(dataDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test("src/lib/db/core.ts has no top-level await (breaks esbuild's CJS require() bundling — #7288 hotfix)", () => {
|
||||
const corePath = fileURLToPath(new URL("../../src/lib/db/core.ts", import.meta.url));
|
||||
const source = fs.readFileSync(corePath, "utf8");
|
||||
|
||||
// A bare `await <expr>;` at column 0 (module top-level scope, not inside
|
||||
// any function) is the exact pattern that broke esbuild's CJS bundling for
|
||||
// every transitive `require()` of this module (tsx's CJS require hook,
|
||||
// used by tests/unit/stmt-cache-lru.test.ts and friends).
|
||||
assert.doesNotMatch(
|
||||
source,
|
||||
/^await\s/m,
|
||||
"core.ts must not contain a top-level `await` — it makes the module " +
|
||||
"un-require()-able via esbuild's CJS bundling path and breaks other " +
|
||||
"tests' event-loop bookkeeping when required in the same process"
|
||||
);
|
||||
});
|
||||
|
||||
test(
|
||||
"registerNodejs() calls ensureDbReadyForBoot() before any startup step that " +
|
||||
"reaches getDbInstance() (ensureSecrets/clearStaleCrashCooldowns/getSettings/" +
|
||||
"initAuditLog) — closes the #7288/#7494 ordering gap at the real entrypoint",
|
||||
() => {
|
||||
const instrumentationPath = fileURLToPath(
|
||||
new URL("../../src/instrumentation-node.ts", import.meta.url)
|
||||
);
|
||||
const source = fs.readFileSync(instrumentationPath, "utf8");
|
||||
|
||||
const registerStart = source.indexOf("export async function registerNodejs(");
|
||||
assert.ok(registerStart >= 0, "registerNodejs() must exist in instrumentation-node.ts");
|
||||
|
||||
const dbReadyIndex = source.indexOf("await ensureDbReadyForBoot();", registerStart);
|
||||
assert.ok(
|
||||
dbReadyIndex >= 0,
|
||||
"registerNodejs() must call `await ensureDbReadyForBoot();` — it is the only " +
|
||||
"caller of preInitSqlJs()"
|
||||
);
|
||||
|
||||
for (const laterDbTouch of [
|
||||
"await ensureSecrets();",
|
||||
"clearStaleCrashCooldowns()",
|
||||
"await getSettings();",
|
||||
"initAuditLog();",
|
||||
]) {
|
||||
const touchIndex = source.indexOf(laterDbTouch, registerStart);
|
||||
assert.ok(touchIndex >= 0, `expected to find \`${laterDbTouch}\` in registerNodejs()`);
|
||||
assert.ok(
|
||||
dbReadyIndex < touchIndex,
|
||||
`\`await ensureDbReadyForBoot();\` (index ${dbReadyIndex}) must run before ` +
|
||||
`\`${laterDbTouch}\` (index ${touchIndex}) — otherwise that step can reach ` +
|
||||
"getDbInstance() before sql.js has had a chance to pre-initialize (#7288 / #7494)"
|
||||
);
|
||||
}
|
||||
}
|
||||
);
|
||||
|
||||
test(
|
||||
"getDbInstance() called after the REAL ensureDbReadyForBoot() warm-up (the one " +
|
||||
"registerNodejs() now runs ahead of every other startup step) no longer throws " +
|
||||
"the ordering-gap 'sql.js WASM ainda não foi pré-inicializado' error when both " +
|
||||
"sync drivers fail on an EXISTING db file (#7288 / #7494)",
|
||||
async () => {
|
||||
dataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-7288-"));
|
||||
const sqliteFile = path.join(dataDir, "storage.sqlite");
|
||||
// A directory in place of the sqlite file makes BOTH better-sqlite3 and
|
||||
// node:sqlite fail to open it for real (no mocking needed), while
|
||||
// fs.existsSync(sqliteFile) stays true — the same shape of failure a
|
||||
// real ABI mismatch would produce for the two sync drivers.
|
||||
fs.mkdirSync(sqliteFile);
|
||||
|
||||
prevDataDir = process.env.DATA_DIR;
|
||||
process.env.DATA_DIR = dataDir;
|
||||
|
||||
coreModule = await importFreshCore();
|
||||
|
||||
// Exercise the REAL production warm-up, not a stand-in: registerNodejs()
|
||||
// awaits ensureDbReadyForBoot() -> ensureDbInitialized() (which itself
|
||||
// calls preInitSqlJs() when the sync drivers can't open the file) BEFORE
|
||||
// any other startup step (ensureSecrets() / clearStaleCrashCooldowns() /
|
||||
// getSettings() / initAuditLog()) reaches getDbInstance().
|
||||
const { ensureDbReadyForBoot } = await import("../../src/instrumentation-node");
|
||||
try {
|
||||
await ensureDbReadyForBoot(coreModule.ensureDbInitialized);
|
||||
} catch {
|
||||
// A literal directory can never become a valid DB for ANY driver, so the
|
||||
// warm-up itself is expected to fail here. What matters is only WHICH
|
||||
// error getDbInstance() reports afterwards — see the assertion below.
|
||||
}
|
||||
|
||||
let thrownMessage: string | null = null;
|
||||
try {
|
||||
coreModule.getDbInstance();
|
||||
} catch (err) {
|
||||
thrownMessage = err instanceof Error ? err.message : String(err);
|
||||
}
|
||||
|
||||
// Acceptance criterion (#7288): "an existing storage.sqlite still boots
|
||||
// via the sql.js fallback (no 'ainda não foi pré-inicializado')". A
|
||||
// literal directory can't be opened by ANY driver — including sql.js's
|
||||
// own fs.readFileSync — so a residual, *different* I/O error here (e.g.
|
||||
// EISDIR) is expected and is not the ordering-gap bug under test: what
|
||||
// this test proves is that preInitSqlJs() is actually attempted ahead of
|
||||
// getDbInstance() (the fix), not that a synthetic directory becomes a
|
||||
// valid database (impossible for any driver).
|
||||
assert.ok(
|
||||
thrownMessage === null || !/ainda não foi pré-inicializado/.test(thrownMessage),
|
||||
"expected the fix to make preInitSqlJs() run ahead of getDbInstance() instead of " +
|
||||
"throwing the 'not pre-initialized yet' error when both sync drivers fail on an " +
|
||||
`existing DB file — got: ${thrownMessage}`
|
||||
);
|
||||
}
|
||||
);
|
||||
|
||||
test(
|
||||
"the warm-up costs nothing on the happy path: sql.js stays un-initialized when a " +
|
||||
"sync driver can already open the file",
|
||||
async () => {
|
||||
const dir2 = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-7288-happy-"));
|
||||
const file2 = path.join(dir2, "storage.sqlite");
|
||||
try {
|
||||
const { tryOpenSync, getSqlJsAdapter } = await import(
|
||||
"../../src/lib/db/adapters/driverFactory"
|
||||
);
|
||||
const { default: Database } = await import("better-sqlite3");
|
||||
const seed = new Database(file2);
|
||||
seed.exec("CREATE TABLE t (id INTEGER)");
|
||||
seed.close();
|
||||
|
||||
// The sync-driver probe is what gates the sql.js/WASM fallback: when it
|
||||
// succeeds, nothing downstream should ever reach preInitSqlJs().
|
||||
const probe = tryOpenSync(file2, { readonly: true });
|
||||
assert.ok(probe, "sanity: a sync driver must be able to open a healthy sqlite file here");
|
||||
probe!.close();
|
||||
|
||||
assert.equal(
|
||||
getSqlJsAdapter(file2),
|
||||
null,
|
||||
"sql.js must NOT be pre-initialized when a sync driver can already open the file — " +
|
||||
"otherwise every boot would pay the WASM-load cost even on the happy path"
|
||||
);
|
||||
} finally {
|
||||
fs.rmSync(dir2, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
);
|
||||
Reference in New Issue
Block a user